Skip to content

fix(observability): bound HTTP metrics route labels#581

Merged
mikewheeleer merged 1 commit into
Talenttrust:mainfrom
Dreamstore2046:codex/talenttrust-metrics-route-cardinality
Jun 29, 2026
Merged

fix(observability): bound HTTP metrics route labels#581
mikewheeleer merged 1 commit into
Talenttrust:mainfrom
Dreamstore2046:codex/talenttrust-metrics-route-cardinality

Conversation

@Dreamstore2046

Copy link
Copy Markdown
Contributor

Summary

  • use Express mount path plus matched route templates for HTTP metrics route labels
  • collapse unmatched requests into a single unmatched bucket
  • add HTTP_METRICS_ROUTE_LABEL_LIMIT and route excess new templates to other
  • document the cardinality guard in README and backend observability/env docs

Security notes

The route label no longer uses concrete request paths. User-controlled identifiers stay out of Prometheus labels, and newly observed templates after the configured cap are attributed to other so route-label cardinality cannot grow without bound.

Validation

  • npm test -- --runInBand src/observability/metrics-service.test.ts: PASS, 9 tests
  • $env:JWT_SECRET='abcdefghijklmnopqrstuvwxyz123456'; npm test -- --runInBand src/config/environment.test.ts: PASS, 35 tests
  • npx eslint src/observability/metrics-service.ts src/observability/metrics-service.test.ts src/config/env.schema.ts src/config/environment.test.ts src/app.ts: PASS
  • npx tsc --noEmit --target ES2022 --module commonjs --strict --esModuleInterop --skipLibCheck --moduleResolution node --types node,jest src/observability/metrics-service.ts src/observability/metrics-service.test.ts src/config/env.schema.ts src/config/environment.test.ts: PASS
  • npm test -- --runInBand: FAIL on existing repository baseline issues outside this change, including src/utils/swrCache.test.ts parse/type errors, src/db/betterSqlite3.ts duplicate identifiers, src/health.ts ProbeSnapshot typing, and webhookDelivery timeout tests.

Closes #558

@Dreamstore2046

Copy link
Copy Markdown
Contributor Author

CI boundary update after the first run:

  • Security Audit: passed.
  • OpenAPI Spec Validation: passed.
  • Lint: failed on existing repository parse errors in src/utils/swrCache.ts and src/utils/swrCache.test.ts. The focused ESLint command for the files touched in this PR passed locally.
  • Test: failed on existing repository baseline issues outside this PR. The CI summary still shows src/observability/metrics-service.test.ts and src/config/environment.test.ts passing; local focused runs passed 9/9 and 35/35 respectively.

I kept this PR scoped to #558 so it does not mix the route-label cardinality fix with unrelated repository baseline repairs.

@mikewheeleer

Copy link
Copy Markdown
Contributor

great stuff — in it goes 👍

@mikewheeleer
mikewheeleer merged commit 3955645 into Talenttrust:main Jun 29, 2026
3 of 5 checks passed
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.

Bound HTTP metrics route-label cardinality to prevent a metric-explosion DoS

2 participants