feat(core): emit low cardinality request handler span names - #23614
Open
isaacs wants to merge 2 commits into
Open
feat(core): emit low cardinality request handler span names#23614isaacs wants to merge 2 commits into
isaacs wants to merge 2 commits into
Conversation
Member
Author
|
bugbot run |
isaacs
changed the title
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 3bf00a6. Configure here.
Contributor
size-limit report 📦
|
isaacs
force-pushed
the
isaacs/low-cardinality-request-handler-span-names
branch
2 times, most recently
from
August 26, 2026 03:20
3293c93 to
7673348
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7673348. Configure here.
isaacs
force-pushed
the
isaacs/low-cardinality-request-handler-span-names
branch
from
August 26, 2026 15:09
277e858 to
9b21ab4
Compare
isaacs
marked this pull request as ready for review
August 26, 2026 15:11
isaacs
requested review from
JPeer264,
andreiborza,
chargome and
s1gr1d
and removed request for
a team
August 26, 2026 15:11
Name `handler` spans after the route they serve when span streaming is enabled, or `Request handler` if no route set. Static mode left as is. Drop Hapi method, as the template dictates. NestJS resolves no route when the span starts. The NestJS callback name stays on `nestjs.callback`. Elysia sets `context.route` when the request enters the compiled handler, which is before the `Handle` phase reports. Read it in the trace listener so streamed handler spans carry the route instead of the `Request handler` fallback. The fallback now applies only when the context has no route. Set `code.function.name` only on the child spans this renames, and only when the handler has a name. Static mode keeps the handler name in the span name, so the attribute adds nothing there, and an anonymous handler has no name to record. Register the Fastify test route from a plugin. Fastify installs the SDK's `onRoute` hook when it flushes its plugin list, which is after root-level routes are in place. A root-level route therefore produces no route handler span, and the test never reached that code path. Also: correct `REQUEST_HANDLER_SPAN_NAME_FALLBACK`: the conventions spell the fallback `Request handler`, and its `@see` link pointed at the resource section. closes #23533 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
isaacs
force-pushed
the
isaacs/low-cardinality-request-handler-span-names
branch
from
August 26, 2026 15:12
9b21ab4 to
667c4bd
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
15
to
22
| getIsolationScope, | ||
| getRootSpan, | ||
| getTraceData, | ||
| hasSpanStreamingEnabled, | ||
| REQUEST_HANDLER_SPAN_NAME_FALLBACK, | ||
| SEMANTIC_ATTRIBUTE_SENTRY_OP, | ||
| SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, | ||
| setHttpStatus, |
Contributor
There was a problem hiding this comment.
Bug: The function hasSpanStreamingEnabled is imported twice from @sentry/core in the same import statement, which is a syntax error and will prevent the module from loading.
Severity: CRITICAL
Suggested Fix
Remove one of the two duplicate hasSpanStreamingEnabled imports from the destructured import list in packages/elysia/src/withElysia.ts to resolve the syntax error.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/elysia/src/withElysia.ts#L15-L22
Potential issue: The `import` statement from `@sentry/core` in
`packages/elysia/src/withElysia.ts` includes the named import `hasSpanStreamingEnabled`
twice. In JavaScript and TypeScript, duplicate bindings within a single `import`
statement are a syntax error. This will cause a `SyntaxError` when the module is parsed,
preventing the file from loading and breaking the entire `@sentry/elysia` integration
for all users. The application will fail to start if it uses this package.
Did we get this right? 👍 / 👎 to inform future reviews.
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.
Name
handlerspans after the route they serve when span streaming is enabled, orRequest handlerif no route set. Static mode left as is.Drop Hapi method, as the template dictates. NestJS resolves no route when the span starts. The NestJS callback name stays on
nestjs.callback.Elysia sets
context.routewhen the request enters the compiled handler, which is before theHandlephase reports. Read it in the trace listener so streamed handler spans carry the route instead of theRequest handlerfallback. The fallback now applies only when the context has no route.Set
code.function.nameonly on the child spans this renames, and only when the handler has a name. Static mode keeps the handler name in the span name, so the attribute adds nothing there, and an anonymous handler has no name to record.Register the Fastify test route from a plugin. Fastify installs the SDK's
onRoutehook when it flushes its plugin list, which is after root-level routes are in place. A root-level route therefore produces no route handler span, and the test never reached that code path.Also: correct
REQUEST_HANDLER_SPAN_NAME_FALLBACK: the conventions spell the fallbackRequest handler, and its@seelink pointed at the resource section.closes #23533