feat(develop): Add new_trace()#16093
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
|
||
| ```python | ||
| import sentry_sdk | ||
|
|
||
| sentry_sdk.continue_trace(request.headers) | ||
| ``` | ||
|
|
There was a problem hiding this comment.
I think we should align this across SDKs. The snippets here currently do not related to any implementation but we should pick one and document it via pseudo code.
There was a problem hiding this comment.
@Lms24 Is the JS example up to date? I have two reservations regarding it:
- It's not top-level API, but scope API, which means users would need to be aware of the scope system and pick the correct scope to put the propagation context on. Given that even we ourselves struggle with this 🤣 I don't think that's a good idea.
- It's not immediately clear what it (a propagation context) does. Compare this with e.g.
continueTracewhich is intuitive.
Could we align on continue_trace()/continueTrace() or something similar?
There was a problem hiding this comment.
Related, I didn't touch these in this PR but I have similar concerns about
traceparent = scope.getTraceparent()
baggage = scope.getBaggage()
Those should ideally also not have anything to do with a specific scope, but rather be top level, and we pick the correct scope to retrieve them from in the background.
traceparent = sentry_sdk.get_traceparent()
baggage = sentry_sdk.get_baggage()
There was a problem hiding this comment.
I'm ok with changing this to use global functions.
There was a problem hiding this comment.
Sorry, these snippets (at the time of writing) were meant to be low-level pseudocode (shouldn't have used the javascript langauge for this). In JS, today, we have Sentry.continueTrace:
Sentry.continueTrace({
sentryTrace: request.headers['sentry-trace'],
baggage: request.headers['baggage'],
}, () => {
Sentry.startSpan({ name: 'test' }, () => {
// ....
});
})Which, besides the callback, is very similar to what is being proposed here IIUC.
There was a problem hiding this comment.
One more thing:
traceparent = sentry_sdk.get_traceparent()
baggage = sentry_sdk.get_baggage()
Sounds fine to me! We went down a slightly different route in JS and exported a Sentry.getTraceData() API which returns a dict/object with both headers (actually, 3, if propagateTraceParent is enabled). Our thinking back then was, there's no reason to only need one of these headers, so let's take care of them together. Not saying we should generalize this because the two separate ones can be combined and built upon. Just pointing out prior art 😅
There was a problem hiding this comment.
@Lms24 Since I'm already updating all the examples, there's
traceparent = scope.getTraceparent()
baggage = scope.getBaggage()
How does this actually look in JS? I found getTraceData(), is that it?
There was a problem hiding this comment.
Haha, commented about the same thing at the same time 😅
There was a problem hiding this comment.
Ok thank you both, updated if anyone wants to give this a last pass.
Lms24
left a comment
There was a problem hiding this comment.
Thanks for taking care of this! I think both, new_trace and continue_trace make sense and are similar to what we already have in JS!
Co-authored-by: Lukas Stracke <lukas.stracke@sentry.io>
Add API to clear the propagation context. Add some examples, too.
Preview: https://develop-docs-git-ivana-span-firstnew-trace.sentry.dev/sdk/telemetry/spans/span-trace-propagation/
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:
LEGAL BOILERPLATE
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.
EXTRA RESOURCES