feat(web): record HTTP request duration metrics by brendan-kellam · Pull Request #1571 · sourcebot-dev/sourcebot

brendan-kellam · 2026-08-11T22:29:21Z

Follows #1570.

Problem

#1570 gave the web process runtime metrics (heap, GC, event loop lag) but nothing about how long requests take, so per-endpoint latency is still invisible. That gap is concrete: an attempt to build a Better Stack chart of /api/health response time found the data does not exist anywhere in telemetry — not as a span (traces go to Sentry), not as a metric, and Uptime monitor data isn't queryable from a Telemetry dashboard.

/api/health is the useful case. It does almost no work, so its duration is essentially event loop queueing delay — which makes it a direct read on whether the process is stalling, and it is the exact endpoint the liveness probe hits with a 1 second timeout.

Change

Adds an http_request_duration_seconds histogram (labels: method, route, status) populated from Node's built-in HTTP diagnostics channels — http.server.request.start and http.server.response.finish.

Why diagnostics channels. Next.js owns the http.Server instance in a standalone build, so there is no request pipeline to wrap. proxy.ts middleware runs in the edge runtime, where prom-client doesn't work. The channels are a supported, in-process observation point that sees every request without patching or monkey-wrapping anything.

Two details that are load-bearing rather than incidental:

Route labels come from Next's own route table. The raw path can't be a label — repository/file paths are unbounded, the first segment is client-supplied, and /api/[...slug] is a catch-all, so full paths or naive truncation both let scanner traffic grow the series count without limit. Instead, the build's .next/routes-manifest.json (78 static + 18 dynamic routes, each with a matching regex, ordered by the router's own resolution priority) is loaded at startup, and each request path is matched to its route pattern:

/browse/github.com/org/repo/-/blob/src/index.ts  ->  /browse/[...path]
/api/auth/callback/github                         ->  /api/auth/[...nextauth]
/settings/connections/42                          ->  /settings/connections/[id]
/wp-admin                                         ->  /[...slug]        (the route that actually serves it)
/api/not-a-real-route                             ->  /api/[...slug]

Cardinality is bounded by the number of defined routes plus /_next and other, and the label set tracks the app automatically at build time — no hand-maintained list to rot. If the manifest is missing or unreadable (e.g. next dev), everything is labelled other: granularity lost, bound kept — it fails closed.

Metrics scrapes are excluded. The channels are process-wide, so they also fire for the metrics server itself — without a filter on the metrics port, every scrape would record itself and the histogram would measure the observer. Verified in the integration test.

Recording is wrapped in try/catch and logs at debug: a metrics failure must never affect request handling.

Test plan

Follow-up

Once this is deployed and scraped, the /api/health p50/p95/p99 chart is a single histogramQuantile query, and the "does health latency spike during GC" correlation becomes one dashboard rather than an inference across two systems. The dashboard is already created and its charts group by source, so the web process joins them automatically.

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- Added HTTP request duration metrics with route, method, and response status details.
- Metrics are collected automatically using configurable duration buckets.
- Routes are normalized and grouped into bounded categories for consistent reporting.
- Metrics-server scrapes are excluded from request measurements.
- Unmatched or unavailable routes are grouped under a fallback category.
Documentation
- Documented the new HTTP request duration metric in the changelog.

Note

^{Cursor Bugbot is generating a summary for commit d839501. Configure here.}

The web process now reports runtime metrics, but nothing about how long requests actually take, so per-endpoint latency is still invisible. That is the signal needed to see a stall from the outside: /api/health does almost no work, so its duration is essentially event loop queueing delay. Add an http_request_duration_seconds histogram labelled by method, route, and status, populated by subscribing to Node's built-in http.server.request.start and http.server.response.finish diagnostics channels. Next.js owns the server instance in a standalone build, so there is no request pipeline to wrap; the channels observe every request without patching anything. Paths are collapsed to a bounded route label. Repository and file paths are unbounded, so labelling by full path would mint a time series per file viewed. Requests to the metrics port are skipped, since the channels are process-wide and every scrape would otherwise record itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

