fix(model_gateway): bound HTTP metrics path label to matched route

[slin1237]

Description

Problem

model_gateway records HTTP layer-1 metrics (smg_http_requests_total, smg_http_request_duration_seconds) with a path label derived from the raw request URI via normalize_path_for_metrics. That helper only replaces dynamic segments at path index > 2, so parameterized routes whose id sits at index ≤ 2 — /v1/responses/{response_id}, /workers/{worker_id}, /v1/conversations/{conversation_id}, /v1/tokenizers/{tokenizer_id}, etc. — pass the raw id through unchanged. The label is then interned into the process-global, never-evicted STRING_INTERNER and emitted as a distinct Prometheus time series. An attacker hitting such a route with many distinct ids drives unbounded metric cardinality and unbounded interner memory growth — a remote DoS.

(Genuinely unmatched paths hit sink_handler, which server.rs registers via .fallback(...) after the metrics/logging .layer(...) calls, so they do not flow through these layers today. The reachable vector is matched parameterized routes.)

Solution

Label HTTP metrics by axum's MatchedPath route template, which is bounded to the registered route table, and map a missing MatchedPath to a fixed "other". A new matched_path_label(&http::Extensions) -> &str helper reads the template from request extensions; both call sites (the HttpMetricsLayer service and the RequestLogger tracing hook) use it. MatchedPath is populated by axum's router before the per-endpoint layer stack runs, so the .layer()-applied middleware (registered on the merged router in server.rs) observes it for matched routes — verified against axum 0.8.9 and by test. The obsolete normalize_path_for_metrics/is_dynamic_id helpers and tests are removed.

Operational note: the path label value changes from normalized request paths to route templates (e.g. /v1/responses/{response_id} instead of /v1/responses/resp_abc → {id}). Dashboards/alerts keyed on the old normalized values should be updated.

Changes

• model_gateway/src/middleware/metrics.rs: replace normalize_path_for_metrics/is_dynamic_id with matched_path_label (returns the MatchedPath template or "other"); use it in the layer; update the module doc.
• model_gateway/src/middleware/logging.rs: RequestLogger::on_request labels via matched_path_label(request.extensions()).
• Tests: matched dynamic route → template label; unmatched → "other"; 1000 distinct ids through the real HttpMetricsLayer do not grow the interner.

Test Plan

• matched_route_uses_template_label — /v1/responses/resp_abc123 against route /v1/responses/{response_id} is observed at a Router::layer-applied middleware as /v1/responses/{response_id} (proves MatchedPath is available at that layer).
• unmatched_path_collapses_to_other — /totally/unregistered/aaaa → "other".
• distinct_ids_on_matched_route_do_not_grow_interner — drives the real HttpMetricsLayer, sends 1000 distinct /v1/responses/resp_{i} requests, asserts global interner growth < 100. Confirmed to FAIL before the fix (reverting the label to req.uri().path() reports "interner grew by 1000 for 1000 distinct request ids") and pass after.
• matched_path_label_defaults_to_other_when_absent — helper returns "other" for empty extensions.

Authoritative gate (sccache disabled, RUSTC_WRAPPER=""):

$ rustup run nightly cargo fmt --all -- --check
FMT CLEAN (exit 0)

$ cargo clippy --workspace --all-targets --all-features -- -D warnings
Finished `dev` profile target(s) in 11.87s
exit 0

$ cargo test -p smg --lib metrics
test result: ok. 50 passed; 0 failed; 997 filtered out — incl. matched_route_uses_template_label, unmatched_path_collapses_to_other, distinct_ids_on_matched_route_do_not_grow_interner

Checklist

• cargo +nightly fmt passes
• cargo clippy --all-targets --all-features -- -D warnings passes
• (Optional) Documentation updated
• (Optional) Please join us on Slack #sig-smg to discuss, review, and merge PRs

Summary by CodeRabbit

• Bug Fixes

  • Improved HTTP request metrics to use route templates for path labeling instead of raw paths, preventing cardinality issues from dynamic path parameters.

• Refactor

  • Simplified HTTP metrics path-labeling logic by adopting route-template-based labeling with improved fallback handling.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: e736e810-1c9a-43b8-b4a3-a26db25c2108

📥 Commits

Reviewing files that changed from the base of the PR and between c05f398 and 278aa2a.

📒 Files selected for processing (2)

• model_gateway/src/middleware/logging.rs
• model_gateway/src/middleware/metrics.rs

---

📝 Walkthrough

Walkthrough

This PR switches HTTP metrics path labeling from normalizing raw request paths to using Axum matched route templates. A new matched_path_label helper extracts template strings from request extensions, defaulting to "other" when unmatched, bounding metric cardinality. Both metrics middleware and logging handler integrate this helper.

Changes

Path label refactoring

Layer / File(s)	Summary
Matched path label helper and metrics documentation model_gateway/src/middleware/metrics.rs	New matched_path_label() helper returns Axum MatchedPath template when present, else "other", to bound metric labels. Metrics documentation and imports updated to reflect template-based labeling via MatchedPath and Request from axum::extract.
Middleware and logging integration model_gateway/src/middleware/metrics.rs, model_gateway/src/middleware/logging.rs	HttpMetricsMiddleware and OnRequest handler compute path labels via matched_path_label(req.extensions()) instead of normalizing raw paths. Both modules import the new helper and pass template-based labels to metrics recording.
Test suite for matched path label behavior model_gateway/src/middleware/metrics.rs	Tests validate matched_path_label behavior for absent MatchedPath, matched dynamic route templates, unmatched path collapsing to "other", and verify that template labels prevent unbounded metrics interner growth across distinct request IDs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• lightseekorg/smg#1328: Both PRs update HTTP metrics emission to include bounded/stable path labels (retrieved PR adds the path dimension to metrics, main PR switches to Axum matched-route-template-derived labels).

Suggested labels

tests, model-gateway

Suggested reviewers

• CatherineSue
• key4ng

Poem

🐰 Hops through metrics with glee,
Route templates bound the cardinality,
No more paths gone wild,
Matched routes keep it mild,
Labels stable as can be! 🎯

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(model_gateway): bound HTTP metrics path label to matched route' accurately and concisely describes the main change: fixing an unbounded metric cardinality issue by binding the HTTP metrics path label to the matched route template.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/metrics-path-cardinality

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[gemini-code-assist]

Code Review

This pull request replaces the custom path normalization logic with Axum's MatchedPath extension to bound metric cardinality and prevent unbounded label growth. Feedback suggests optimizing performance on the hot path by using intern_string instead of .to_owned() on the matched path label to avoid allocating a new String on every HTTP request.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

To avoid allocating a new String on every single HTTP request, we can leverage the existing intern_string helper to obtain a cheaply cloneable Arc<str> instead of calling .to_owned(). Since the matched path templates are bounded and server-controlled, they are safe to intern.

Suggested change

	let path = matched_path_label(req.extensions()).to_owned();
	let path = crate::observability::metrics::intern_string(matched_path_label(req.extensions()));

References

1. For types that are frequently cloned on hot paths and represent a small, repeated set of values, use an interned string type like Arc<str> to improve performance by making clones cheap.

[claude]

Clean fix. matched_path_label correctly reads the MatchedPath extension (populated by axum's router before Router::layer()-applied middleware runs) and falls back to "other" for unmatched paths. The .to_owned() in the call method is necessary since req is moved into the async block. Tests cover matched, unmatched, and interner-growth scenarios through the real layer stack. No issues found — 0 Important, 0 Nit, 0 Pre-existing.