fix(deno): Clear pre-existing OTel global before registering TracerProvider#19723
fix(deno): Clear pre-existing OTel global before registering TracerProvider#19723
Conversation
…ovider Supabase Edge Runtime (and Deno's native OTel) pre-registers on the `@opentelemetry/api` global, causing `trace.setGlobalTracerProvider()` to silently fail. Call `trace.disable()` first so Sentry's provider always wins. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
logaretm
requested review from
Lms24,
andreiborza,
isaacs and
logaretm
and removed request for
logaretm
size-limit report 📦
|
There was a problem hiding this comment.
Without this fix, AI SDK OTel spans (gen_ai.*) never reach Sentry because the Sentry TracerProvider is never actually set as the global
H: Could you clarify this? I suspect that either no spans at all are sent or all of them should be sent. What makes gen_ai spans special here? This sounds to me like an agent partially analyzed a problem and didn't grasp the full scope of it. I don't think we can merge this until we know the consequences and the current state of tracing.
| export function setupOpenTelemetryTracer(): void { | ||
| // Clear any pre-existing OTel global registration (e.g. from Supabase Edge Runtime | ||
| // or Deno's built-in OTel) so Sentry's TracerProvider gets registered successfully. | ||
| trace.disable(); |
There was a problem hiding this comment.
m l: I'm wondering if this backfires for people using Sentry with a custom OTel setup or deliberately with Deno's native tracing (OTLP exporter). The good news is that we don't document this setup for Deno, so I think we can just ignore it for the moment and walk back on this change if anyone complains.
Update: I just saw that we gate this function call with skipOpenTelemetrySetup, so users can opt out of it. That's good. So I guess the worst consequence here is that anyone using native tracing with Sentry might need to set this flag now. Which we can classify as a fix because that's how we intended the SDK to work anyway. Downgraded from logaf M to L
There was a problem hiding this comment.
comment, no direct action required: I'm fine with these unit tests for now but to be clear these don't prove that the fix works as intended in an actual app. Long-term I'd like us to at least add one e2e app for Deno (or an integration tests setup like for Node) to more reliably verify this. This goes back to my main review comment that I don't think we fully grasped the scope of the current behavior yet. If we had such a test, we could more reliably say that at least some spans are sent.
2 participants
Summary
trace.disable()beforetrace.setGlobalTracerProvider()in@sentry/deno's OTel tracer setupTracerProvideron the@opentelemetry/apiglobal (Symbol.for('opentelemetry.js.api.1'))gen_ai.*) never reach Sentry because the SentryTracerProvideris never actually set as the globalContext
Supabase Edge Runtime (Deno 2.1.4+) registers its own
TracerProviderbefore user code runs. The OTel API'ssetGlobalTracerProvideris a no-op if a provider is already registered, so Sentry's tracer silently gets ignored. Callingtrace.disable()clears the global, allowingsetGlobalTracerProviderto succeed.This matches the pattern already used in
cleanupOtel()in the test file and is safe because:Sentry.init()Test plan
should override pre-existing OTel provider with Sentry providertest — simulates a pre-existing provider and verifies Sentry overrides itshould override native Deno OpenTelemetry when enabledtest — verifies Sentry captures spans even whenOTEL_DENO=truegen_aispans appear in Sentry🤖 Generated with Claude Code
Closes #19724 (added automatically)