diff --git a/README.md b/README.md index 04af095c..daefef1c 100644 --- a/README.md +++ b/README.md @@ -399,7 +399,7 @@ rate limiter so a scraper on a fixed interval is never throttled. | `process_resident_memory_bytes`, `nodejs_heap_size_used_bytes`, ... | gauge | Memory usage | | `process_cpu_user_seconds_total`, `process_cpu_system_seconds_total` | counter | CPU usage | | `http_requests_total` | counter | Requests by `method`, `route`, `status_code` | -| `http_request_duration_seconds` | histogram | Request latency by `method`, `route`, `status_code` | +| `http_request_duration_seconds` | histogram | Request latency by `method`, `route`, `status_code`; buckets at 10ms, 50ms, 100ms, 500ms, 1s, 5s | | `db_pool_connections_open` | gauge | Connections open in the Prisma pool | | `db_pool_connections_busy` | gauge | Connections executing a query | | `db_pool_connections_idle` | gauge | Connections open but unused | diff --git a/stellar-payment-platform/src/metrics.js b/stellar-payment-platform/src/metrics.js index f303dd36..4cbebffe 100644 --- a/stellar-payment-platform/src/metrics.js +++ b/stellar-payment-platform/src/metrics.js @@ -12,12 +12,13 @@ const httpRequestCounter = new client.Counter({ labelNames: ['method', 'route', 'status_code'], }); -// Custom histogram: request duration in seconds +// Custom histogram: request duration in seconds, bucketed for SLO work +// (10ms, 50ms, 100ms, 500ms, 1s, 5s) so p50/p95/p99 latency can be derived. const httpRequestDuration = new client.Histogram({ name: 'stellar_tags_http_request_duration_seconds', help: 'HTTP request duration in seconds', labelNames: ['method', 'route', 'status_code'], - buckets: [0.005, 0.01, 0.025, 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5, 10], + buckets: [0.01, 0.05, 0.1, 0.5, 1, 5], }); // The Prisma and Redis clients are created after this module is imported, so @@ -112,7 +113,13 @@ function metricsMiddleware(req, res, next) { res.on('finish', () => { const duration = (Date.now() - start) / 1000; - const route = req.route?.path ?? req.path ?? 'unknown'; + // Normalize the route label: use the matched route pattern (never the raw + // path, query string, or concrete ids), include the router mount prefix + // (e.g. "/api/v1"), and collapse unmatched/404 paths to "unknown" so label + // cardinality stays bounded even under arbitrary-path probing. + const route = req.route + ? `${req.baseUrl || ''}${req.route.path}` + : 'unknown'; const labels = { method: req.method, route, diff --git a/stellar-payment-platform/tests/metrics.test.js b/stellar-payment-platform/tests/metrics.test.js index bb653a8b..a71ef5fe 100644 --- a/stellar-payment-platform/tests/metrics.test.js +++ b/stellar-payment-platform/tests/metrics.test.js @@ -139,6 +139,60 @@ describe('Prometheus Metrics Endpoint', () => { expect(metricsText).toContain('status_code="404"'); }); + test('should expose the configured latency buckets (10ms, 50ms, 100ms, 500ms, 1s, 5s)', async () => { + await request(app).get('/health'); + + const response = await request(app).get('/metrics'); + const metricsText = response.text; + + for (const le of ['0.01', '0.05', '0.1', '0.5', '1', '5', '+Inf']) { + expect(metricsText).toMatch( + new RegExp(`stellar_tags_http_request_duration_seconds_bucket\\{.*le="${le.replace('+', '\\+')}"`), + ); + } + }); + + test('should record the normalized route including the API version mount', async () => { + await request(app).get('/api/v1/users?limit=5'); + + const response = await request(app).get('/metrics'); + const metricsText = response.text; + + // The route label is the matched pattern with its mount prefix, never + // the raw query string. + expect(metricsText).toMatch(/route="\/api\/v1\/users"/); + expect(metricsText).not.toMatch(/limit=5/); + }); + + test('should collapse unmatched routes to a bounded "unknown" label', async () => { + // A random path must not create a per-path label (cardinality guard). + await request(app).get('/definitely-not-a-real-route-42'); + + const response = await request(app).get('/metrics'); + const metricsText = response.text; + + expect(metricsText).toContain('status_code="404"'); + expect(metricsText).toMatch(/route="unknown"/); + expect(metricsText).not.toContain('definitely-not-a-real-route-42'); + }); + + test('should record latency for every request in the histogram', async () => { + await request(app).get('/health'); + await request(app).post('/federation').send({}); + + const response = await request(app).get('/metrics'); + const metricsText = response.text; + + // Both the sum (with a real duration value) and the count must be present + // for the duration histogram after requests are made. + expect(metricsText).toMatch( + /stellar_tags_http_request_duration_seconds_count\{.*method="GET".*\} [1-9]\d*/, + ); + expect(metricsText).toMatch( + /stellar_tags_http_request_duration_seconds_count\{.*method="POST".*\} [1-9]\d*/, + ); + }); + test('should handle different HTTP methods in metrics', async () => { // Make requests with different methods await request(app).get('/health');