fix: use endpoint templates in metrics labels#115
Conversation
Prevents high-cardinality metric labels in Prometheus by mapping request paths to endpoint templates (e.g., /docs/:slug, /apis/:slug). API routes keep their exact path (/api/page, /api/search). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a toEndpoint(pathname) helper and routing maps to normalize URL pathnames into stable endpoint templates; updates SSR render and HTTP request metrics to use the normalized endpoint label; includes tests for mapping behavior and a CI env tweak for the dev-server smoke test on Windows. ChangesRoute Normalization for Telemetry
CI Workflow
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/chronicle/src/server/plugins/telemetry.ts`:
- Line 75: The code uses new URL(event.req.url).pathname which can throw for
relative URLs; replace that with a safe extraction like using event.path if
available or falling back to event.req.url?.split('?')[0] before calling
toEndpoint. Update the endpoint assignment in telemetry.ts (the const endpoint =
... line) to derive a safePath = event.path || event.req.url?.split('?')[0] (or
an empty string fallback) and then call toEndpoint(safePath) so toEndpoint and
endpoint are never fed an invalid URL.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bae0bef3-66c9-4d6c-ae32-f96abb005d8a
📒 Files selected for processing (1)
packages/chronicle/src/server/plugins/telemetry.ts
| const route = new URL(event.req.url).pathname | ||
| requestCounter.add(1, { method, route, status: res.status }) | ||
| requestDuration.record(duration, { method, route, status: res.status }) | ||
| const endpoint = toEndpoint(new URL(event.req.url).pathname) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for event.req.url usage patterns in the codebase to determine format
# Search for existing usage of event.req.url
rg -n -C3 'event\.req\.url' --type=ts
# Search for URL parsing patterns
rg -n -C3 'new URL.*event\.req' --type=tsRepository: raystack/chronicle
Length of output: 2143
🌐 Web query:
Nitro h3 event.req.url format full URL or relative path
💡 Result:
Nitro/H3: event.req.url is a relative URL/path component (not a full absolute URL). In Nitro, you typically access the request URL parts via: - event.url.pathname for the matched pathname (example middleware/routing uses event.url.pathname) [1] - event.req.url is then used as the path-ish value when Nitro/H3 creates the H3 event (and is consumed as such by proxy/middleware expectations); in an H3 bug report, overwriting node req.url with a decoded path is explicitly called out as wrong because it’s “consumed by HTTP proxies and Node.js middleware that expect the percent-encoded form” [2]. That implies req.url holds the (encoded) path/relative portion, not an origin-based full URL. If you need a full absolute URL, Nitro provides getRequestURL(event) (discussed in Nitro issues about origin/hostname extraction) rather than relying on event.req.url [3]. So, the answer to “Nitro h3 event.req.url format full URL or relative path” is: relative path (path + query), not an absolute URL with scheme/host.
Citations:
- 1: https://github.com/nitrojs/nitro/blob/HEAD/src/runtime/internal/app.ts
- 2: h3 v1.15.7+ overwrites req.url with decoded path, breaking proxied requests with percent-encoded UTF-8 h3js/h3#1354
- 3: getRequestURL in a server plugin, middleware, etc. does not consistently return the actual origin or hostname of the deployed site/worker nitrojs/nitro#2388
Fix unsafe new URL(event.req.url) usage in telemetry (can throw).
In Nitro/H3, event.req.url is typically a relative path (path + query), so new URL(event.req.url).pathname can throw TypeError: Invalid URL and break request metrics. This matches the repo’s existing pattern using event.path || event.req.url?.split('?')[0] in packages/chronicle/src/server/routes/[...slug].md.ts.
🔧 Proposed fix
- const endpoint = toEndpoint(new URL(event.req.url).pathname)
+ const pathname = event.path || event.req.url?.split('?')[0] || '/'
+ const endpoint = toEndpoint(pathname)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const endpoint = toEndpoint(new URL(event.req.url).pathname) | |
| const pathname = event.path || event.req.url?.split('?')[0] || '/' | |
| const endpoint = toEndpoint(pathname) |
🤖 Prompt for 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.
In `@packages/chronicle/src/server/plugins/telemetry.ts` at line 75, The code uses
new URL(event.req.url).pathname which can throw for relative URLs; replace that
with a safe extraction like using event.path if available or falling back to
event.req.url?.split('?')[0] before calling toEndpoint. Update the endpoint
assignment in telemetry.ts (the const endpoint = ... line) to derive a safePath
= event.path || event.req.url?.split('?')[0] (or an empty string fallback) and
then call toEndpoint(safePath) so toEndpoint and endpoint are never fed an
invalid URL.
node-process runner uses fork() + IPC which is flaky on Windows CI runners — child process sometimes never sends ready signal, causing health check to hang until job timeout. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 participants
Summary
/api/*) keep exact path since they're fixed endpoints/docs/guides/installation/docs/:slug/apis/petstore/listPets/apis/:slug/api/page/api/page/api/search/api/search/assets/chunk-abc.js/assets/:file/_content/docs/intro.mdx/_content/:path/llms.txt/llms.txtTest plan
🤖 Generated with Claude Code