fix(core): Set http header span attributes as string arrays - #24231
Open
Lms24 wants to merge 2 commits into
Open
fix(core): Set http header span attributes as string arrays#24231Lms24 wants to merge 2 commits into
Lms24 wants to merge 2 commits into
Conversation
Lms24
added this pull request to stack #24232
September 9, 2026 09:58
Contributor
size-limit report 📦
|
Lms24
marked this pull request as ready for review
September 9, 2026 10:33
Lms24
requested review from
JPeer264,
logaretm,
mydea and
nicohrubec
and removed request for
a team
September 9, 2026 10:33
s1gr1d
approved these changes
Sep 9, 2026
| @@ -310,25 +310,26 @@ export function httpHeadersToSpanAttributes( | |||
| spanAttributes[`${prefix}${lowerKey}.${cookieKey}`] = cookieValue; | |||
Member
There was a problem hiding this comment.
Because you mentioned the "cookie extra case" - that's this line, right?
If we keep that, we should probably add http.request.header.cookie.<key> and http.response.header.set-cookie.<key> to conventions. I would keep it as a string as it's just a string value:
http.request.header.cookie.session_id = "abc123"
http.request.header.cookie.theme = "dark"
If we want to stay fully convention-conforming, we could save the cookie as a string array as we don't need the extra per-cookie key anymore with string arrays:
http.request.header.cookie = ["session_id=abc123; theme=dark"]
http.response.header.set-cookie = ["session_id=abc123", "theme=dark; Path=/"]
Member
Author
There was a problem hiding this comment.
After talking a bit about this within the team and confirming scrubbing behaviour with Ingest, we'll go with leaving the cookie headers as-is. I'm gonna add new attributes to conventions to cover these attributes explicitly to document the slight divergence from the http.request.header .<key>: string[] pattern.
Lms24
force-pushed
the
lms/http-header-attributes-as-arrays
branch
from
September 10, 2026 10:37
0ab5c18 to
cfd419b
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
310
to
+313
| spanAttributes[`${prefix}${lowerKey}.${cookieKey}`] = cookieValue; | ||
| } | ||
| } else { | ||
| spanAttributes[`${prefix}${lowerKey}`] = FILTERED_VALUE; | ||
| spanAttributes[`${prefix}${lowerKey}`] = [FILTERED_VALUE]; |
Contributor
There was a problem hiding this comment.
Bug: Malformed cookie headers are set as an array ['[Filtered]'], but the PR description states they should be single strings, creating a type inconsistency for span attributes.
Severity: MEDIUM
Suggested Fix
To ensure type consistency as described in the PR, change the assignment for filtered malformed cookie headers from an array to a single string. Modify line 313 from spanAttributes[...]= [FILTERED_VALUE] to spanAttributes[...] = FILTERED_VALUE.
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/core/src/utils/request.ts#L310-L313
Potential issue: The code at `request.ts:313` handles malformed or empty `Cookie`
headers by setting the corresponding span attribute to `[FILTERED_VALUE]`, which is an
array. This contradicts the PR's stated goal that "Cookie attributes ... stay single
strings". While correctly parsed cookies result in string attributes, this special case
for malformed headers introduces a type inconsistency. Downstream consumers expecting
only string values for cookie-related attributes may fail when encountering an array in
these edge-case scenarios, which can occur with custom clients or proxies.
Did we get this right? 👍 / 👎 to inform future reviews.
Lms24
force-pushed
the
lms/http-header-attributes-as-arrays
branch
from
September 10, 2026 11:07
cfd419b to
b2eb726
Compare
![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 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 b2eb726. Configure here.
Base automatically changed from
lms/fix-core-http-header-normalization
to
develop
September 10, 2026 13:15
`httpHeadersToSpanAttributes` now writes `http.request.header.<key>` / `http.response.header.<key>` as a string array instead of a single string. Headers sent multiple times previously got their values joined with `;`, which lost the boundaries between values; each value is now its own array entry. This matches the Sentry/OTel semantic conventions and the shape the outgoing-fetch (undici) instrumentation already emits for the same attributes. `packages/browser`'s `httpContext` integration is updated too, so `http.request.header.referer` isn't the one header attribute left as a string. Cookie attributes (`http.request.header.cookie.<name>`) stay single strings, as a cookie only ever has one value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Lms24
force-pushed
the
lms/http-header-attributes-as-arrays
branch
from
September 10, 2026 13:15
b2eb726 to
a71135a
Compare
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.
Follow-up to #24222:
httpHeadersToSpanAttributesnow setshttp.request.header.<key>/http.response.header.<key>asArray<string> instead ofstring`. This aligns SDK behaviour with the type specified in sentry and otel conventions.Notes:
http.request.header.cookie.<name>,set-cookie.<name>) stay single strings. A bit of a divergence from conventions but I think this should be fine, given this attribute doesn't fully fit the key pattern in conventions. probably worth adding a dedicated entry for this attribute in conventions. A cookie has exactly one value, so no need for an array here.packages/browser'shttpContextintegration writeshttp.request.header.refereritself (it deliberately doesn't use the core util, for bundle size). It's included here so it isn't the one header attribute left as a string.