rest: route-label the direct mux.Handle registrations (#166)#170
Merged
Conversation
The {ticket_id}/{sub} dispatcher (ticketSubResource) applied requireScope
and cacheControl explicitly but not handle()'s third wrap, routeLabel —
GET /api/v1/tickets/{ticket_id}/messages metered under route="", the
label documented as "rejected before routing ever happened". Wrap the
registration in routeLabel OUTSIDE the dispatcher (handle()'s order), so
the messages 200s, the wrong-scope 403s, and the unknown-sub 404s all
meter under the registration pattern.
The ref/messages parity shim shared the gap: its frozen 400 is a routed
response and metered under route="" too. Same one-line wrap — without it
the restored invariant ("" means the request never reached routing,
nothing else) would still be false and the updated comments would lie.
Its scope/cacheControl exemptions are ruled and unchanged.
Both gap-documenting comments (the registration-site note, the
metricLabels.route field doc) now state the invariant instead of the gap.
Four new metrics pins: messages 200 / scope 403 / unknown-sub 404 under
the sub-resource pattern, ref/messages 400 under its literal pattern.
Closes #166
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- metrics_test.go: 'the one registration handle() cannot serve' overstated — the ref/messages shim and catch-all are equally outside handle(); reword to 'the scoped-resource registration handle() cannot serve'. - metrics_test.go: 'both direct registrations wrapped' miscounts the three direct mux.Handle sites; now 'both #166 registrations wrapped (the catch-all was already labeled)'. - metrics.go: route=="" enumeration now includes the pre-routing panic path (deferred relabel meters it as route="" status="500"), not only the auth 401/503 tiers. Comments only; zero behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Re-review after #166 found five neighboring paraphrases of the route="" semantics left divergent from the canonical metricLabels doc ("never reached routing: the auth 401/503 tiers, or a pre-routing panic"): - metrics.go Help strings (both counter and histogram): enumeration was incomplete (missing the pre-routing-panic case); now matches the canonical doc's content. No test pins Help text. - redirect.go: the never-"" exclusivity claim ("auth rejected before routing") is false since panics also produce ""; re-anchored both the writeCleanPathRedirect comment and the top-of-file quotation (which quoted the old canonical doc verbatim and pointed at rest.go; the canonical doc lives in metrics.go). redirect.go is outside this branch's prior diff — deliberate scope expansion, same invariant. - rest.go catch-all registration comment: same stale framing. - metrics_test.go: re-anchored the clean-path-307 test's quotation, and pinned the counted set in the ref/messages test ("the other direct mux.Handle registration #166 wrapped" — there are three direct registrations; the catch-all predates #166). Comment/Help-string-only; zero behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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
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.
Closes #166.
The tickets sub-resource registration (
GET /api/v1/tickets/{ticket_id}/{sub}) lacked therouteLabelwraphandle()applies everywhere else, so its traffic metered under Prometheusroute=""— conflated with "rejected before routing", the counters PRD #112 designates as the rate-limiting evidence base. Wrapped it (outside the scope gate, mirroringhandle()'s order so 403s meter under the route) and — judgment call validated in review — the ref/messages parity shim too, since its frozen 400 is a routed response and the invariant is false without it.route=""again means exactly one thing: never routed.route=""paraphrases package-wide aligned on the canonical doc, incl. operator-facing Help stringsTwo review rounds; all findings resolved in-branch. Mechanical invariant guard deferred to a tracked follow-up.
🤖 Generated with Claude Code