feat(skills): Add span streaming migration skill for JavaScript#130
feat(skills): Add span streaming migration skill for JavaScript#130chargome wants to merge 3 commits into
Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
String matching in ignoreSpans uses substring includes, not glob patterns. The `*` was treated as a literal character. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| if (span.name?.includes('/health')) { | ||
| span.name = '[filtered]'; | ||
| } | ||
| // 'attributes' replaces 'data' | ||
| if (span.attributes) { | ||
| delete span.attributes['http.request.body']; | ||
| } | ||
| return span; | ||
| }), | ||
| }); | ||
| ``` |
There was a problem hiding this comment.
Bug: The beforeSendSpan migration example only shows a server-side configuration, creating ambiguity for browser projects that should use spanStreamingIntegration().
Severity: MEDIUM
Suggested Fix
Update section 2.2 to provide explicit, separate examples for migrating beforeSendSpan in both browser and server environments, similar to the structure in section 2.1. The browser example should show Sentry.withStreamedSpan(...) used alongside Sentry.spanStreamingIntegration().
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: skills/sentry-span-streaming/SKILL.md#L185-L199
Potential issue: The skill documentation for migrating `beforeSendSpan` in section 2.2
only provides a server-side configuration example using `traceLifecycle: 'stream'`. It
omits an equivalent example for browser-side SDKs, which should use
`Sentry.spanStreamingIntegration()`. This ambiguity could cause an AI agent to
incorrectly apply the server-side configuration to a browser project. This might result
in a redundant or conflicting configuration where both `spanStreamingIntegration()` and
`traceLifecycle: 'stream'` are added, or the agent may become confused about the correct
approach for browser environments, leading to an incorrect migration.
There was a problem hiding this comment.
should we just remove other options in the code example and only feature beforeSendSpan?
Lms24
left a comment
Lms24
left a comment
There was a problem hiding this comment.
Some comments but otherwise LGTM!
| | Go | Not yet available | | ||
| | Other SDKs | Not yet available | | ||
|
|
||
| If the user's project does not use a JavaScript Sentry SDK, inform them that span streaming is currently only available for JavaScript SDKs and stop here. |
There was a problem hiding this comment.
l: given we have the table below, we can reduce the number of places follow-up work on this skill has to touch when a new SDK is ready. WDYT?
| If the user's project does not use a JavaScript Sentry SDK, inform them that span streaming is currently only available for JavaScript SDKs and stop here. | |
| If the user's project does not use a supported SDK, inform them that span streaming is currently only available for JavaScript SDKs and stop here. |
| cat go.mod 2>/dev/null | grep sentry | ||
| ``` | ||
|
|
||
| If a non-JavaScript Sentry SDK is detected, inform the user that span streaming is not yet available for their platform. |
There was a problem hiding this comment.
| If a non-JavaScript Sentry SDK is detected, inform the user that span streaming is not yet available for their platform. | |
| If an unsupported Sentry SDK is detected, inform the user that span streaming is not yet available for their platform. |
| }); | ||
| ``` | ||
|
|
||
| Place `spanStreamingIntegration()` **before** `browserTracingIntegration()` in the array. |
There was a problem hiding this comment.
hmm is this really important? I thought I built the integrations independently of ordering 😅 but might have missed something
| if (span.name?.includes('/health')) { | ||
| span.name = '[filtered]'; | ||
| } | ||
| // 'attributes' replaces 'data' | ||
| if (span.attributes) { | ||
| delete span.attributes['http.request.body']; | ||
| } | ||
| return span; | ||
| }), | ||
| }); | ||
| ``` |
There was a problem hiding this comment.
should we just remove other options in the code example and only feature beforeSendSpan?
|
|
||
| ### 2.4 Configure `ignoreSpans` (Optional) | ||
|
|
||
| `ignoreSpans` works in both static and streaming modes. It filters spans at creation time, preventing them from being recorded or sent. |
There was a problem hiding this comment.
It filters spans at creation time
that's only correct for streaming though :( which makes it all the harder to explain this feature to clankers and users:
streaming: ignoreSpans is applied at span start. Only data available at span start (attributes and span name) is taken into account
static: ignoreSpans is applied at root span end. Only data available at this time (attributes and span name) is taken into account
A clanker might know what data is available when, so it probably makes sense to explain this. WDYT?
2 participants
Adds a skill for migrating sentry setups from transaction based tracing to span streaming.
This PR only covers JS but has placeholders for other languages.