coderabbitai · 2026-08-11T22:29:32Z

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a717e7a1-5bc9-4482-8553-b83841f0ecaa

📥 Commits

Reviewing files that changed from the base of the PR and between d839501 and 0867a0e.

📒 Files selected for processing (4)

CHANGELOG.md
packages/web/src/httpMetrics.integration.test.ts
packages/web/src/httpMetrics.test.ts
packages/web/src/httpMetrics.ts

Walkthrough

The web server now exports http_request_duration_seconds metrics. Diagnostic channels record completed requests with bounded route labels, method, status, and duration. Runtime registration starts collection, and unit and integration tests validate normalization and aggregation.

Changes

HTTP metrics instrumentation

Layer / File(s)	Summary
HTTP duration metric contract `packages/web/src/promClient.ts`	Defines and registers the `httpRequestDuration` histogram with method, route, and status labels.
Request tracking and route normalization `packages/web/src/httpMetrics.ts`, `packages/web/src/instrumentation.ts`	Loads route manifests, normalizes routes, bounds route labels, excludes metrics-server traffic, records completed request durations, prevents duplicate subscriptions, and starts collection during runtime registration.
Metric behavior validation and release notes `packages/web/src/httpMetrics.test.ts`, `packages/web/src/httpMetrics.integration.test.ts`, `CHANGELOG.md`	Tests route normalization, cardinality limits, request aggregation, status labels, observation counts, histogram bounds, and metrics-port exclusion. Documents the new metric.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Sequence Diagram(s)

sequenceDiagram
  participant NodeRegistration
  participant NodeDiagnosticChannels
  participant httpMetrics
  participant httpRequestDuration
  NodeRegistration->>httpMetrics: Start HTTP metrics collection
  NodeDiagnosticChannels->>httpMetrics: Publish request start and completion messages
  httpMetrics->>httpMetrics: Normalize route and exclude metrics-server traffic
  httpMetrics->>httpRequestDuration: Observe method, route, status, and duration

Possibly related PRs

sourcebot-dev/sourcebot#1570: Adds the web Prometheus registry and metrics-server integration used by this instrumentation.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: adding HTTP request duration metrics for the web server.

✨ 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 brendan/web-http-metrics

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Truncating path depth bounded depth, not breadth. The first path segment is client-supplied and /api/[...slug] is a catch-all, so /wp-admin, /.env, and /api/<anything> each minted a new time series. Scanner traffic could grow the series count without limit, which is exactly what the normalization was supposed to prevent. Match the truncated path against a known-route set and report anything else as `other`, bounding distinct route labels to that set plus one regardless of what is requested. A route missing from the set loses granularity rather than breaking, so it fails closed. Also strengthens the metrics-port exclusion assertion. It checked for the absence of a `/metrics` label, which became vacuous once unknown paths collapse to `other` — it now asserts the total observation count, and fails if the port filter is removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

coderabbitai

Actionable comments posted: 1

🤖 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 `@packages/web/src/httpMetrics.integration.test.ts`:
- Around line 38-42: Update the integration test setup around startHttpMetrics
so process.env.WEB_METRICS_PORT is assigned before the httpMetrics module is
imported, ensuring createEnv(options) reads the ephemeral metrics port during
module initialization. Replace the static import with a deferred import after
the environment assignment and invoke startHttpMetrics from that loaded module.

🪄 Autofix

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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7cb3db6d-2e81-48e4-a016-a97834000cd9

📥 Commits

Reviewing files that changed from the base of the PR and between 84a75e4 and 9e595eb.

📒 Files selected for processing (6)

CHANGELOG.md
packages/web/src/httpMetrics.integration.test.ts
packages/web/src/httpMetrics.test.ts
packages/web/src/httpMetrics.ts
packages/web/src/instrumentation.ts
packages/web/src/promClient.ts

