ref(core): Gate updateName() custom source on an OTel inference brand#21649
Conversation
size-limit report 📦
|
Lms24
left a comment
Lms24
left a comment
There was a problem hiding this comment.
Naive q: Now that we vendored in instrumentation, we can set a source whenever that instrumentation calls span.updateName, right? I wonder if we can use this to our advantage and essentially get rid of any span.updateName and updateSpanName differences.
Users who want to mark a name as intentionally chosen should use
updateSpanName() which always sets source via setAttributes().
My main concern is that this will never work reliably, as span.updateName() is the much more intuitive way to do this for users. When I added the alternative API, it was because I didn't see another way to differ between user- and instrumentation-initiated name updates. Now that we control the instrumentation, I think we could do this. WDYT?
I was thinking about this too, the problem is there are cases where we don't control the span name setting, e.g. if nextjs calls
Yeah, this is also my concern. It makes |
SentrySpan's `updateName()` now sets `sentry.source: 'custom'` only if the span was not started by a tracer (SentryTrace in the future). With the upcoming SentryTracerProvider, that creates native SentrySpans instead of OTel SDK spans, we need a way to make sure `updateName()` does not set source 'custom' on all spans unconditionally. Otherwise, spans created, that had their name updated, outside of Sentry's influence (i.e. inside frameworks like nextjs) would also have source set to custom which interferes with the final source inference at the end of a span. The need for this will be more apparent in follow-up PRs when we introduce the SentryTracerProvider and data inference that used to be done in our span exporter before.
andreiborza
force-pushed
the
ab/updatename-source
branch
from
12d196a to
e3c7978
Compare
andreiborza
changed the title
updateName() when source exists
|
I reworked the PR to use a completely different approach after discussing with @Lms24. We are gating not setting source to custom only if the span was created by a TracerProvider (which will brand Spans). This allows us to keep the existing behavior of |
Lms24
left a comment
Lms24
left a comment
There was a problem hiding this comment.
Thanks for reworking this! I think scoping this more narrowly to traceProvider-emitted spans makes sense!
3 participants
What
SentrySpan's
updateName()now setssentry.source: 'custom'only if the span was not started by a tracer (SentryTrace in the future).Why
With the upcoming SentryTracerProvider, that creates native SentrySpans instead of OTel SDK spans, we need a way to make sure
updateName()does not set source 'custom' on all spans unconditionally. Otherwise, spans created, that had their name updated, outside of Sentry's influence (i.e. inside frameworks like nextjs) would also have source set to custom which interferes with the final source inference at the end of a span.The need for this will be more apparent in follow-up PRs when we introduce the SentryTracerProvider and data inference that used to be done in our span exporter before.