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.
There was a problem hiding this comment.
Agreed on the e2e gap — the unit tests verify the mechanism but not real-world behavior. We tested manually with a Supabase Edge Function + AI SDK and confirmed gen_ai.* spans appear in Sentry with the fix and don't without it. Happy to share the test app as a reference.
The e2e infrastructure is all there (Verdaccio, test-utils, Playwright). A Deno e2e app would just need a Deno.serve() server with Sentry + a route triggering OTel-instrumented spans, then Playwright tests asserting they arrive. Could be a follow-up PR.
There was a problem hiding this comment.
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?
The scope is OTel-instrumented spans specifically:
Sentry.startSpan()works fine — it goes through Sentry's internal span pipeline, not the OTel globalTracerProvider. We confirmed this: customSentry.startSpanspans showed up in Sentry.- Spans from OTel-instrumented libraries are lost — AI SDK (and anything else using
@opentelemetry/api'stracer.startSpan()) hits the pre-existing Supabase provider instead of Sentry's. Thegen_ai.*spans from AI SDK were the ones we noticed missing, but it would affect any OTel instrumentation.
This was confirmed in the test application I was running https://github.com/sergical/supabase-ai-test/blob/main/supabase/functions/ai-chat/index.ts#L14
Supabase Edge Runtime registers a TracerProvider on the OTel global before user code runs. trace.setGlobalTracerProvider() is a no-op when a provider already exists (just logs a diag warning), so Sentry's provider silently fails to register. Sentry's own API bypasses this because it doesn't go through the OTel global to create spans.
I'll update the PR description to be more precise about this.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
- Add @sentry/deno to package.json so matrix builder triggers on SDK changes - Add Deno setup step in build.yml for CI runtime - Fix deno.json relative import paths in copyToTemp.ts for temp dir copies Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?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 prepared a fix for the issue found in the latest run.
- ✅ Fixed: E2E test filters look for root span in child spans array
- Changed the transaction wait filters to match on event.transaction for root spans so the tests no longer hang.
Or push these changes by commenting:
@cursor push 23e98514d3
Preview (23e98514d3)
diff --git a/dev-packages/e2e-tests/test-applications/deno/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/deno/tests/transactions.test.ts
--- a/dev-packages/e2e-tests/test-applications/deno/tests/transactions.test.ts
+++ b/dev-packages/e2e-tests/test-applications/deno/tests/transactions.test.ts
@@ -3,7 +3,7 @@
test('Sends transaction with Sentry.startSpan', async ({ baseURL }) => {
const transactionPromise = waitForTransaction('deno', event => {
- return event?.spans?.some(span => span.description === 'test-sentry-span') ?? false;
+ return event?.transaction === 'test-sentry-span';
});
await fetch(`${baseURL}/test-sentry-span`);
@@ -62,7 +62,7 @@
test('OTel span appears as child of Sentry span (interop)', async ({ baseURL }) => {
const transactionPromise = waitForTransaction('deno', event => {
- return event?.spans?.some(span => span.description === 'sentry-parent') ?? false;
+ return event?.transaction === 'sentry-parent';
});
await fetch(`${baseURL}/test-interop`);This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
dev-packages/e2e-tests/test-applications/deno/tests/transactions.test.ts
Show resolved
Hide resolved
Replace `existsSync` check + `readFileSync` with a single `readFileSync` wrapped in try-catch to eliminate the CodeQL-flagged race condition. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Deno couldn't find `@opentelemetry/api` in node_modules because it wasn't listed in package.json. Adding `"nodeModulesDir": "auto"` lets Deno auto-install npm: specifier deps on startup. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Revert `nodeModulesDir: "auto"` and add `@opentelemetry/api` to package.json instead. This lets pnpm install OTel into node_modules so Deno resolves it in manual mode without creating a `.deno/` dir that duplicates playwright. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.*from AI SDK, or any library using@opentelemetry/api) never reach Sentry because Sentry'sTracerProviderfails to register as the global. Sentry's ownstartSpan()API is unaffected since it bypasses the OTel global.Context
Supabase Edge Runtime (Deno 2.1.4+) registers its own
TracerProviderbefore user code runs. The OTel API'strace.setGlobalTracerProvider()is a no-op if a provider is already registered (it only logs a diag warning), so Sentry's tracer silently gets ignored.What works without the fix:
Sentry.startSpan()— goes through Sentry's internal pipeline, not the OTel global.What breaks without the fix: Any spans created via
@opentelemetry/api(AI SDK'sgen_ai.*spans, HTTP instrumentations, etc.) — these hit the pre-existing Supabase provider instead of Sentry's.Calling
trace.disable()clears the global, allowingtrace.setGlobalTracerProvider()to succeed. This matches the pattern already used incleanupOtel()in the test file and is safe because:Sentry.init()skipOpenTelemetrySetupso users with custom OTel setups can opt outTest plan
should override pre-existing OTel provider with Sentry providerunit test — simulates a pre-existing provider and verifies Sentry overrides itshould override native Deno OpenTelemetry when enabledunit test — verifies Sentry captures spans even whenOTEL_DENO=truedev-packages/e2e-tests/test-applications/deno/) — Deno server with pre-existing OTel provider, 5 tests:Sentry.captureException)Sentry.startSpantransactiontracer.startSpandespite pre-existing provider (core regression test)tracer.startActiveSpan(AI SDK pattern)Sentry.startSpan()spans appeared in Sentry both before and after the fix, butgen_ai.*OTel spans only appeared after the fix🤖 Generated with Claude Code
Closes #19724