feat: Span Streaming - #1400
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Lms24
left a comment
Lms24
left a comment
There was a problem hiding this comment.
This looks mostly good to me, thanks for taking this on!
Two comments we should address:
- the timing issue when merging startup and pageload spans
- do we still need users to set
spanStreamingIntegration()?
All other comments are nits/recommendations, so feel free to disregard them/follow-up on in later PRs.
The renderer SDK flushes streamed spans on an interval so the pageload span tree can arrive over multiple envelopes. Since the pageload segment gets re-created in the startup trace, pageload spans sent directly from earlier envelopes would reference a segment that is never sent. We now buffer streamed span envelopes while waiting for the renderer pageload and merge all spans from the pageload trace into the startup span. Buffered spans from other traces are forwarded unmodified once the wait ends.
Numeric attribute types are inferred from the value so whole numbers get typed 'integer' and fractional values 'double'.
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 d40e90d. Configure here.
Lms24
left a comment
Lms24
left a comment
There was a problem hiding this comment.
Nice, thanks for applying my suggestions! The buffering approach now looks correct to me.
| if (extractTraceId) { | ||
| extracted.push(...container.items.filter((span) => span.trace_id === extractTraceId)); | ||
| container.items = container.items.filter((span) => span.trace_id !== extractTraceId); | ||
| } |
There was a problem hiding this comment.
super-l: Span envelopes always only carry spans of one specific traceId. So, these checks shouldn't be necessary. But they don't hurt either and it's a good safety measure. Happy to leave this up to you.
| }, | ||
| "dependencies": { | ||
| "@sentry/browser": "10.67.0", | ||
| "@sentry/conventions": "^0.16.0", |
There was a problem hiding this comment.
super-l: 0.19.0 is the latest version but if you wanna stay in lockstep with v10 of the JS repo, 0.16.0 is fine.
2 participants
Tests span streaming mode and ensures and
startupTracingIntegrationworks when this mode is enabled.