…hardcoded list The hardcoded KNOWN_ROUTES set bounded cardinality but rots: new routes silently degrade to `other` until someone updates the file, and two-segment truncation collapses distinct routes (everything under /api/ee/* became one label). Next's build already emits .next/routes-manifest.json with every defined route and a matching regex, ordered by the router's own resolution priority. Load that at startup: exact-match static routes, then first dynamic regex wins, mirroring how the server actually routes the request. Labels become the route pattern itself (/browse/[...path], /settings/connections/[id]), so cardinality is bounded by the number of defined routes plus /_next and `other`, and the label set tracks the app automatically at build time. Scanner traffic now lands on the catch-all routes that genuinely serve it (/[...slug], /api/[...slug]) rather than a synthetic bucket. If the manifest is missing or unreadable, everything is labelled `other` — granularity lost, bound kept. Verified against the production build's manifest (78 static + 18 dynamic routes) and the running pod: the standalone server chdirs to the app dir, so the cwd-relative manifest path resolves. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

coderabbitai

Actionable comments posted: 1

🤖 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 `@packages/web/src/httpMetrics.ts`:
- Around line 59-63: Update the catch block in initRouteTable to assign
routeTable = undefined before returning false, ensuring initialization failures
discard any previously loaded route table and subsequent requests use the
OTHER_ROUTE fallback.

🪄 Autofix

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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c5daa8f0-6a9f-4a10-b470-2185ac676f2c

📥 Commits

Reviewing files that changed from the base of the PR and between 9e595eb and 829ecb3.

📒 Files selected for processing (3)

packages/web/src/httpMetrics.integration.test.ts
packages/web/src/httpMetrics.test.ts
packages/web/src/httpMetrics.ts

🚧 Files skipped from review as they are similar to previous changes (1)

packages/web/src/httpMetrics.test.ts

cursor

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 0867a0e. Configure here.}

cursor · 2026-08-12T23:36:22Z

+                    method: request.method ?? 'UNKNOWN',
+                    route: normalizeRoute(pathname),
+                    status: response.statusCode,
+                },


Unbounded HTTP method label cardinality

Medium Severity

The method label uses the raw client-supplied request.method with no allowlist. Route labels were carefully bounded via the manifest to stop scanner traffic from exploding series count, but arbitrary methods remain unbounded and multiply every route×status series under hostile or unusual clients.

^{Reviewed by Cursor Bugbot for commit 0867a0e. Configure here.}

This comment has been minimized.

Sign in to view

docs: add changelog entry for HTTP request duration metrics

a0e55b2

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

cursor Bot reviewed Aug 11, 2026

View reviewed changes

Comment thread packages/web/src/httpMetrics.ts

coderabbitai Bot reviewed Aug 11, 2026

View reviewed changes

Comment thread packages/web/src/httpMetrics.integration.test.ts

coderabbitai Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread packages/web/src/httpMetrics.ts

fix(web): extend HTTP latency histogram buckets

d839501

cursor Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread packages/web/src/httpMetrics.ts

brendan-kellam added 3 commits August 12, 2026 16:24

fix(web): label rewritten routes in HTTP metrics

2a94465

fix(web): clear stale HTTP metrics route table

3bfa7a3

Merge branch 'main' into brendan/web-http-metrics

0867a0e

brendan-kellam merged commit 5e13dbb into main Aug 12, 2026
11 of 12 checks passed

brendan-kellam deleted the brendan/web-http-metrics branch August 12, 2026 23:27

github-actions Bot mentioned this pull request Aug 12, 2026

Sourcebot Roadmap 🚀 #459

Open

cursor Bot reviewed Aug 12, 2026

View reviewed changes

Navigation Menu

Uh oh!

feat(web): record HTTP request duration metrics - #1571

feat(web): record HTTP request duration metrics#1571
brendan-kellam merged 8 commits into
mainfrom
brendan/web-http-metrics

brendan-kellam commented Aug 11, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

This comment has been minimized.

coderabbitai Bot commented Aug 11, 2026 •

edited

Loading

Review failed

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot Aug 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

brendan-kellam commented Aug 11, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Change

Test plan

Follow-up

Summary by CodeRabbit

Uh oh!

This comment has been minimized.

coderabbitai Bot commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot Aug 12, 2026

Choose a reason for hiding this comment

Unbounded HTTP method label cardinality

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

brendan-kellam commented Aug 11, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 11, 2026 •

edited

Loading