feat(metrics): add Prometheus metrics for infrastructure monitoring#914
Conversation
Signed-off-by: Arturo Reuschenbach Puncernau <reuschenbach@gmail.com>
Signed-off-by: Arturo Reuschenbach Puncernau <reuschenbach@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new ChangesAurora Prometheus HTTP Metrics
Sequence DiagramsequenceDiagram
participant Fastify
participant AuroraHttpMetricsCollector
participant extractLabels
participant PrometheusRegistry
Fastify->>AuroraHttpMetricsCollector: onRequest (capture hrtime start)
Fastify->>AuroraHttpMetricsCollector: onResponse (request complete)
AuroraHttpMetricsCollector->>extractLabels: extractLabels(req, reply)
extractLabels-->>AuroraHttpMetricsCollector: {status_code, method, route, endpoint_type, project_id}
AuroraHttpMetricsCollector->>PrometheusRegistry: requestsTotal.inc(labels)
AuroraHttpMetricsCollector->>PrometheusRegistry: requestDuration.observe(labels, duration)
Fastify->>AuroraHttpMetricsCollector: onError (exception)
AuroraHttpMetricsCollector->>PrometheusRegistry: exceptionsTotal.inc({exception_class})
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ArtieReus
changed the title
Signed-off-by: Arturo Reuschenbach Puncernau <reuschenbach@gmail.com>
Signed-off-by: Arturo Reuschenbach Puncernau <reuschenbach@gmail.com>
Signed-off-by: Arturo Reuschenbach Puncernau <reuschenbach@gmail.com>
Signed-off-by: Arturo Reuschenbach Puncernau <reuschenbach@gmail.com>
There was a problem hiding this comment.
Pull request overview
Adds Prometheus metrics support to the aurora server to improve infrastructure monitoring (HTTP request volume, latency, and exceptions), including a /metrics scrape endpoint and route normalization to manage label cardinality.
Changes:
- Add
prom-clientdependency and wire a dedicatedRegistryinto the Fastify server. - Introduce a Fastify plugin that records request counters/histograms and exception counters with low-cardinality route labeling.
- Add documentation describing exposed metrics, labels, and example PromQL queries.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Locks prom-client and its transitive dependencies. |
| packages/aurora/package.json | Adds prom-client dependency. |
| packages/aurora/src/server/server.ts | Registers the metrics collector and exposes GET /metrics. |
| packages/aurora/src/server/aurora-fastify-plugins/index.ts | Exports the new metrics plugin. |
| packages/aurora/src/server/aurora-fastify-plugins/httpMetricsCollector.ts | Implements request/latency/exception metrics with route grouping and labels. |
| packages/aurora/src/server/aurora-fastify-plugins/httpMetricsCollector.test.ts | Adds tests for metrics exposure and basic labeling. |
| packages/aurora/docs/0013_prometheus_metrics.md | Documents metrics design, labels, and example queries. |
| commitlint.config.mjs | Adds metrics as an allowed commit scope. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Arturo Reuschenbach Puncernau <reuschenbach@gmail.com>
Signed-off-by: Arturo Reuschenbach Puncernau <reuschenbach@gmail.com>
Signed-off-by: Arturo Reuschenbach Puncernau <reuschenbach@gmail.com>
1. Replace createServer() with plain Fastify instance to avoid booting the full server with @fastify/vite. This makes tests faster and more reliable by focusing on unit-testing the metrics plugin behavior. 2. Improve /metrics exclusion test to actually verify that /metrics route never appears in the collected metrics, not just checking the status code.
The previous note about server tests being excluded from vitest.config.ts was misleading. Tests now run successfully using a plain Fastify instance approach that avoids Vite startup issues.
The comment said Vite dev server paths are "excluded from metrics" but they're actually recorded with endpoint_type="vite-dev" and route="vite-dev". Updated comment to clarify they're grouped (not excluded) to prevent cardinality explosion.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
packages/aurora/src/server/aurora-fastify-plugins/httpMetricsCollector.ts (4)
141-148: 💤 Low valueConsider explicit property check instead of truthy check for
project_id.The truthy check at line 141 means an explicit
input.project_id = ""would fall through to the batch lookup logic, treating an empty string as "absent" rather than "explicitly set to empty". Since empty string is your sentinel for "not applicable" (line 109), this may be acceptable, but using"project_id" in inputorinput.hasOwnProperty("project_id")would be more explicit.♻️ More explicit property presence check
- if (input.project_id) { + if ("project_id" in input && input.project_id) { projectId = input.project_id } else if (typeof input === "object") {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/aurora/src/server/aurora-fastify-plugins/httpMetricsCollector.ts` around lines 141 - 148, The condition checking input.project_id at line 141 uses a truthy check which treats an explicitly set empty string (the sentinel value for "not applicable") as falsy, causing it to fall through to the batch lookup logic. Replace the truthy check with an explicit property presence check using either "project_id" in input or input.hasOwnProperty("project_id") to properly distinguish between a property that is not set versus one that is explicitly set to an empty string.
216-224: ⚡ Quick winDocument that deeper SPA paths are intentionally truncated.
The special-case patterns for
/projectsand/accountsonly preserve the top-level structure (e.g.,/projects/:id/compute) and discard deeper segments like/instancesor/instance-123. This is a deliberate cardinality-control decision but isn't immediately obvious. A brief comment would help future maintainers understand the trade-off.📝 Suggested inline documentation
// Pattern: /projects/:id/service + // Intentionally truncates deeper paths (e.g., /projects/abc/compute/instances → /projects/:id/compute) + // to limit label cardinality while preserving service-level granularity. if (segments[0] === "projects" && segments.length >= 3) { const service = segments[2] // compute, network, storage, services return `/projects/:id/${service}` } // Pattern: /accounts/:id + // Truncates sub-paths (e.g., /accounts/abc/settings → /accounts/:id) for cardinality control. if (segments[0] === "accounts" && segments.length >= 2) { return "/accounts/:id" }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/aurora/src/server/aurora-fastify-plugins/httpMetricsCollector.ts` around lines 216 - 224, Add a comment to explain that the special-case patterns for segments[0] === "projects" and segments[0] === "accounts" deliberately truncate deeper path segments as a cardinality-control decision. The comment should clarify that while deeper segments like /instances or /instance-123 are discarded, this is intentional to limit the number of unique metric labels. Place this comment before the if-block for projects to document this trade-off between metric accuracy and cardinality management, so future maintainers understand why only the top-level resource structure (e.g., /projects/:id/compute) is preserved.
119-119: 💤 Low valueRedundant query string removal.
The
procedurePathis derived fromurlPath(line 114), which already has the query string removed at line 105. This split on"?"is redundant but harmless.♻️ Simplify by removing redundant operation
- const cleanPath = procedurePath.split("?")[0] + const cleanPath = procedurePath const parts = cleanPath.split(".")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/aurora/src/server/aurora-fastify-plugins/httpMetricsCollector.ts` at line 119, The `cleanPath` variable assignment at line 119 performs a redundant split on "?" when the source `procedurePath` is derived from `urlPath` which has already had its query string removed at line 105. Remove the redundant `.split("?")[0]` operation and assign `cleanPath` directly to `procedurePath` since it is already clean.
252-252: ⚡ Quick winInconsistent UUID validation between
normalizeApiPathandnormalizeSpaRoute.
normalizeSpaRoute(line 232) uses a strict RFC-compliant UUID regex, whilenormalizeApiPath(line 252) uses a loose pattern/^[0-9a-f-]{36}$/ithat would match invalid strings like 36 consecutive hyphens. For consistency and correctness, both functions should use the same precise UUID pattern.♻️ Use consistent UUID pattern
function normalizeApiPath(path: string): string { + const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i return path .split("/") .map((segment) => { - if (segment.match(/^\d+$/) || segment.match(/^[0-9a-f-]{36}$/i)) { + if (segment.match(/^\d+$/) || segment.match(UUID_REGEX)) { return ":id" } return segment }) .join("/") }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/aurora/src/server/aurora-fastify-plugins/httpMetricsCollector.ts` at line 252, The UUID validation pattern in normalizeApiPath uses a loose regex `/^[0-9a-f-]{36}$/i` that can match invalid strings like 36 consecutive hyphens, while normalizeSpaRoute already uses a strict RFC-compliant UUID pattern. Update the UUID validation regex in the normalizeApiPath function to match the strict pattern already implemented in normalizeSpaRoute to ensure consistency and correctness across both functions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/aurora/src/server/aurora-fastify-plugins/httpMetricsCollector.ts`:
- Around line 141-148: The condition checking input.project_id at line 141 uses
a truthy check which treats an explicitly set empty string (the sentinel value
for "not applicable") as falsy, causing it to fall through to the batch lookup
logic. Replace the truthy check with an explicit property presence check using
either "project_id" in input or input.hasOwnProperty("project_id") to properly
distinguish between a property that is not set versus one that is explicitly set
to an empty string.
- Around line 216-224: Add a comment to explain that the special-case patterns
for segments[0] === "projects" and segments[0] === "accounts" deliberately
truncate deeper path segments as a cardinality-control decision. The comment
should clarify that while deeper segments like /instances or /instance-123 are
discarded, this is intentional to limit the number of unique metric labels.
Place this comment before the if-block for projects to document this trade-off
between metric accuracy and cardinality management, so future maintainers
understand why only the top-level resource structure (e.g.,
/projects/:id/compute) is preserved.
- Line 119: The `cleanPath` variable assignment at line 119 performs a redundant
split on "?" when the source `procedurePath` is derived from `urlPath` which has
already had its query string removed at line 105. Remove the redundant
`.split("?")[0]` operation and assign `cleanPath` directly to `procedurePath`
since it is already clean.
- Line 252: The UUID validation pattern in normalizeApiPath uses a loose regex
`/^[0-9a-f-]{36}$/i` that can match invalid strings like 36 consecutive hyphens,
while normalizeSpaRoute already uses a strict RFC-compliant UUID pattern. Update
the UUID validation regex in the normalizeApiPath function to match the strict
pattern already implemented in normalizeSpaRoute to ensure consistency and
correctness across both functions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3d010c92-e408-4fc1-9778-dde4dfb738e4
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
commitlint.config.mjspackages/aurora/docs/0013_prometheus_metrics.mdpackages/aurora/package.jsonpackages/aurora/src/server/aurora-fastify-plugins/httpMetricsCollector.test.tspackages/aurora/src/server/aurora-fastify-plugins/httpMetricsCollector.tspackages/aurora/src/server/aurora-fastify-plugins/index.tspackages/aurora/src/server/server.ts
andypf
left a comment
andypf
left a comment
There was a problem hiding this comment.
Overall Review - Edge Cases & Error Handling ✅
Great implementation! I reviewed the error handling and edge cases. The good news: all error scenarios are handled defensively and won't impact the normal request flow. 🎉
Key Strengths
- ✅ All metrics collection wrapped in try-catch
- ✅ Defensive programming (checks for missing start time)
- ✅ Graceful degradation on parse errors
- ✅ No impact on application availability
Summary: Does it affect normal flow if errors happen? NO ✅
The implementation follows the golden rule: "Metrics collection must never impact application availability"
I'll add some specific inline comments with minor suggestions for improvement (all optional, not critical).
andypf
left a comment
andypf
left a comment
There was a problem hiding this comment.
💡 Suggestion: Add debug logging for skipped metrics
When metricsStartTime is missing, the metric is silently skipped. Consider adding debug logging for visibility:
if (!request.metricsStartTime) {
fastify.log.debug({ url: request.url }, 'Metrics start time missing, skipping metric collection')
return
}Impact: Better observability, helps debug if metrics are missing for certain requests.
Priority: Nice-to-have (not critical)
andypf
left a comment
andypf
left a comment
There was a problem hiding this comment.
⚠️ Note: High Cardinality Risk with project_id Label
The project_id label contains user-provided values. If you have thousands of projects, this creates high cardinality in Prometheus.
Impact:
- Each unique label combination creates a new time series
- Can affect Prometheus query performance and memory usage
- Does NOT affect application availability ✅
Recommendations:
- Document expected cardinality (e.g., "supports up to 10,000 projects")
- Consider making
project_idlabel optional via config - Monitor Prometheus memory usage in production
Is this a blocker? No, but worth documenting and monitoring.
andypf
left a comment
andypf
left a comment
There was a problem hiding this comment.
📊 Observation: Exception Tracking Coverage
The onError hook only fires for unhandled errors that reach Fastify's error handler. It won't catch:
- Errors caught and handled within route handlers
- Errors transformed to HTTP 5xx responses without throwing
- Silent failures in middleware
Impact: ✅ Does NOT affect normal flow, but exception metrics may be incomplete.
Recommendation:
For complete error visibility, also monitor status_code=~"5.." in the aurora_requests_total metric. Example Prometheus query:
rate(aurora_requests_total{status_code=~"5.."}[5m])
Optional Enhancement: Add a separate counter for 5xx responses:
if (reply.statusCode >= 500) {
serverErrorsTotal.inc({ status_code: reply.statusCode.toString() })
}This is optional - current implementation is fine!
This should be fine. This is the setup we have already in Elektra. If it turns to a problem then we skip the project.
Only tracks unhandled exceptions that reach Fastify's error handler. Errors caught and handled in route handlers (e.g., tRPC errors) are tracked via |
Add debug log when metricsStartTime is missing to improve observability and help diagnose edge cases where the onRequest hook didn't fire. Uses structured logging consistent with existing error logging patterns.
Signed-off-by: Arturo Reuschenbach Puncernau <reuschenbach@gmail.com>
Summary
Add Prometheus metrics collection to the aurora package for infrastructure monitoring. This implementation provides visibility into HTTP request performance, API usage patterns, and error rates.
The metrics are designed for production use with low cardinality and focus on backend API monitoring. A separate user behavior tracking layer will be implemented in the dashboard app in the future.
Changes Made
prom-client(v15.1.3) dependency for Prometheus metricshttpMetricsCollector.tsFastify plugin for metrics collection/metricsendpoint for Prometheus scrapingservice/actionformat (e.g.,compute/listImages)*.js,*.css,*.png)vite-dev:idplaceholdersproject_idlabel for per-project monitoringMetrics Exposed
aurora_requests_total (Counter)
aurora_request_duration_seconds (Histogram)
aurora_exceptions_total (Counter)
project_idlabel extracted from URL for multi-tenancy insightsRelated Issues
N/A
Screenshots (if applicable)
Example metrics output:
aurora_requests_total{status_code="200",method="post",route="compute/listImages",endpoint_type="trpc",project_id="abc-123"} 245
aurora_request_duration_seconds_sum{status_code="200",method="post",route="compute/listImages",endpoint_type="trpc",project_id="abc-123"} 1.234
Testing Instructions
pnpm i- Install dependencies (including prom-client)pnpm build- Build the aurora packagecurl http://localhost:3000/metrics- Verify metrics endpoint returns Prometheus formatManual Testing
service/actionformatvite-dev/projects/{id}/...URLsChecklist
Documentation
packages/aurora/docs/0013_prometheus_metrics.mdwith implementation detailsSummary by CodeRabbit
New Features
GET /metricsendpoint for Prometheus scrapingproject_idwhere available (with/metricsexcluded from counting)Documentation
Tests
Chores
prom-clientdependencymetrics