Skip to content

fix(hono-server): emit http_requests_total from the transport so every inbound mount is counted (#9650) - #9746

Merged
huangyiirene merged 9 commits into
mainfrom
claude/issue-9650-http-metrics-auth-and-rest-coverage
Aug 19, 2026
Merged

fix(hono-server): emit http_requests_total from the transport so every inbound mount is counted (#9650)#9746
huangyiirene merged 9 commits into
mainfrom
claude/issue-9650-http-metrics-auth-and-rest-coverage

Conversation

@os-zhuang

@os-zhuangos-zhuang commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Part of #9650. This PR carries the fix, replacing the measurement-only scope it opened with. It supersedes this PR's earlier "measurement only, no fix" framing: the design fork that made the card expensive was ruled on 2026-08-18, and what remained was the seam plus its pins. Everything below is measured on 8957c72227 (origin/main merged at 9ff11921a2).

⚠️ It does not discharge the card on its own — see "What still blocks the card" at the end. The closing keyword is deliberately withheld.

The ruling this implements

Emit http_requests_total from the Hono adapter itself, as a raw-app middleware installed at the end of HonoServerPlugin.init() next to installMiddlewareSeam() — the layer where every inbound request on that transport converges, including all getRawApp() mounts. Route label = PATTERN (c.req.routePath), never the concrete path.

Why the transport, and not one layer up

The counter had exactly one emitter: a Proxy the dispatcher built over its ownIHttpServer handle. Measured on this branch, that left at least 14 inbound surfaces uncounted in two structurally different classes — and the first class is unreachable by construction:

  • getRawApp() mounts (11): auth, metadata HMR, cloud-connection, marketplace proxy and install-local, runtime-config, trigger-api, webhooks, approvals, the console SPA, the unknown-hostname guard. These never touch IHttpServer, so no wrapper at that level can ever see them.
  • http.server consumers mounting through verb methods (7): the REST data API via RouteManager, storage, i18n, settings, datasource admin, the dispatcher itself, and the non-Hono conformance adapter.

The change

filewhat
packages/plugins/plugin-hono-server/src/adapter.tsnew installHttpMetricsSeam(metrics) — idempotent, mounts one app.use('*') that emits http_requests_total{method,route,status} after next()
packages/plugins/plugin-hono-server/src/hono-plugin.tsnew observability.metrics option; resolves the canonical chain and installs the seam immediately beforeinstallMiddlewareSeam()
packages/runtime/src/http-metrics-inbound-coverage.hono.integration.test.tsthe two "does NOT count" assertions flipped; new section 4 pins the seam's edges

Route label is the matched pattern.routePath(c) from hono/route — the non-deprecated spelling of the ruled c.req.routePath, returning the same value with a safe fallback instead of an index throw. Measured: two requests to /api/v1/data/abc123 and /api/v1/data/def456 produce one series, /api/v1/data/:id; the auth wildcard is labelled /api/v1/auth/*, not /api/v1/auth/sign-in/email.

Order is load-bearing, in both directions. Before the middleware seam, so a request the use() chain short-circuits — the inbound rate limiter's 429 — is still counted; a refused request is exactly the one an operator alerts on. Still at the end of init(), because every route in the platform mounts in some plugin's start() and Hono composes matched handlers in registration order.

Resolution chain, the canonical one from ObservabilityServicePlugin: explicit option, then the observability:metrics service, then nothing at all — no backend configured means no middleware installed, so an unconfigured deployment pays no per-request cost.

Documented expectations, stated rather than left to be discovered

  • A transport that does not implement this seam reports no HTTP metrics. The seam is Hono's; packages/qa/http-conformance deliberately omits getRawApp() and has no equivalent, so it emits nothing. A zero there means "not instrumented", never "no traffic". The transport-agnostic successor is filed as spec: give IHttpServer a response-observing hook so HTTP metrics are transport-agnostic instead of Hono-only #9835, per the ruling.
  • A preflight OPTIONS the transport's own CORS built-in answers is not counted — it short-circuits before this seam and never reaches a route. Pinned as a case.

The acceptance criterion, and how it was proved

The two assertions that read "does NOT count" now read "counts", and they flip together — a seam reaching one mount and not the other would reproduce the card one surface over. The positive control (the dispatcher's own route IS counted) stays: without it, "now counted" would be indistinguishable from a metrics injection that counts everything.

The harness is unchanged where it matters: a real Hono adapter booted in the shipped serve.ts plugin order, the realRouteManager from @objectstack/rest, and the exact getRawApp().all(basePath + '/*') construct from auth-plugin.ts:1622-1630.

Ablation. Predictions written to a file before the run: delete the call block, expect RED, and specifically 7 of 18 red with the seven named. Both legs rebuilt, and scripts/ablation-dist-preflight.mjs proved the artifact the suite consumes actually changed — marker absent after ablation, present after restore.

legdist preflightresult
ablated✓ marker absent from all 6 built filesTests 7 failed | 11 passed (18) — exactly the seven predicted
restored✓ marker present in 2 built filesTests 18 passed (18)

The CORS case failed on its positive leg (expected [] to include 'POST') while not.toContain('OPTIONS') passed vacuously against an empty array — which is why that positive leg is there.

Verification, gate union re-run at 8957c72227

Derived with node scripts/pm/dispatch-gates.mjs from the actual changed paths (11 path-matched + 5 convention-triggered), plus pnpm lint and check:slot-lookup, which that derivation still does not name.

gateresult
pnpm --filter @objectstack/runtime exec vitest run (the pin)Tests 18 passed (18)
pnpm --filter @objectstack/plugin-hono-server testTest Files 18 passed, Tests 211 passed (211)
pnpm --filter @objectstack/hono testTests 73 passed (73)
pnpm --filter @objectstack/plugin-hono-server typecheckclean
pnpm lint (eslint . --no-inline-config)exit 0, empty output
check:slot-lookupOK, 107 unswept sites, none new, baseline verified
check:route-envelopeOK — and see the cross-card note below
check:type-check-debt (--re-measure, built closure)OK, 33 entries in 380.8s, 1926 raw errors, none above its recorded number, surplus: none
check:nul-bytesOK, 6273 files, no raw control bytes
check:engine-double-contractOK, 321 pinned
check:where-matcherOK, 258 matchers, none new
check:query-options-erasureOK, ratchet holds, none new
check:type-check-coverageOK, 64/77 packages
check:cross-package-test-inputs / check:test-source-alias / check:type-source-resolutionOK
check:changeset-gate-self-tests, check-empty-changeset, check-adr-0087-registration, check-changeset-no-major, check:objectui-changesetOK, 1 non-breaking changeset seen
scripts/docs-audit/check-affected-docs.mjsOK, 242 self-test cases

Full-workspace build before the ratchet: Tasks: 70 successful, 70 total.

The phantom green caught a real defect

pnpm --filter @objectstack/runtime typecheck is green and does not read the harness file — that tsconfig excludes **/*.test.ts. check:type-check-debt did read it and went red at TEST_DEBT 227 → 228: a CORS option spelled origin where HonoCorsOptions declares origins, so the test's CORS config was silently falling back to defaults. Fixed at the call site; re-measured directly by compiling the hidden test layer: 227 total, 0 from this file, matching the frozen ledger exactly. ⛔ The ledger was not raised.

Cross-card note — scripts/check-route-envelope.mjs counters did not move

#9436 owns that file this round. Verified by running the gate twice on the same tree, once with adapter.ts reverted to the merge base: identical output both times (161 hand-built bodies; current-user-endpoints.ts closed at unenveloped 9). This PR moves no counter there. ⛔ Neither that script nor eslint.config.mjs's UNCONTRACTED_SLOTS nor slot-lookup-baseline.json was touched.

What still blocks the card, and why the closing keyword is withheld

The seam is correct and inert in a shipped objectstack serve deployment: serve.ts registers no ObservabilityServicePlugin anywhere in the repo and builds new HonoServerPlugin({ port }) with no observability, so neither link of the resolution chain exists and no middleware is installed. Filed as sub-issues of #9650:

Also filed, as the ruling directed: #9835, the IHttpServer response-observing contract hook.


Generated by Claude Code

… both mounts (#9650)
Measurement harness only - no fix. Pins what the counter observes on the
current wiring and what each of the two pre-declared candidate seams covers.
Co-Authored-By: Claude <noreply@anthropic.com>
…te seams (#9650)
Measurement only, no fix: the seam choice is a design fork and is being
escalated rather than picked.
- section 1 pins what http_requests_total observes on the current wiring:
the dispatcher's own routes yes, the auth getRawApp() mount no, the REST
data route mounted through the real RouteManager no.
- section 2 measures seam A (register the instrumented proxy back as the
http.server service): getRawApp passes through unwrapped, only
get/post/delete are trapped, registerService refuses a second
registration, and a consumer resolving in an earlier start() keeps the
raw handle.
- section 3 measures the raw-app middleware seam at three install points
and shows the framework-agnostic use() seam cannot observe status.
Co-Authored-By: Claude <noreply@anthropic.com>
@os-zhuangos-zhuang added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 18, 2026 — with Claude
@github-actions

github-actionsBot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-hono-server, touching 12 documentable anchor(s).

8 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx(via auth.login (sdk))
  • content/docs/api/index.mdx(via /api/v1/auth/sign-in/email (route))
  • content/docs/deployment/cli.mdx(via /api/v1/* (route), /api/v1/auth/* (route))
  • content/docs/deployment/production-readiness.mdx(via MetricsRegistry (symbol), http_requests_total (literal))
  • content/docs/deployment/self-hosting.mdx(via /api/v1/auth/* (route))
  • content/docs/getting-started/your-first-project.mdx(via /api/v1/auth/sign-in/email (route))
  • content/docs/permissions/authentication.mdx(via HonoServerPlugin (symbol), auth.login (sdk), /api/v1/auth/* (route), /api/v1/auth/sign-in/email (route))
  • content/docs/protocol/kernel/index.mdx(via HonoServerPlugin (symbol))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17.mdx(via HonoServerPlugin (symbol))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: /api/v1/:object/:id (route, 61 pages)
  • 1 name(s) were too generic to anchor anything (single lowercase words)

Coarse fallback — 7 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 9ff11921a26f44a62ca834651adaa54105468536packageMentionDocs.

Which tree this was computed on

This run read content/docs from a8e6e2d54383df7891b69b74e7e4539d337cc1ed — the merge of head 8957c72227868200a75fd3ea44a77176e353b7c7 into base 9ff11921a26f44a62ca834651adaa54105468536, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin a8e6e2d54383df7891b69b74e7e4539d337cc1ed && git checkout a8e6e2d54383df7891b69b74e7e4539d337cc1ed
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9ff11921a26f44a62ca834651adaa54105468536 8957c72227868200a75fd3ea44a77176e353b7c7 && git checkout -B drift-repro 9ff11921a26f44a62ca834651adaa54105468536 && git merge --no-ff 8957c72227868200a75fd3ea44a77176e353b7c7
node scripts/docs-audit/affected-docs.mjs --json 9ff11921a26f44a62ca834651adaa54105468536

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 9ff11921a26f44a62ca834651adaa54105468536 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

The service-lookup slots carried `getService<any>` purely so a stub could
expose an `id` marker. Typed to IHttpServer (core-service-contracts.ts:155)
and switched to identity comparison instead, which needs no marker member
and is the stronger assertion: a Proxy is never === its target.
No baseline and no eslint config were touched.
Co-Authored-By: Claude <noreply@anthropic.com>
…y inbound mount is counted (#9650)
The counter had one emitter — a Proxy the runtime dispatcher built over its
own IHttpServer handle — so it saw only the routes the dispatcher registered.
Auth (getRawApp) and the REST data API (RouteManager), the two
highest-traffic inbound surfaces, were outside it while the docs told
operators to alert on exactly that counter.
Emit it from the Hono adapter instead, as a raw-app middleware installed at
the end of HonoServerPlugin.init() beside installMiddlewareSeam() — the one
layer every inbound request converges on. Route label is the matched PATTERN,
never the concrete path.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WeN7F6jQFpcqW2BN56RdPa
@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tooling and removed size/m labels Aug 19, 2026
…gins, not origin)
Caught by check:type-check-debt, not by `pnpm --filter @objectstack/runtime
typecheck` — that program excludes **/*.test.ts, so the file it would have
flagged is not in it. TEST_DEBT is back at its frozen 227, 0 from this file.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WeN7F6jQFpcqW2BN56RdPa
@huangyiirenehuangyiirene changed the title test(runtime): measure http_requests_total inbound coverage and both candidate seams (#9650)fix(hono-server): emit http_requests_total from the transport so every inbound mount is counted (#9650)Aug 19, 2026
@huangyiirenehuangyiirene removed the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Aug 19, 2026 — with Claude
@huangyiirene
huangyiirene marked this pull request as ready for review August 19, 2026 03:38
@huangyiirene
huangyiirene added this pull request to the merge queueAug 19, 2026
Merged via the queue into main with commit 152bff8Aug 19, 2026
31 of 32 checks passed
@huangyiirene
huangyiirene deleted the claude/issue-9650-http-metrics-auth-and-rest-coverage branch August 19, 2026 03:52
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@os-zhuang@huangyiirene@claude