Skip to content

api: add metrics-only listener - #1637

Open
luanmdang wants to merge 3 commits into
aptly-dev:masterfrom
luanmdang:api-metrics-listener
Open

api: add metrics-only listener#1637
luanmdang wants to merge 3 commits into
aptly-dev:masterfrom
luanmdang:api-metrics-listener

Conversation

@luanmdang

Copy link
Copy Markdown

Motivation

Aptly currently exposes Prometheus metrics through the REST API listener. Operators who need a loopback or monitoring-only scrape address must expose the API on that address too.

Changes

  • Add an optional CLI-only -metrics-listen=host:port flag to aptly api serve.
  • Serve only exact GET and HEAD requests to /metrics on the dedicated plain-HTTP TCP listener.
  • Require enableMetricsEndpoint and preserve the existing /api/metrics endpoint, including HEAD support.
  • Pre-bind both listeners, stop the sibling if either server fails, and drain both concurrently with a bounded graceful shutdown before waiting for background tasks.
  • Keep systemd socket activation scoped to the API listener while binding the metrics listener separately.
  • Update command help, shell completions, the man page, and contributor metadata.
  • Add focused routing, startup, failure, shutdown, race, and end-to-end system coverage.

The listener intentionally provides no TLS or authentication; operators can apply those controls through network policy or a proxy.

Testing

  • go test ./cmd -count=10
  • go test -race ./cmd
  • go test -race ./api -check.f APISuite.TestHeadMetrics
  • go vet ./cmd ./api
  • go mod verify
  • pinned golangci-lint and flake8 checks
  • full Docker unit suite
  • full Docker system suite, split into shards

Copilot AI lite review requested due to automatic review settings September 10, 2026 17:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The man page example formatting is inconsistent and the new HEAD metrics unit test asserts a response body for HEAD, which contradicts correct HTTP semantics and the system test expectations.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds support for running a dedicated “metrics-only” HTTP listener alongside the existing Aptly API listener, enabling operators to expose /metrics on a separate address without also exposing the full REST API.

Changes:

  • Adds -metrics-listen=host:port to aptly api serve and coordinates startup/shutdown of API + metrics servers.
  • Restricts the dedicated listener to exact /metrics with only GET/HEAD, while preserving /api/metrics (now with explicit HEAD support).
  • Adds unit tests plus an end-to-end system test, and updates docs/completions.
File summaries
File Description
system/t12_api/metrics_listener.py System test covering dual listeners, routing restrictions, methods, and shutdown behavior.
man/aptly.1 Documents -metrics-listen and adds an example invocation.
completion.d/aptly Bash completion: adds -metrics-listen= for aptly api serve.
completion.d/_aptly Zsh completion: adds -metrics-listen argument.
cmd/api_serve.go Implements metrics-only listener wiring, routing adapter, and coordinated shutdown.
cmd/api_serve_test.go Unit tests for handler routing, listener validation/selection, shutdown, and sibling-stop behavior.
AUTHORS Adds contributor entry.
api/router.go Adds explicit HEAD /api/metrics route when metrics are enabled.
api/api_test.go Adds API suite coverage for HEAD /api/metrics.
Review details
  • Files reviewed: 9/9 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread api/api_test.go
Comment thread man/aptly.1
Comment on lines 2445 to 2447
.P
$ aptly api serve \-listen=:8080 $ aptly api serve \-listen=unix:///tmp/aptly\.sock
$ aptly api serve \-listen=:8080 $ aptly api serve \-listen=unix:///tmp/aptly\.sock $ aptly api serve \-listen=:8080 \-metrics\-listen=127\.0\.0\.1:9090
.
luanmdang and others added 2 commits September 10, 2026 13:02
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.14%. Comparing base (f59b0d2) to head (e8d161f).

Files with missing lines Patch % Lines
cmd/api_serve.go 79.60% 20 Missing and 11 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1637      +/-   ##
==========================================
- Coverage   77.37%   77.14%   -0.24%     
==========================================
  Files         165      165              
  Lines       15747    15891     +144     
==========================================
+ Hits        12185    12259      +74     
- Misses       2356     2421      +65     
- Partials     1206     1211       +5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@luanmdang

Copy link
Copy Markdown
Author

The remaining failures are all Debian 11/Bullseye jobs. Bullseye LTS ended 8-31, and its security repo metadata expired 9-7, so apt-get update fails before the project builds. Looks like all other lint and coverage build passes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants