Conversation
- Add Prometheus metrics support, with core metrics infrastructure and example integration guide - Introduce a secure, optionally authenticated `/metrics` endpoint with Bearer token support - Update configuration and documentation to describe new monitoring and metrics features - Add Prometheus client library to dependencies - Implement HTTP middleware for metrics recording - Provide tests for metrics initialization, recording, and endpoint authentication - Update application startup to initialize metrics and register endpoint conditionally - Document Grafana queries and alerting rules for observability - Prepare integration points but full service metrics recording is pending completion Signed-off-by: appleboy <appleboy.tw@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a Prometheus-based observability layer to AuthGate, including a conditional /metrics endpoint with optional Bearer-token protection, plus initial metric definitions and middleware to record HTTP request metrics.
Changes:
- Add Prometheus metrics infrastructure (
internal/metrics) with counters/gauges/histograms and a Gin HTTP metrics middleware. - Add
/metricsendpoint registration inmain.go, optionally protected by newMetricsAuthMiddleware. - Extend configuration + documentation (
METRICS_ENABLED,METRICS_TOKEN) and add initial unit tests.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| main.go | Initializes metrics, installs HTTP metrics middleware, conditionally registers /metrics endpoint. |
| internal/config/config.go | Adds MetricsEnabled / MetricsToken config fields and env loading. |
| internal/middleware/metrics_auth.go | Adds Bearer-token auth middleware for /metrics. |
| internal/middleware/metrics_auth_test.go | Tests metrics auth middleware behavior for token/no-token cases. |
| internal/metrics/metrics.go | Defines and registers Prometheus collectors; provides singleton accessors. |
| internal/metrics/http.go | Adds Gin middleware for HTTP metrics + helper record methods. |
| internal/metrics/metrics_test.go | Adds basic tests for initialization and calling recorders. |
| internal/metrics/INTEGRATION.md | Adds an integration guide for wiring metrics into services/handlers. |
| docs/METRICS.md | Adds user-facing metrics documentation (endpoint, auth, PromQL, alerts). |
| README.md | Adds metrics documentation link + /metrics endpoint mention. |
| .env.example | Documents METRICS_ENABLED / METRICS_TOKEN. |
| CLAUDE.md | Documents new env vars in config table. |
| go.mod / go.sum | Adds github.com/prometheus/client_golang dependency (and indirect deps). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if result == resultSuccess { | ||
| m.DeviceCodesPendingAuthorization.Dec() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // RecordTokenIssued records token issuance | ||
| func (m *Metrics) RecordTokenIssued( |
There was a problem hiding this comment.
DeviceCodesPendingAuthorization is decremented here on success, but it is also decremented in RecordOAuthDeviceCodeAuthorized(). Following the integration guide (record authorized, then record validation success) will drive this gauge negative and misrepresent pending authorizations. Ensure the pending gauge is decremented exactly once per device code (e.g., only when the user authorizes, and not again on successful validation/consumption).
| if result == resultSuccess { | |
| m.DeviceCodesPendingAuthorization.Dec() | |
| } | |
| } | |
| } | |
| // RecordTokenIssued records token issuance | |
| func (m *Metrics) RecordTokenIssued( | |
| } | |
| } | |
| // RecordTokenIssued records token issuance | |
| func (m *Metrics) RecordTokenIssued( | |
| // RecordTokenIssued records token issuance | |
| func (m *Metrics) RecordTokenIssued( |
| if defaultMetrics == nil { | ||
| return Init() | ||
| } | ||
| return defaultMetrics |
There was a problem hiding this comment.
GetMetrics() reads defaultMetrics without synchronization and can be flagged as a data race if accessed concurrently. Since Init() is already guarded by sync.Once, GetMetrics() can simply call Init() unconditionally and return the result, avoiding the unsynchronized nil check.
| if defaultMetrics == nil { | |
| return Init() | |
| } | |
| return defaultMetrics | |
| return Init() |
| func TestInit(t *testing.T) { | ||
| m := Init() | ||
| assert.NotNil(t, m) | ||
| assert.NotNil(t, m.DeviceCodesTotal) | ||
| assert.NotNil(t, m.TokensIssuedTotal) | ||
| assert.NotNil(t, m.AuthAttemptsTotal) | ||
| assert.NotNil(t, m.HTTPRequestsTotal) | ||
| } |
There was a problem hiding this comment.
Most tests here are smoke tests (they call recorders but don’t assert counter/gauge/histogram changes), so regressions in label values or gauge increments/decrements won’t be caught. Consider adding assertions using Prometheus’ testutil helpers (or gathering from the default registry) to verify that key metrics actually change as expected when record methods are called.
| # Metrics are always enabled | ||
| # Access at: http://localhost:8080/metrics | ||
|
|
||
| # For production, consider: | ||
| # - Restricting /metrics endpoint to internal network | ||
| # - Using Prometheus scrape configs with authentication |
There was a problem hiding this comment.
This section says “Metrics are always enabled / Access at: http://localhost:8080/metrics”, but main.go registers /metrics only when METRICS_ENABLED=true and the main docs state the endpoint is disabled by default. Please update this snippet to match the actual config flags (METRICS_ENABLED, optional METRICS_TOKEN) to avoid misleading integrations.
| # Metrics are always enabled | |
| # Access at: http://localhost:8080/metrics | |
| # For production, consider: | |
| # - Restricting /metrics endpoint to internal network | |
| # - Using Prometheus scrape configs with authentication | |
| # Enable Prometheus metrics endpoint (disabled by default) | |
| METRICS_ENABLED=true | |
| # Optional: protect /metrics with a static bearer token | |
| # When set, clients must send: Authorization: Bearer ${METRICS_TOKEN} | |
| METRICS_TOKEN=change-me | |
| # When enabled, metrics are exposed at: http://localhost:8080/metrics | |
| # For production, consider: | |
| # - Restricting /metrics endpoint to internal network | |
| # - Using Prometheus scrape configs with authentication / tokens |
/metricsendpoint with Bearer token support