feat: Emit low-cardinality http.client span names for node:http and undici - #23690
Merged
Conversation
…ndici
With span streaming, `http.client` spans are named `{method} {url.domain}` instead of
`{method} {sanitized-url}`, falling back to the method alone when there is no domain.
Covers outgoing `node:http`/`https` requests, undici, and `googleCloudHttpIntegration`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
size-limit report 📦
|
The outgoing request span now carries `url.domain`, and the assertion compares `data` exactly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 1c6aa69. Configure here.
chargome
marked this pull request as ready for review
August 27, 2026 16:13
chargome
requested review from
JPeer264 and
isaacs
and removed request for
a team
August 27, 2026 16:13
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
222
to
228
| [HTTP_REQUEST_METHOD]: requestMethod, | ||
| [ATTR_HTTP_REQUEST_METHOD_ORIGINAL]: request.method, | ||
| [URL_FULL]: filterCollectedUrl(requestUrl.toString()), | ||
| [URL_DOMAIN]: requestUrl.hostname || undefined, | ||
| [URL_PATH]: requestUrl.pathname, | ||
| [URL_QUERY]: filterCollectedUrlQuery(getUrlQuery(requestUrl.search)), | ||
| [URL_FRAGMENT]: getUrlFragment(requestUrl.hash), |
Contributor
There was a problem hiding this comment.
Bug: The streaming path for undici/fetch does not strip the internal SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME attribute for data: URLs, leading to a potential high-cardinality transaction name in Sentry.
Severity: LOW
Suggested Fix
Before serializing the span for streaming, delete the SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME attribute from the span's attributes. This would mirror the cleanup logic already present in the static (non-streaming) path's SentrySpan._convertSpanToTransaction() function.
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/node/src/integrations/node-fetch/undici-instrumentation.ts#L222-L228
Potential issue: When span streaming is enabled for `undici`/`fetch` and a `data:` URL
is used, the internal attribute `SEMANTIC_ATTRIBUTE_SENTRY_CUSTOM_SPAN_NAME` is not
removed from the streamed span's attributes. The static (non-streaming) path correctly
deletes this attribute before sending the transaction, but the streaming path does not.
This inconsistency causes the attribute to be leaked, which can result in the Sentry
backend using a high-cardinality name for the transaction instead of the intended
low-cardinality one.
Did we get this right? 👍 / 👎 to inform future reviews.
Lms24
reviewed
Aug 31, 2026
Member
There was a problem hiding this comment.
l: unless I'm missing something, can we still assert on the span name here?
| // domain is kept. Outgoing requests have no route to parameterize. | ||
| const client = getClient(); | ||
| const method = request.method?.toUpperCase(); | ||
| const domain = urlObject && !isURLObjectRelative(urlObject) ? urlObject.hostname : undefined; |
Member
There was a problem hiding this comment.
Related to #23682 (comment), I guess server-side, we can't really get a doman/hostname for relative URLs 🤔 So omitting it for relative URLs is probably fine... wdyt? logaf-lower-than-in-browser tbh 😅
Comment on lines
+119
to
+124
| // With span streaming the span already carries a low-cardinality name, so it must not be | ||
| // renamed back to something containing the URL. | ||
| const client = getClient(); | ||
| if (!client || !hasSpanStreamingEnabled(client)) { | ||
| span.updateName(`${request.method || 'GET'} ${sanitizedUrl}`); | ||
| } |
Member
There was a problem hiding this comment.
q: should we also just set the request method in a new else block when a URL does not start with :data? Or is this already handled on a lower level?
Member
Author
There was a problem hiding this comment.
Already handled in getOutgoingRequestSpanData
…90-feed # Conflicts: # packages/core/src/integrations/http/get-outgoing-span-data.ts
The e2e outgoing-fetch spans dropped their name assertion when the name stopped carrying the URL. Every outgoing span in that app shares one domain, so keep `url.full` for selecting the span and assert the name separately. Cover the no-domain fallback too: a request with no host leaves the URL relative, which a server runtime cannot resolve against a page origin the way a browser can. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XvHhCTmsaPtA8vMDuHXSwd
Member
Author
|
Added more tests! |
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
2 participants
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.
With span streaming,
http.clientspans are named{method} {url.domain}(GET api.example.com)instead of
{method} {sanitized-url}, falling back to the method alone when there is no domain.traceLifecycle: 'static'is unchanged.Covers outgoing
node:http/httpsrequests, undici (globalfetch), andgoogleCloudHttpIntegration. The fetch and XHR side is #23682, which also carries themigration note for both.
Spans also get a
url.domainattribute in both lifecycles, so the value in the name staysfilterable — same as the
resource.*port.Ref #23527