Conversation
Member
Author
|
bugbot run |
Contributor
size-limit report 📦
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 701694c. Configure here.
| return { hostname: match[1], port: port <= 65535 ? port : undefined }; | ||
| } | ||
| return { hostname: host || 'localhost', port: undefined }; | ||
| } |
There was a problem hiding this comment.
Host header parser mishandles IPv6
Medium Severity
splitHostHeader splits on the last : plus digits, so an IPv6 Host value such as [::1]:8080 keeps the brackets in server.address, and a missing header becomes localhost. Both values are wrong for server.address on every incoming server span.
Reviewed by Cursor Bugbot for commit 701694c. Configure here.
| // `Host` header land on `server.address`; the header wins when both are set. | ||
| // `url.path`, `url.query` and `http.request.method` come from `attributes` below, which is why | ||
| // the old `http.target` (path plus query) has no separate replacement here. | ||
| [SERVER_ADDRESS]: request.getHeader('host') ?? request.host, |
There was a problem hiding this comment.
Client spans embed port in address
Medium Severity
Outgoing HTTP spans copy the Host header or URL.host into server.address, so the port stays in the address and server.port is never set. Server spans already split those with splitHostHeader. Client traces therefore disagree with the server spans and with the server.* spec.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 701694c. Configure here.
| 'network.local.port': expect.any(Number), | ||
| 'network.peer.address': expect.any(String), | ||
| 'server.port': expect.any(Number), | ||
| 'http.response.status_code': 200, |
There was a problem hiding this comment.
E2E tests remap peer port wrongly
High Severity
These toEqual payloads list server.address twice and map net.peer.port to server.port. The second key wins, so the Host-header address is never asserted, and the extra network.peer.port the SDK still emits makes the strict equality fail. I flagged this because the testing conventions in the review rules require tests to assert the new attributes thoroughly.
Additional Locations (2)
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 701694c. Configure here.
| return ( | ||
| transactionEvent.contexts?.trace?.data?.['http.target'] === `/generation-functions?metadataTitle=${testTitle}` | ||
| ); | ||
| return transactionEvent.contexts?.trace?.data?.['url.path'] === `/generation-functions?metadataTitle=${testTitle}`; |
There was a problem hiding this comment.
Tests match query on url.path
High Severity
waitForTransaction and the Next.js 15 tracesSampler now compare url.path to a string that still includes the query. url.path is pathname-only; http.target used to carry path plus query. Those waiters never match, so the tests time out. I flagged this because the testing conventions in the review rules require tests to cover the new attributes correctly.
Additional Locations (2)
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 701694c. Configure here.
msonnb
force-pushed
the
ms/deprecated-http-net-attributes
branch
from
August 24, 2026 08:35
71a94a4 to
afc9274
Compare
msonnb
changed the title
http.* and net.* span attributes on HTTPS spanshttp.* span attributes on HTTP spans
msonnb
force-pushed
the
ms/deprecated-http-net-attributes
branch
3 times, most recently
from
August 24, 2026 13:25
5177d40 to
743aa06
Compare
Part of the v11 migration away from attributes `@sentry/conventions` marks deprecated. Scoped to the `http.*` attributes on HTTP spans; the `net.*` attributes are migrated separately in #23301. Straight renames: `http.method` -> `http.request.method`, `http.status_code` -> `http.response.status_code`, `http.scheme` -> `url.scheme`, `http.user_agent` -> `user_agent.original`, `http.response_content_length` -> `http.response.body.size`, `http.response_transfer_size` -> `http.response.size`, and `url.same_origin` -> `http.request.same_origin`. `http.request_content_length`, `http.request_content_length_uncompressed`, `http.response_content_length_uncompressed` and `http.status_text` are left alone — they are not in `@sentry/conventions` at all, so they have no replacement to move to. Three cases needed more than a rename: - `http.target` carried pathname *and* query, while `url.path` is the pathname only. The core server span set neither `url.query` nor `url.fragment`, so dropping `http.target` would have lost the query — it now sets both, which the node server span already did. - Consumers that matched on `http.target` were repointed at `url.path`: the react-router low-quality-transaction filter and the TanStack Start tunnel-route filter, both `ignoreSpans` rules against our own spans that would otherwise have silently stopped matching. The Next.js readers keep `http.target` as a fallback behind a `url.path` primary, since they also see spans from a user's own OpenTelemetry instrumentation. All other read-side fallbacks are untouched for the same reason. - `http.host`, `http.flavor` and `http.client_ip` are dropped without a replacement being set here. Their replacements — `server.address`, `network.protocol.version` and `client.address` — are introduced by #23301, so setting them here too would mean two PRs writing the same keys with different values. `SanitizedRequestData`, the shape backing `http` breadcrumb data, now keys the method as `http.request.method`. Span attributes in the touched files are now imported from `@sentry/conventions/attributes` rather than written as string literals. That is what surfaced `url.same_origin` as deprecated; as a literal it was invisible. `no-unfiltered-url-attributes` no longer guards `http.target`: nothing sets it, and its replacement `url.path` is a bare pathname with no query to filter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
msonnb
force-pushed
the
ms/deprecated-http-net-attributes
branch
from
August 25, 2026 12:03
743aa06 to
477d4a1
Compare
msonnb
changed the title
http.* span attributes on HTTP spanshttp.* span attributes on HTTP spans
Member
Author
|
will stack this |
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.
Replaces the
http.*span attributes@sentry/conventionsmarks deprecated.Straight renames
http.method->http.request.methodhttp.status_code->http.response.status_codehttp.scheme->url.schemehttp.user_agent->user_agent.originalhttp.response_content_length->http.response.body.sizehttp.response_transfer_size->http.response.sizeurl.same_origin->http.request.same_originhttp.request_content_length,http.request_content_length_uncompressed,http.response_content_length_uncompressedandhttp.status_textare left alone. They are not in@sentry/conventionsat all, so they have no replacement to move to.Changes that are not renames
http.host,http.flavorandhttp.client_ipThese are dropped without a replacement being set here. Their replacements
server.address,network.protocol.versionandclient.addressare introduced by #23301 as part of thenet.*alignment.url.queryandurl.fragmenton core server spanshttp.targetheld the pathname and the query.url.pathholds only the pathname.The server span in
@sentry/coreset neitherurl.querynorurl.fragment. Droppinghttp.targetwould therefore have lost the query. That span now sets both. The server span in@sentry/nodealready set both.Consumers that matched on
http.targetTwo
ignoreSpansrules match spans that the SDK itself emits. Both now match onurl.path:@sentry/react-router@sentry/tanstackstart-reactThe readers in
@sentry/nextjsstill readhttp.target, but only after they readurl.path. These readers also receive spans from an OpenTelemetry instrumentation that the user set up, and that instrumentation still emits the old attributes. Every other read-side fallback stays for the same reason.SanitizedRequestDataThis type is the shape of
httpbreadcrumb data. It now useshttp.request.methodas the key for the request method.no-unfiltered-url-attributesThis lint rule no longer guards
http.target. The SDK no longer setshttp.target. Its replacementurl.pathholds a pathname without a query, so there is nothing to filter.part of #18895