Repository navigation
docs: Unknown routes are now recorded in HTTP metrics (path="other"), access logs, and request IDs - #72
Closed
github-actions[bot] wants to merge 2 commits into
Closed
docs: Unknown routes are now recorded in HTTP metrics (path="other"), access logs, and request IDs#72github-actions[bot] wants to merge 2 commits into
github-actions[bot] wants to merge 2 commits into
Conversation
Signed-off-by: XinyueZhang369 <zoeyzhang369@gmail.com>
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Signed-off-by: XinyueZhang369 <zoeyzhang369@gmail.com>
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
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.
What this PR does
Are requests to unknown paths visible in SMG's HTTP metrics, access logs, and request-ID correlation, and under what path label?
Why we need it
Source change: smg-project/smg@0a58a6c
Commit 0a58a6c (#2679, landed after the v1.11.0 version bump) added attach_edge_layers in model_gateway/src/server.rs: the sink_handler 404 fallback is now registered before the body-limit, logging (middleware::create_logging_layer), HTTP-metrics (HttpMetricsLayer), RequestIdLayer, and CORS layers, with the test unknown_routes_cross_the_edge_middleware asserting an unknown route gets x-request-id. In current source, model_gateway/src/middleware/metrics.rs matched_path_label maps requests with no MatchedPath to "other" (test unmatched_path_collapses_to_other), and model_gateway/src/middleware/logging.rs RequestLogger/ResponseLogger record smg_http_requests_total and log started/finished lines inside the http_request span for every request crossing the layer. Three current docs claims are now wrong: src/lib/content/reference/metrics.md line 51 says requests that match no route "get a bare 404 from a fallback outside the metrics layer and are not recorded"; src/lib/content/getting-started/logging.md line 142 says "A request to an unknown path gets a bare 404 and is not logged" (and lines 132/465 scope the http_request span and request IDs to requests "that match a route"); src/lib/content/getting-started/monitoring.md line 76 says the http_request span is exported "for each HTTP request that matches a route". This is one independent concern: a single fallback relocation makes unknown routes observable across metrics, logs, and request IDs. The fix is on main only (post-v1.11.0), so the edit should describe main and may note that v1.11.0 and earlier skipped these requests.
Scope: observability / unknown-route-edge-middleware. Other concerns are deferred.
How to test
git diff --checkandpnpm checkand the productionpnpm build.Source-level follow-up validation
The outer CORS layer handles OPTIONS directly; those requests bypass the inner logging, metrics, and request-ID layers. The documentation explicitly retains this exception. The corrected diff passed
git diff --check,pnpm check, andpnpm build.Checklist