docs(js): Add New Spans guide#17834
Conversation
…ick-start/splitlayout-cleanup
…ick-start/splitlayout-cleanup
… smi/span-first/js
inventarSarah
requested review from
Lms24,
chargome,
cleptric,
mydea and
sfanahata
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
![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 b8d6819. Configure here.
There was a problem hiding this comment.
we mix "service span" and "root span" in this doc. A couple of thoughts:
- I really like the "service span" name (as opposed to "segment span" how we call it in SDK code). I've probably been quite out of the loop here: Is this the public name we ended up choosing for it? Just want to confirm.
- It's probably fine to use "root span" when we talk about how things used to be with transactions but further below, we explain the
ignoreSpansbehaviour differences for child vs. root spans. Should we adjust to "child vs. service spans" there?
There was a problem hiding this comment.
- "service span" is a naming recommendation provided by Shannon -- I've collected the full list here
- I would really like to get use all the different span correctly -- it would be great if you (reviewers) leave a comment when the guide uses the wrong (or not best fitting) name
|
|
||
| <PlatformSection notSupported={["javascript.nextjs", "javascript.nuxt", "javascript.sveltekit", "javascript.astro", "javascript.solidstart", "javascript.capacitor", "javascript.tanstackstart-react", "javascript.remix", "javascript.react-router"]}> | ||
|
|
||
| - **Check the Sentry dashboard**: Spans should appear in the Traces view shortly after they complete. Traces look the same as in transaction mode, but without transactions. |
There was a problem hiding this comment.
Traces look the same as in transaction mode, but without transactions
This is true but unless I missed something, there are very few visual indications that actually point out that a span is a (new) span and not a transaction 😬 The only thing that comes to mind is that it's not showing a JSON button anymore. I don't think there's harm in leaving in this sentence but users might get confused at what they should be looking in the product.
This is of course not a primary docs issue but rather a product/UI issue. We might want to think about some kind of indicator (and, separate topic, please bring back JSON for streamed spans lol). That is assuming, I didn't miss any obvious indication 😅 Will ask product folks about this.
There was a problem hiding this comment.
In that case, I will update this to something like
"Traces look almost the same as in transaction mode. The main difference is that they have no transactions and only consist of spans."
| ); | ||
|
|
||
| const MainLive = HttpLive.pipe(Layer.provide(SentryLive)); | ||
| MainLive.pipe(Layer.launch, NodeRuntime.runMain); |
There was a problem hiding this comment.
Bug: The Effect server-side code snippet calls NodeRuntime.runMain but is missing the required import for NodeRuntime, which will cause a runtime ReferenceError.
Severity: CRITICAL
Suggested Fix
Add the import statement import { NodeRuntime } from "@effect/platform-node"; to the server-side code snippet in platform-includes/performance/enable-stream-mode/javascript.effect.mdx.
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: platform-includes/performance/enable-stream-mode/javascript.effect.mdx#L54
Potential issue: The server-side initialization code snippet in the documentation file
`platform-includes/performance/enable-stream-mode/javascript.effect.mdx` uses
`NodeRuntime.runMain` to launch the application. However, it fails to import
`NodeRuntime` from `@effect/platform-node`. Users who copy and paste this code will
encounter a `ReferenceError: NodeRuntime is not defined` when they try to run their
server, causing the application to crash immediately on startup.
3 participants
DESCRIBE YOUR PR
Adds a new page documenting stream mode for all JS SDKs, covering:
beforeSendSpan,ignoreSpans)Based on this draft: #17621
Michi's draft includes sections on distributed tracing, which I removed in this version. The APIs work identically in both modes; the topics are advanced enough that users are better served by the existing page, where all the details live, and users who already have distributed tracing set up don't need to change anything when switching to stream mode. Happy to discuss if you disagree.
Questions/Considerations for you
Closes: #17835
IS YOUR CHANGE URGENT?
Help us prioritize incoming PRs by letting us know when the change needs to go live.
SLA
Thanks in advance for your help!
PRE-MERGE CHECKLIST
Make sure you've checked the following before merging your changes:
EXTRA RESOURCES