feat: add /metrics endpoint with Prometheus support and configuration options#431
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR implements an opt-in Prometheus-compatible ChangesMetrics Endpoint Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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 unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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.
Inline comments:
In `@libs/observability/src/process-stats/process-stats.collector.ts`:
- Around line 86-91: In the defaultReadFdCount function replace the direct
require('node:fs') usage with the filesystem helpers from `@frontmcp/utils`:
import or call the utils' readdirSync (or equivalent synchronous directory read)
to list '/proc/self/fd' and return its length; keep the existing platform check
(process.platform !== 'linux') and catch behavior unchanged, and ensure types
match the previous usage (i.e., treat the utils API as returning a string[] so
defaultReadFdCount still returns number | undefined). Use the function name
defaultReadFdCount to locate the change.
In `@libs/observability/src/prometheus/render.ts`:
- Around line 91-117: The code groups metric samples into byName using only
entry.name, which lets counters and gauges with the same name merge and produce
a wrong `# TYPE` block; update the grouping logic in the loops that handle
counters and gauges (the blocks that reference METRIC_NAME_REGEX, formatFloat,
sortedLabelString and manipulate byName and group) to detect cross-type
collisions: if byName.get(entry.name) exists with a different group.type, either
skip adding the conflicting sample and record/log the collision or create
separate keys that include the metric type (e.g., `${entry.name}|${type}`) so
counters and gauges are isolated; ensure help propagation (the group.help
assignment logic) still applies only within the same-type group.
In `@libs/sdk/src/metrics/__tests__/metrics.routes.spec.ts`:
- Around line 37-133: The tests mutate process.env['FRONTMCP_METRICS_TOKEN']
without restoring it; capture the original value at suite start (e.g. const
originalMetricsToken = process.env['FRONTMCP_METRICS_TOKEN'] or use a let) and
restore it in an afterEach hook so each test leaves env unchanged; update the
describe block around registerMetricsRoutes to save the value before token-auth
tests run and call process.env['FRONTMCP_METRICS_TOKEN'] = originalMetricsToken
(or delete it if undefined) in afterEach to isolate state changes.
In `@libs/sdk/src/metrics/__tests__/metrics.service.spec.ts`:
- Line 121: Replace the incorrect use of Parameters with ConstructorParameters
when deriving the type of the MetricsService constructor's second argument in
the tests: locate the casts using Parameters<typeof MetricsService>[1] (used
around the mock constructor args for MetricsService) and change them to
ConstructorParameters<typeof MetricsService>[1]; ensure both occurrences (the
one near the first mock and the second near the other mock) are updated so the
test types correctly reflect the class constructor signature.
In `@libs/skills/catalog/frontmcp-observability/references/metrics-endpoint.md`:
- Around line 99-102: The fenced code block containing the Prometheus sample
(the lines including "# TYPE my_cache_hits_total counter" and
"my_cache_hits_total{tier=\"l1\"} 1") lacks a language tag and triggers MD040;
update the opening triple-backtick to include a language tag (e.g., "text") so
the block becomes a typed fence (```text) to satisfy markdownlint and make the
snippet explicit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b2c61afa-f38c-492a-9428-a56621236b19
📒 Files selected for processing (29)
.gitignoredocs/docs.jsondocs/frontmcp/deployment/metrics.mdxlibs/observability/src/index.tslibs/observability/src/process-stats/__tests__/process-stats.collector.spec.tslibs/observability/src/process-stats/index.tslibs/observability/src/process-stats/process-stats.collector.tslibs/observability/src/prometheus/__tests__/render.spec.tslibs/observability/src/prometheus/index.tslibs/observability/src/prometheus/render.tslibs/sdk/src/common/metadata/front-mcp.metadata.tslibs/sdk/src/common/tokens/front-mcp.tokens.tslibs/sdk/src/common/types/options/index.tslibs/sdk/src/common/types/options/metrics/index.tslibs/sdk/src/common/types/options/metrics/interfaces.tslibs/sdk/src/common/types/options/metrics/schema.tslibs/sdk/src/front-mcp/front-mcp.tslibs/sdk/src/metrics/__tests__/metrics.routes.spec.tslibs/sdk/src/metrics/__tests__/metrics.service.spec.tslibs/sdk/src/metrics/index.tslibs/sdk/src/metrics/metrics.errors.tslibs/sdk/src/metrics/metrics.routes.tslibs/sdk/src/metrics/metrics.service.tslibs/sdk/src/server/server.instance.tslibs/skills/__tests__/skills-validation.spec.tslibs/skills/catalog/frontmcp-observability/SKILL.mdlibs/skills/catalog/frontmcp-observability/examples/metrics-endpoint/enable-metrics-endpoint.mdlibs/skills/catalog/frontmcp-observability/references/metrics-endpoint.mdlibs/skills/catalog/skills-manifest.json
Contributor
Performance Test ResultsStatus: ✅ All tests passed Summary
Total: 101 tests across 21 projects 📊 View full report in workflow run Generated at: 2026-05-24T17:09:02.670Z |
…sport configurations
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/sdk/src/metrics/metrics.routes.ts (1)
70-71:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSet
Content-Typeonly on success paths.
Content-Typeis set to metrics format before branching, so the Line 96 JSON 500 path can inherittext/plainon some adapters. Move the content-type assignment into each success branch (or explicitly set JSON content type in error branches).💡 Suggested patch
const result = service.getMetrics(); res.setHeader?.('Cache-Control', 'no-store'); - res.setHeader?.('Content-Type', result.contentType); if ((config.format ?? 'prometheus') === 'json') { try { + res.setHeader?.('Content-Type', result.contentType); res.status(200).json(JSON.parse(result.body)); } catch { // `getMetrics()` builds the JSON via `JSON.stringify`, so this // branch should be unreachable — but if a downstream override // produces malformed JSON we surface a 500 rather than letting // the parse exception escape the route handler. + res.setHeader?.('Content-Type', 'application/json; charset=utf-8'); res.status(500).json({ error: 'internal_error', message: 'Failed to serialise metrics JSON', }); } return; } if (typeof res.send === 'function') { + res.setHeader?.('Content-Type', result.contentType); res.status(200); res.send(result.body); return; } // Prometheus scrape format is `text/plain` — wrapping it in JSON // would silently break every scraper. Surface a 500 instead so the // adapter mismatch is visible. + res.setHeader?.('Content-Type', 'application/json; charset=utf-8'); res.status(500).json({ error: 'internal_error', message: 'Server adapter does not support Prometheus text format. Use `format: "json"` or upgrade the adapter.', });Also applies to: 93-99
🤖 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 `@libs/sdk/src/metrics/metrics.routes.ts` around lines 70 - 71, The Content-Type header is being set unconditionally (res.setHeader?.('Content-Type', result.contentType)) which lets error paths inherit the wrong type; update the metrics handler to set Content-Type only in each success branch that sends a formatted metric (use result.contentType where the response body is written in the success branches) and explicitly set 'application/json' (or appropriate error content type) in the error/500 branches (e.g., the JSON error response at the path that returns 500). Locate the header usage in metrics.routes.ts (the res.setHeader calls and the success branches that use result.contentType) and move or add the header assignments accordingly so errors never inherit the success content type.
🤖 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.
Outside diff comments:
In `@libs/sdk/src/metrics/metrics.routes.ts`:
- Around line 70-71: The Content-Type header is being set unconditionally
(res.setHeader?.('Content-Type', result.contentType)) which lets error paths
inherit the wrong type; update the metrics handler to set Content-Type only in
each success branch that sends a formatted metric (use result.contentType where
the response body is written in the success branches) and explicitly set
'application/json' (or appropriate error content type) in the error/500 branches
(e.g., the JSON error response at the path that returns 500). Locate the header
usage in metrics.routes.ts (the res.setHeader calls and the success branches
that use result.contentType) and move or add the header assignments accordingly
so errors never inherit the success content type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 41cccfc0-0198-452b-967c-656df2ced164
📒 Files selected for processing (13)
libs/observability/src/process-stats/__tests__/process-stats.collector.spec.tslibs/observability/src/process-stats/process-stats.collector.tslibs/observability/src/telemetry/__tests__/telemetry.factory.spec.tslibs/sdk/src/front-mcp/front-mcp.tslibs/sdk/src/metrics/__tests__/metrics.routes.spec.tslibs/sdk/src/metrics/__tests__/metrics.service.spec.tslibs/sdk/src/metrics/index.tslibs/sdk/src/metrics/metrics.routes.tslibs/sdk/src/metrics/metrics.service.tslibs/skills/catalog/frontmcp-observability/references/metrics-endpoint.mdlibs/utils/src/fs/fs.tslibs/utils/src/fs/index.tslibs/utils/src/index.ts
✅ Files skipped from review due to trivial changes (1)
- libs/skills/catalog/frontmcp-observability/references/metrics-endpoint.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
/metricsendpoint for Prometheus metrics scraping (disabled by default; enable viametrics: { enabled: true }configuration).Documentation