Uh oh!
There was an error while loading. Please reload this page.
feat(browser): Add support for streamed spans in httpContextIntegration - #20464
Conversation
| const reqData = getHttpRequestData(); | ||
| span.attributes = { |
There was a problem hiding this comment.
l: instead of spreading this here, can we just set the attributes we want to set directly? IMHO we can just have a utility we keep using like setIfNotExists(span.attributes, 'url.full', reqData.url), or similar? Not 100% sure but I'd suspect this is faster at scale than creating a new object all the time when processing spans like this?
There was a problem hiding this comment.
We have a utility @sentry/core that does exactly what we would want I think (ref). We could export that from core and then use it in any integrations we need to port. I copied the approach here from how Lukas did the cultureContext integration (ref), not sure if he had a specific reason in mind not to use this. Wdyt?
There was a problem hiding this comment.
sounds good to me (exporting this and using it). FWIW the approach from lukas and in this PR are not bad I just think it is slightly better (probably) to avoid creating new objects for these things all the time - and this will likely happen quite a lot if there are multiple event processors all just adding one or two attributes!
size-limit report 📦
|
Uh oh!
There was an error while loading. Please reload this page.
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 7c5c438. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
…ration (#20481) Applies the same `safeSetSpanJSONAttributes` refactor from #20464 to the `cultureContext` integration's `processSegmentSpan` hook. This gives us in-place mutation instead of spread-and-reassign and nullish values are skipped (these get dropped during serialization anyways but doesn't hurt to just not assign them in the first place). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This PR adds span processing support for the
httpContextIntegration:processSegmentSpancallback onhttpContextIntegrationthat adds the attributes. All attributes are already registered in sentry conventions: https://getsentry.github.io/sentry-conventions/attributes/url/#url-full and https://getsentry.github.io/sentry-conventions/attributes/http/#http-request-header-keyCloses#20378