feat(core)!: Return StreamedSpanJSON from spanToJSON - #22968
Conversation
![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 54994d4. Configure here.
| debug.log(`[Tracing] Finishing current active span with op: ${activeSpanJson?.attributes[SENTRY_OP]}`); | ||
| // If there's an open active span, we need to finish it before creating an new one. | ||
| activeSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON, 'cancelled'); | ||
| activeSpan.setAttribute(SENTRY_IDLE_SPAN_FINISH_REASON, 'cancelled'); |
There was a problem hiding this comment.
Broken open-span end checks
High Severity
spanToJSON now returns StreamedSpanJSON, where end_timestamp is always set (_endTime ?? _startTime). Call sites still treat it like the old optional timestamp, so open-span checks such as !end_timestamp never succeed. That can leave idle spans unfinished, skip lazy-route name updates, and mis-detect whether a navigation span has ended. In updateNavigationSpan, the condition is also inverted from the previous !timestamp check.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 54994d4. Configure here.
|
|
||
| if (this._mountSpan && includeRender) { | ||
| const startTime = spanToJSON(this._mountSpan).timestamp; | ||
| const startTime = spanToJSON(this._mountSpan).start_timestamp; |
There was a problem hiding this comment.
Wrong render span start time
Medium Severity
The profiler render span previously started from the mount span’s end time (timestamp). It now reads start_timestamp, so the render span incorrectly includes mount duration instead of measuring time on screen from mount completion to unmount.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 54994d4. Configure here.
| const { op, description } = spanToJSON(rootSpan); | ||
| if (op === 'navigation' && description === '-1') { | ||
| const { attributes, name } = spanToJSON(rootSpan); | ||
| if (attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] === 'navigation' && name === '-1') { |
There was a problem hiding this comment.
Shadowed back-navigation name
Medium Severity
Destructuring name from spanToJSON shadows the outer name (location.pathname). For back-button navigations where the span name is -1, updateName now reassigns -1 instead of the real pathname, so those spans keep an unusable name.
Reviewed by Cursor Bugbot for commit 54994d4. Configure here.
| * Convert a span to the intermediate {@link StreamedSpanJSON} representation. | ||
| */ | ||
| export function spanToStreamedSpanJSON(span: Span): StreamedSpanJSON { | ||
| export function spanToJSON(span: Span): StreamedSpanJSON { |
There was a problem hiding this comment.
Feat PR lacks integration tests
Low Severity
This is a feat PR, but the diff only updates unit tests and does not add an integration or E2E test covering the new spanToJSON → StreamedSpanJSON behavior across packages. Flagged because the review rules require at least one integration or E2E test for feat PRs.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 54994d4. Configure here.
size-limit report 📦
|
1 participant
StreamedSpanJSONfromspanToJSON