feat: Add custom distributed tracing docs for JS/Node #8469
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
mydea
force-pushed
the
fn/continueTrace
branch
from
95df29f to
8805616
Compare
lforst
reviewed
Nov 10, 2023
Contributor
lforst
left a comment
lforst
left a comment
There was a problem hiding this comment.
Sorry for big review. I wanted to use the opportunity to clean this up in general.
src/platform-includes/distributed-tracing/custom-instrumentation/javascript.mdx
Show resolved
Hide resolved
src/platform-includes/distributed-tracing/custom-instrumentation/javascript.mdx
Outdated
Show resolved
Hide resolved
| - Inject tracing information to the outgoing request when sending outgoing requests. | ||
|
|
||
| To learn more, see our <PlatformLink to="/distributed-tracing/">Distributed Tracing</PlatformLink> docs. | ||
| To learn more, see our <PlatformLink to="/usage/distributed-tracing/">Distributed Tracing</PlatformLink> docs. |
Contributor
There was a problem hiding this comment.
I feel conflicted about having these two pages 🤔 Or rather about linking back to that page here. People are here for manual instrumentation. Why should we link back to auto-instrumentation. I think we should either delete this line or link to the auto-instrumentation all the way at the top of this page in-case people got lost.
Member
Author
There was a problem hiding this comment.
I think the idea of this is that on that page we describe distributed tracing as a concept, generally 🤔 (I copied this from other existing pages for custom instrumentation, FWIW)
Contributor
There was a problem hiding this comment.
@mydea - I agree that's the point of the link. Ideally, conceptual information and how-to information should be separate, which would make this less confusing. As it is, I think the link is ok though.
src/platform-includes/distributed-tracing/custom-instrumentation/javascript.mdx
Outdated
Show resolved
Hide resolved
src/platform-includes/distributed-tracing/custom-instrumentation/javascript.mdx
Outdated
Show resolved
Hide resolved
|
|
||
| ## Enabling Distributed Tracing without `Http` Integration | ||
|
|
||
| If you do not want to use the `Http` integration in conjuction with `tracing: true`, you can manually extract and inject tracing information into your application. For this, you must: |
Contributor
There was a problem hiding this comment.
Suggested change
| If you do not want to use the `Http` integration in conjuction with `tracing: true`, you can manually extract and inject tracing information into your application. For this, you must: | |
| If you do not want to use the `Http` or `Undici` integrations, you can manually trace your application. For this, you must: |
src/platform-includes/distributed-tracing/custom-instrumentation/javascript.mdx
Outdated
Show resolved
Hide resolved
src/platform-includes/distributed-tracing/custom-instrumentation/node.mdx
Outdated
Show resolved
Hide resolved
src/platform-includes/distributed-tracing/custom-instrumentation/javascript.mdx
Outdated
Show resolved
Hide resolved
src/platform-includes/distributed-tracing/custom-instrumentation/node.mdx
Outdated
Show resolved
Hide resolved
Co-authored-by: Luca Forstner <[email protected]>
shanamatthews
approved these changes
Nov 14, 2023
Contributor
shanamatthews
left a comment
shanamatthews
left a comment
There was a problem hiding this comment.
Ok from my end, once you fix the formatting issues I commented on.
src/platform-includes/distributed-tracing/custom-instrumentation/javascript.mdx
Outdated
Show resolved
Hide resolved
src/platform-includes/distributed-tracing/custom-instrumentation/javascript.mdx
Outdated
Show resolved
Hide resolved
src/platform-includes/distributed-tracing/custom-instrumentation/javascript.mdx
Outdated
Show resolved
Hide resolved
src/platform-includes/distributed-tracing/custom-instrumentation/javascript.mdx
Outdated
Show resolved
Hide resolved
src/platform-includes/distributed-tracing/custom-instrumentation/javascript.mdx
Show resolved
Hide resolved
src/platform-includes/distributed-tracing/custom-instrumentation/node.mdx
Outdated
Show resolved
Hide resolved
src/platform-includes/distributed-tracing/custom-instrumentation/node.mdx
Outdated
Show resolved
Hide resolved
src/platform-includes/distributed-tracing/how-to-use/javascript.mdx
Outdated
Show resolved
Hide resolved
src/platform-includes/distributed-tracing/custom-instrumentation/node.mdx
Outdated
Show resolved
Hide resolved
Co-authored-by: Shana Matthews <[email protected]>
Lms24
approved these changes
Nov 14, 2023
src/platform-includes/distributed-tracing/custom-instrumentation/javascript.mdx
Outdated
Show resolved
Hide resolved
lforst
approved these changes
Nov 14, 2023
Co-authored-by: Lukas Stracke <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds docs for custom distributed tracing for Node & Browser JavaScript, including the new
continueTracemethod, which was added here: getsentry/sentry-javascript#9164.We do not show this for Next, Remix, Sveltekit and Astro - not 100% sure if this is correct, or if there are other guides where we should/should not show this.