From a04ef195b20b4d293b38e497cc9c56ba82dc2aac Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Tue, 11 Aug 2026 16:02:07 +0200 Subject: [PATCH 1/3] ref(cloudflare)!: Use shared `SentryTracerProvider` for OpenTelemetry interop Replaces the bespoke Cloudflare tracer with `SentryTracerProvider` from `@sentry/opentelemetry`, and covers `enableOpenTelemetrySetup` end to end in workerd with two integration suites. The enabled suite documents that `startActiveSpan` detaches into a root transaction of its own rather than nesting under the request span: it always resolves an explicit context, and Cloudflare installs no OTel context manager, so `context.active()` never carries the request span. `startSpan` passes no context and does nest, so both paths are asserted to pin the difference. The two workers are kept identical apart from the option itself, so the disabled suite also proves the wider OTel surface stays inert when the provider is off. Co-Authored-By: Claude Opus 5 --- MIGRATION.md | 2 + .../opentelemetry-tracer/disabled/index.ts | 41 +++++++ .../opentelemetry-tracer/disabled/test.ts | 36 +++++++ .../disabled/wrangler.jsonc | 6 ++ .../opentelemetry-tracer/enabled/index.ts | 40 +++++++ .../opentelemetry-tracer/enabled/test.ts | 100 ++++++++++++++++++ .../enabled/wrangler.jsonc | 6 ++ docs/migration/v11-end-state.md | 2 + packages/cloudflare/package.json | 1 + packages/cloudflare/src/baseSdk.ts | 10 -- .../cloudflare/src/opentelemetry/tracer.ts | 74 +------------ packages/cloudflare/src/request.ts | 6 +- packages/cloudflare/src/sdk.ts | 11 ++ .../cloudflare/test/opentelemetry.test.ts | 4 - 14 files changed, 252 insertions(+), 87 deletions(-) create mode 100644 dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/disabled/index.ts create mode 100644 dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/disabled/test.ts create mode 100644 dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/disabled/wrangler.jsonc create mode 100644 dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/index.ts create mode 100644 dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/test.ts create mode 100644 dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/wrangler.jsonc diff --git a/MIGRATION.md b/MIGRATION.md index 6edbb8ad61b1..f404947e0272 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -115,6 +115,8 @@ Sentry registers a minimal OpenTelemetry-compatible tracer provider, context man Spans go to Sentry. This is not a general OpenTelemetry pipeline: there is no exporter and no OTLP output. Sentry also refuses to register its provider if you already registered one of your own, logging a warning instead. If you want a real OpenTelemetry pipeline, use setup 3. +`@sentry/cloudflare/request` does not support this option. That entry point exists for runtimes that cannot enable the `nodejs_compat` compatibility flag (e.g. Shopify Oxygen) and sets up a reduced client without the OpenTelemetry tracer. Use the main `@sentry/cloudflare` entry point if you need setup 2. + ##### 3. Your own OpenTelemetry, Sentry linked to it Leave `enableOpenTelemetrySetup` unset or set it to `false`, turn Sentry tracing off, use your own OpenTelemetry setup, and add the Sentry `otlpIntegration()`: diff --git a/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/disabled/index.ts b/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/disabled/index.ts new file mode 100644 index 000000000000..3fe6e7fbb09a --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/disabled/index.ts @@ -0,0 +1,41 @@ +import { SpanKind, trace } from '@opentelemetry/api'; +import * as Sentry from '@sentry/cloudflare'; + +interface Env { + SENTRY_DSN: string; +} + +export default Sentry.withSentry( + (env: Env) => ({ + dsn: env.SENTRY_DSN, + tracesSampleRate: 1.0, + traceLifecycle: 'static', + // Deliberately left unset — the global tracer provider stays OTel's noop, so the spans below + // must not reach Sentry. + }), + { + async fetch() { + const tracer = trace.getTracer('integration-test-tracer'); + + const inactive = tracer.startSpan('otel inactive', { attributes: { 'test.attribute': 'inactive' } }); + inactive.end(); + + await tracer.startActiveSpan( + 'otel parent', + { kind: SpanKind.CLIENT, attributes: { 'test.attribute': 'parent' } }, + async otelParent => { + await Sentry.startSpan({ name: 'sentry child' }, async () => { + const otelGrandchild = tracer.startSpan('otel grandchild', { + attributes: { 'test.attribute': 'grandchild' }, + }); + otelGrandchild.end(); + }); + + otelParent.end(); + }, + ); + + return new Response('ok'); + }, + } satisfies ExportedHandler, +); diff --git a/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/disabled/test.ts b/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/disabled/test.ts new file mode 100644 index 000000000000..79d23f259e3c --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/disabled/test.ts @@ -0,0 +1,36 @@ +import { SENTRY_ORIGIN } from '@sentry/conventions/attributes'; +import type { Event } from '@sentry/core'; +import { expect, it } from 'vitest'; +import { SHORT_UUID_MATCHER } from '../../../../expect'; +import { createRunner } from '../../../../runner'; + +it('drops spans emitted through @opentelemetry/api when `enableOpenTelemetrySetup` is not enabled', async ({ + signal, +}) => { + const runner = createRunner(__dirname) + .expect(envelope => { + const transactionEvent = envelope[1]?.[0]?.[1] as Event; + + expect(transactionEvent.transaction).toBe('GET /'); + + // Only the Sentry span survives; it re-parents onto the request span because the noop OTel + // span it was nested under never became a real parent. + expect(transactionEvent.spans).toEqual([ + { + data: { [SENTRY_ORIGIN]: 'manual' }, + description: 'sentry child', + parent_span_id: transactionEvent.contexts?.trace?.span_id, + span_id: SHORT_UUID_MATCHER, + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: transactionEvent.contexts?.trace?.trace_id, + origin: 'manual', + }, + ]); + }) + .start(signal); + + await runner.makeRequest('get', '/'); + await runner.completed(); +}); diff --git a/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/disabled/wrangler.jsonc b/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/disabled/wrangler.jsonc new file mode 100644 index 000000000000..8e75e042c16e --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/disabled/wrangler.jsonc @@ -0,0 +1,6 @@ +{ + "name": "cloudflare-opentelemetry-tracer-disabled", + "main": "index.ts", + "compatibility_date": "2025-06-17", + "compatibility_flags": ["nodejs_compat"], +} diff --git a/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/index.ts b/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/index.ts new file mode 100644 index 000000000000..2e95710a8d79 --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/index.ts @@ -0,0 +1,40 @@ +import { SpanKind, trace } from '@opentelemetry/api'; +import * as Sentry from '@sentry/cloudflare'; + +interface Env { + SENTRY_DSN: string; +} + +export default Sentry.withSentry( + (env: Env) => ({ + dsn: env.SENTRY_DSN, + tracesSampleRate: 1.0, + traceLifecycle: 'static', + enableOpenTelemetrySetup: true, + }), + { + async fetch() { + const tracer = trace.getTracer('integration-test-tracer'); + + const inactive = tracer.startSpan('otel inactive', { attributes: { 'test.attribute': 'inactive' } }); + inactive.end(); + + await tracer.startActiveSpan( + 'otel parent', + { kind: SpanKind.CLIENT, attributes: { 'test.attribute': 'parent' } }, + async otelParent => { + await Sentry.startSpan({ name: 'sentry child' }, async () => { + const otelGrandchild = tracer.startSpan('otel grandchild', { + attributes: { 'test.attribute': 'grandchild' }, + }); + otelGrandchild.end(); + }); + + otelParent.end(); + }, + ); + + return new Response('ok'); + }, + } satisfies ExportedHandler, +); diff --git a/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/test.ts b/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/test.ts new file mode 100644 index 000000000000..b189ae79521f --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/test.ts @@ -0,0 +1,100 @@ +import { SENTRY_KIND, SENTRY_ORIGIN, SENTRY_SOURCE } from '@sentry/conventions/attributes'; +import type { Event } from '@sentry/core'; +import { expect, it } from 'vitest'; +import { SHORT_UUID_MATCHER, UUID_MATCHER } from '../../../../expect'; +import { createRunner } from '../../../../runner'; + +// One request produces two transactions, so both are asserted in a single test — every +// `createRunner` spawns its own `wrangler dev`, and a cold workerd boot dwarfs the assertions. +// Each callback leads with the transaction name: in unordered mode the runner matches an envelope +// by trying the callbacks until one does not throw, so that first assertion selects the envelope. +it('captures spans emitted through @opentelemetry/api and nests them with Sentry spans', async ({ signal }) => { + let requestTraceId: string | undefined; + let otelTraceId: string | undefined; + + const runner = createRunner(__dirname) + .expect(envelope => { + const event = envelope[1]?.[0]?.[1] as Event; + expect(event.transaction).toBe('GET /'); + + // An inactive OTel span carries no explicit context, so the tracer falls back to the Sentry + // active span — the incoming request span — and the span lands in the request transaction. + expect(event.spans).toEqual([ + { + data: { [SENTRY_ORIGIN]: 'manual', 'test.attribute': 'inactive' }, + description: 'otel inactive', + parent_span_id: event.contexts?.trace?.span_id, + span_id: SHORT_UUID_MATCHER, + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: event.contexts?.trace?.trace_id, + origin: 'manual', + }, + ]); + + requestTraceId = event.contexts?.trace?.trace_id; + }) + .expect(envelope => { + const event = envelope[1]?.[0]?.[1] as Event; + expect(event.transaction).toBe('otel parent'); + + // `startActiveSpan` always resolves an explicit context, and Cloudflare installs no OTel + // context manager — so `context.active()` never carries the request span and the span becomes + // a root of its own. The absent `parent_span_id` below is what proves the detachment. + expect(event.contexts?.trace).toEqual({ + data: { + [SENTRY_ORIGIN]: 'manual', + [SENTRY_SOURCE]: 'custom', + 'sentry.sample_rate': 1, + [SENTRY_KIND]: 'client', + 'test.attribute': 'parent', + }, + span_id: SHORT_UUID_MATCHER, + trace_id: UUID_MATCHER, + status: 'ok', + origin: 'manual', + }); + + // Below that root the two APIs interleave correctly: the tracer publishes its active span on + // the Sentry scope, so the Sentry span picks it up as parent and the next OTel span picks up + // the Sentry one in turn. Spans are ordered by start time, so `sentry child` comes first. + const sentryChildSpanId = event.spans?.[0]?.span_id; + + expect(event.spans).toEqual([ + { + data: { [SENTRY_ORIGIN]: 'manual' }, + description: 'sentry child', + parent_span_id: event.contexts?.trace?.span_id, + span_id: SHORT_UUID_MATCHER, + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: event.contexts?.trace?.trace_id, + origin: 'manual', + }, + { + data: { [SENTRY_ORIGIN]: 'manual', 'test.attribute': 'grandchild' }, + description: 'otel grandchild', + parent_span_id: sentryChildSpanId, + span_id: SHORT_UUID_MATCHER, + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: event.contexts?.trace?.trace_id, + origin: 'manual', + }, + ]); + + otelTraceId = event.contexts?.trace?.trace_id; + }) + .unordered() + .start(signal); + + await runner.makeRequest('get', '/'); + await runner.completed(); + + // Detaching from the request span does not start a new trace — both transactions stay together. + expect(requestTraceId).toEqual(UUID_MATCHER); + expect(otelTraceId).toBe(requestTraceId); +}); diff --git a/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/wrangler.jsonc b/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/wrangler.jsonc new file mode 100644 index 000000000000..8767a6a48cde --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/wrangler.jsonc @@ -0,0 +1,6 @@ +{ + "name": "cloudflare-opentelemetry-tracer", + "main": "index.ts", + "compatibility_date": "2025-06-17", + "compatibility_flags": ["nodejs_compat"], +} diff --git a/docs/migration/v11-end-state.md b/docs/migration/v11-end-state.md index 7ebb9724d7ad..532b286ea63d 100644 --- a/docs/migration/v11-end-state.md +++ b/docs/migration/v11-end-state.md @@ -633,6 +633,8 @@ Attribute availability remains runtime-dependent. For example, browser and Worke - The `code.filepath` and `code.function` span attributes on `ui.long_animation_frame` spans were renamed to `code.file.path` and `code.function.name`. - The `fs_error` span attribute on `file` spans was replaced by `error.type`. The value changed from the full error message to just the syscall's error code instead (`ENOENT`). +- The Cloudflare-specific `sentry.cloudflare_tracer` span attribute is no longer set. `@sentry/cloudflare` now creates spans through the shared `SentryTracerProvider`, so spans emitted via `@opentelemetry/api` no longer carry a marker distinguishing them from other Sentry spans. +- Span attributes now use the shared `@sentry/conventions` package under the hood. #### Attribute constants diff --git a/packages/cloudflare/package.json b/packages/cloudflare/package.json index 12f8391ca1f0..db4b7374af76 100644 --- a/packages/cloudflare/package.json +++ b/packages/cloudflare/package.json @@ -50,6 +50,7 @@ "@opentelemetry/api": "^1.9.1", "@sentry/conventions": "^0.20.0", "@sentry/core": "10.67.0", + "@sentry/opentelemetry": "10.67.0", "@sentry/server-utils": "10.67.0", "magic-string": "~0.30.21" }, diff --git a/packages/cloudflare/src/baseSdk.ts b/packages/cloudflare/src/baseSdk.ts index 4ffc248aad1a..824cff4ba87d 100644 --- a/packages/cloudflare/src/baseSdk.ts +++ b/packages/cloudflare/src/baseSdk.ts @@ -18,7 +18,6 @@ import { makeFlushLock } from './flush'; import { fetchIntegration } from './integrations/fetch'; import { httpServerIntegration } from './integrations/httpServer'; import { INTEGRATION_NAME as SPOTLIGHT_INTEGRATION_NAME, spotlightIntegration } from './integrations/spotlight'; -import { setupOpenTelemetryTracer } from './opentelemetry/tracer'; import { makeCloudflareTransport } from './transport'; import { defaultStackParser } from './vendor/stacktrace'; @@ -94,9 +93,6 @@ export function initWithDefaultIntegrations( stackParser: stackParserFromStackParserOptions(options.stackParser || defaultStackParser), integrations: getIntegrationsToSetup(options), transport: options.transport || makeCloudflareTransport, - // Like most Node-based SDKs, Cloudflare defaults to running without a Sentry OpenTelemetry tracer - // provider. Scope isolation is handled by the entrypoint wrappers' AsyncLocalStorage strategy. - enableOpenTelemetrySetup: options.enableOpenTelemetrySetup ?? false, flushLock, }; @@ -110,12 +106,6 @@ export function initWithDefaultIntegrations( } /*! rollup-include-development-only-end */ - // Opt-in only: when `enableOpenTelemetrySetup` is `true`, set up a custom trace provider so spans - // emitted via `@opentelemetry/api` are captured by Sentry. See the option's docs for the caveats. - if (clientOptions.enableOpenTelemetrySetup) { - setupOpenTelemetryTracer(); - } - const client = initAndBind(CloudflareClient, clientOptions) as CloudflareClient; // An instrumented module that first evaluates AFTER this init (e.g. a driver diff --git a/packages/cloudflare/src/opentelemetry/tracer.ts b/packages/cloudflare/src/opentelemetry/tracer.ts index bb83a8550588..e910303dd527 100644 --- a/packages/cloudflare/src/opentelemetry/tracer.ts +++ b/packages/cloudflare/src/opentelemetry/tracer.ts @@ -1,80 +1,10 @@ -import type { Context, Span, SpanOptions, Tracer, TracerProvider } from '@opentelemetry/api'; import { trace } from '@opentelemetry/api'; -import { startInactiveSpan, startSpanManual } from '@sentry/core'; +import { SentryTracerProvider } from '@sentry/opentelemetry'; /** * Set up a mock OTEL tracer to allow inter-op with OpenTelemetry emitted spans. * This is not perfect but handles easy/common use cases. */ export function setupOpenTelemetryTracer(): void { - trace.setGlobalTracerProvider(new SentryCloudflareTraceProvider()); -} - -class SentryCloudflareTraceProvider implements TracerProvider { - private readonly _tracers: Map = new Map(); - - public getTracer(name: string, version?: string, options?: { schemaUrl?: string }): Tracer { - const key = `${name}@${version || ''}:${options?.schemaUrl || ''}`; - if (!this._tracers.has(key)) { - this._tracers.set(key, new SentryCloudflareTracer()); - } - - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - return this._tracers.get(key)!; - } -} - -class SentryCloudflareTracer implements Tracer { - public startSpan(name: string, options?: SpanOptions): Span { - return startInactiveSpan({ - ...options, - name, - attributes: { - ...options?.attributes, - 'sentry.cloudflare_tracer': true, - }, - }); - } - - /** - * NOTE: This does not handle `context` being passed in. It will always put spans on the current scope. - */ - public startActiveSpan unknown>(name: string, fn: F): ReturnType; - public startActiveSpan unknown>(name: string, options: SpanOptions, fn: F): ReturnType; - public startActiveSpan unknown>( - name: string, - options: SpanOptions, - context: Context, - fn: F, - ): ReturnType; - public startActiveSpan unknown>( - name: string, - options: unknown, - context?: unknown, - fn?: F, - ): ReturnType { - const opts = (typeof options === 'object' && options !== null ? options : {}) as SpanOptions; - - const spanOpts = { - ...opts, - name, - attributes: { - ...opts.attributes, - 'sentry.cloudflare_tracer': true, - }, - }; - - const callback = ( - typeof options === 'function' - ? options - : typeof context === 'function' - ? context - : typeof fn === 'function' - ? fn - : () => {} - ) as F; - - // In OTEL the semantic matches `startSpanManual` because spans are not auto-ended - return startSpanManual(spanOpts, callback) as ReturnType; - } + trace.setGlobalTracerProvider(new SentryTracerProvider()); } diff --git a/packages/cloudflare/src/request.ts b/packages/cloudflare/src/request.ts index 07d5435e4084..cb66b7329274 100644 --- a/packages/cloudflare/src/request.ts +++ b/packages/cloudflare/src/request.ts @@ -53,7 +53,11 @@ type InitSdk = (options: CloudflareOptions) => CloudflareClient | undefined; * getDefaultIntegrations(options)` in `options` to get the full set instead. */ export function wrapRequestHandler( - wrapperOptions: RequestHandlerWrapperOptions, + wrapperOptions: Omit & { + // `enableOpenTelemetrySetup` is only honored by `init` from `sdk.ts`; this entry point + // initializes the SDK via `initBaseSdk`, where setting it would have no effect. + options: Omit; + }, handler: (...args: unknown[]) => Response | Promise, ): Promise { return wrapRequestHandlerWithInit(wrapperOptions, handler, initBaseSdk); diff --git a/packages/cloudflare/src/sdk.ts b/packages/cloudflare/src/sdk.ts index 12bcb420d788..9df22e5d9e63 100644 --- a/packages/cloudflare/src/sdk.ts +++ b/packages/cloudflare/src/sdk.ts @@ -1,6 +1,7 @@ import type { Integration } from '@sentry/core'; import { getBaseDefaultIntegrations, initWithDefaultIntegrations } from './baseSdk'; import type { CloudflareClient, CloudflareOptions } from './client'; +import { setupOpenTelemetryTracer } from './opentelemetry/tracer'; /** * Get the default integrations for the Cloudflare SDK. @@ -13,5 +14,15 @@ export function getDefaultIntegrations(options: CloudflareOptions): Integration[ * Initializes the cloudflare SDK. */ export function init(options: CloudflareOptions): CloudflareClient | undefined { + // Like most Node-based SDKs, Cloudflare defaults to running without a Sentry OpenTelemetry tracer + // provider. Scope isolation is handled by the entrypoint wrappers' AsyncLocalStorage strategy. + options.enableOpenTelemetrySetup ??= false; + + // Opt-in only: when `enableOpenTelemetrySetup` is `true`, set up a custom trace provider so spans + // emitted via `@opentelemetry/api` are captured by Sentry. See the option's docs for the caveats. + if (options.enableOpenTelemetrySetup) { + setupOpenTelemetryTracer(); + } + return initWithDefaultIntegrations(options, getDefaultIntegrations); } diff --git a/packages/cloudflare/test/opentelemetry.test.ts b/packages/cloudflare/test/opentelemetry.test.ts index 3d5d10bf5781..8e98eddefcf9 100644 --- a/packages/cloudflare/test/opentelemetry.test.ts +++ b/packages/cloudflare/test/opentelemetry.test.ts @@ -75,7 +75,6 @@ describe('opentelemetry compatibility', () => { expect(transactionEvent?.spans?.length).toBe(0); expect(transactionEvent?.transaction).toBe('test'); expect(transactionEvent?.contexts?.trace?.data).toEqual({ - 'sentry.cloudflare_tracer': true, 'sentry.origin': 'manual', 'sentry.sample_rate': 1, 'sentry.source': 'custom', @@ -84,7 +83,6 @@ describe('opentelemetry compatibility', () => { expect(transactionEvent2?.spans?.length).toBe(1); expect(transactionEvent2?.transaction).toBe('test 2'); expect(transactionEvent2?.contexts?.trace?.data).toEqual({ - 'sentry.cloudflare_tracer': true, 'sentry.origin': 'manual', 'sentry.sample_rate': 1, 'sentry.source': 'custom', @@ -95,7 +93,6 @@ describe('opentelemetry compatibility', () => { expect.objectContaining({ description: 'test 3', data: { - 'sentry.cloudflare_tracer': true, 'sentry.origin': 'manual', 'test.attribute': 'test2', }, @@ -141,7 +138,6 @@ describe('opentelemetry compatibility', () => { expect.objectContaining({ description: 'otel span', data: { - 'sentry.cloudflare_tracer': true, 'sentry.origin': 'manual', }, }), From af9a86c80a8917a6e490002055c9f72f5ed69a7b Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Fri, 14 Aug 2026 11:06:57 +0200 Subject: [PATCH 2/3] fixup! ref(cloudflare)!: Use shared `SentryTracerProvider` for OpenTelemetry interop --- .../opentelemetry-tracer/enabled/test.ts | 83 +++++++------------ packages/opentelemetry/src/tracer.ts | 9 +- 2 files changed, 37 insertions(+), 55 deletions(-) diff --git a/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/test.ts b/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/test.ts index b189ae79521f..03339b53ef53 100644 --- a/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/test.ts +++ b/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/test.ts @@ -1,76 +1,60 @@ -import { SENTRY_KIND, SENTRY_ORIGIN, SENTRY_SOURCE } from '@sentry/conventions/attributes'; +import { SENTRY_KIND, SENTRY_ORIGIN } from '@sentry/conventions/attributes'; import type { Event } from '@sentry/core'; import { expect, it } from 'vitest'; -import { SHORT_UUID_MATCHER, UUID_MATCHER } from '../../../../expect'; +import { SHORT_UUID_MATCHER } from '../../../../expect'; import { createRunner } from '../../../../runner'; -// One request produces two transactions, so both are asserted in a single test — every -// `createRunner` spawns its own `wrangler dev`, and a cold workerd boot dwarfs the assertions. -// Each callback leads with the transaction name: in unordered mode the runner matches an envelope -// by trying the callbacks until one does not throw, so that first assertion selects the envelope. it('captures spans emitted through @opentelemetry/api and nests them with Sentry spans', async ({ signal }) => { - let requestTraceId: string | undefined; - let otelTraceId: string | undefined; - const runner = createRunner(__dirname) .expect(envelope => { const event = envelope[1]?.[0]?.[1] as Event; expect(event.transaction).toBe('GET /'); - // An inactive OTel span carries no explicit context, so the tracer falls back to the Sentry - // active span — the incoming request span — and the span lands in the request transaction. + const requestSpanId = event.contexts?.trace?.span_id; + const traceId = event.contexts?.trace?.trace_id; + + // Cloudflare installs no OTel context manager, so `context.active()` never carries a span. + // Neither tracer API passes an explicit context here, so both fall back to the Sentry active + // span — the incoming request span — and everything stays in the request transaction. + const otelParentSpanId = event.spans?.[1]?.span_id; + const sentryChildSpanId = event.spans?.[2]?.span_id; + + // Spans are ordered by start time. expect(event.spans).toEqual([ { data: { [SENTRY_ORIGIN]: 'manual', 'test.attribute': 'inactive' }, description: 'otel inactive', - parent_span_id: event.contexts?.trace?.span_id, + parent_span_id: requestSpanId, span_id: SHORT_UUID_MATCHER, start_timestamp: expect.any(Number), status: 'ok', timestamp: expect.any(Number), - trace_id: event.contexts?.trace?.trace_id, + trace_id: traceId, origin: 'manual', }, - ]); - - requestTraceId = event.contexts?.trace?.trace_id; - }) - .expect(envelope => { - const event = envelope[1]?.[0]?.[1] as Event; - expect(event.transaction).toBe('otel parent'); - - // `startActiveSpan` always resolves an explicit context, and Cloudflare installs no OTel - // context manager — so `context.active()` never carries the request span and the span becomes - // a root of its own. The absent `parent_span_id` below is what proves the detachment. - expect(event.contexts?.trace).toEqual({ - data: { - [SENTRY_ORIGIN]: 'manual', - [SENTRY_SOURCE]: 'custom', - 'sentry.sample_rate': 1, - [SENTRY_KIND]: 'client', - 'test.attribute': 'parent', + { + data: { [SENTRY_ORIGIN]: 'manual', [SENTRY_KIND]: 'client', 'test.attribute': 'parent' }, + description: 'otel parent', + parent_span_id: requestSpanId, + span_id: SHORT_UUID_MATCHER, + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: traceId, + origin: 'manual', }, - span_id: SHORT_UUID_MATCHER, - trace_id: UUID_MATCHER, - status: 'ok', - origin: 'manual', - }); - - // Below that root the two APIs interleave correctly: the tracer publishes its active span on - // the Sentry scope, so the Sentry span picks it up as parent and the next OTel span picks up - // the Sentry one in turn. Spans are ordered by start time, so `sentry child` comes first. - const sentryChildSpanId = event.spans?.[0]?.span_id; - - expect(event.spans).toEqual([ + // Below the OTel parent the two APIs interleave correctly: the tracer publishes its active + // span on the Sentry scope, so the Sentry span picks it up as parent and the next OTel span + // picks up the Sentry one in turn. { data: { [SENTRY_ORIGIN]: 'manual' }, description: 'sentry child', - parent_span_id: event.contexts?.trace?.span_id, + parent_span_id: otelParentSpanId, span_id: SHORT_UUID_MATCHER, start_timestamp: expect.any(Number), status: 'ok', timestamp: expect.any(Number), - trace_id: event.contexts?.trace?.trace_id, + trace_id: traceId, origin: 'manual', }, { @@ -81,20 +65,13 @@ it('captures spans emitted through @opentelemetry/api and nests them with Sentry start_timestamp: expect.any(Number), status: 'ok', timestamp: expect.any(Number), - trace_id: event.contexts?.trace?.trace_id, + trace_id: traceId, origin: 'manual', }, ]); - - otelTraceId = event.contexts?.trace?.trace_id; }) - .unordered() .start(signal); await runner.makeRequest('get', '/'); await runner.completed(); - - // Detaching from the request span does not start a new trace — both transactions stay together. - expect(requestTraceId).toEqual(UUID_MATCHER); - expect(otelTraceId).toBe(requestTraceId); }); diff --git a/packages/opentelemetry/src/tracer.ts b/packages/opentelemetry/src/tracer.ts index b70e91ae7102..f72aac2f200f 100644 --- a/packages/opentelemetry/src/tracer.ts +++ b/packages/opentelemetry/src/tracer.ts @@ -63,12 +63,17 @@ export class SentryTracer implements Tracer { fn?: F, ): ReturnType { const options = typeof optionsOrFn === 'function' ? {} : optionsOrFn; - const ctx = typeof contextOrFn === 'function' || contextOrFn === undefined ? context.active() : contextOrFn; + const explicitCtx = typeof contextOrFn === 'function' || contextOrFn === undefined ? undefined : contextOrFn; + const ctx = explicitCtx ?? context.active(); const callback = ( typeof optionsOrFn === 'function' ? optionsOrFn : typeof contextOrFn === 'function' ? contextOrFn : fn ) as F; - const span = this.startSpan(name, options, ctx); + // Only forward a context the caller actually passed. Forwarding the resolved `context.active()` + // instead would read as an explicit root request on runtimes without an OTel context manager + // (e.g. Cloudflare), where `context.active()` is always `ROOT_CONTEXT`, detaching every span + // from the Sentry active span instead of nesting under it. + const span = this.startSpan(name, options, explicitCtx); // Run the span's callback under the isolation scope captured when the span was created, so scope state // used or set during the span (tags, breadcrumbs, captured errors) belongs to that span and stays From 1377b41f1f78e125b161cabfc555a18d76941827 Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Fri, 14 Aug 2026 11:06:57 +0200 Subject: [PATCH 3/3] fix(opentelemetry): Scope startActiveSpan without an OTel context manager Without a context manager (e.g. Cloudflare) `context.with` runs the callback directly, so the span set on the current scope leaked past the callback and later spans parented to it. Fork the scope in that case, and detect the parent of an ignored span through `getActiveSpan()` so it is not made active either. --- .../opentelemetry-tracer/enabled/index.ts | 5 + .../opentelemetry-tracer/enabled/test.ts | 25 +++++ .../cloudflare/test/opentelemetry.test.ts | 104 +++++++++++++++++- packages/opentelemetry/src/tracer.ts | 27 ++++- 4 files changed, 159 insertions(+), 2 deletions(-) diff --git a/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/index.ts b/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/index.ts index 2e95710a8d79..5cd8e1547a7e 100644 --- a/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/index.ts +++ b/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/index.ts @@ -34,6 +34,11 @@ export default Sentry.withSentry( }, ); + // Neither span may attach to the finished `otel parent` once `startActiveSpan` has returned. + const otelAfter = tracer.startSpan('otel after active', { attributes: { 'test.attribute': 'after' } }); + otelAfter.end(); + await Sentry.startSpan({ name: 'sentry after active' }, async () => {}); + return new Response('ok'); }, } satisfies ExportedHandler, diff --git a/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/test.ts b/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/test.ts index 03339b53ef53..564dd31f7684 100644 --- a/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/test.ts +++ b/dev-packages/cloudflare-integration-tests/suites/tracing/opentelemetry-tracer/enabled/test.ts @@ -68,6 +68,31 @@ it('captures spans emitted through @opentelemetry/api and nests them with Sentry trace_id: traceId, origin: 'manual', }, + // Without an OTel context manager `context.with` cannot restore anything, so the tracer has to + // fork the scope itself. Otherwise the finished `otel parent` would stay active and both of + // these would hang off it instead of the request span. + { + data: { [SENTRY_ORIGIN]: 'manual', 'test.attribute': 'after' }, + description: 'otel after active', + parent_span_id: requestSpanId, + span_id: SHORT_UUID_MATCHER, + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: traceId, + origin: 'manual', + }, + { + data: { [SENTRY_ORIGIN]: 'manual' }, + description: 'sentry after active', + parent_span_id: requestSpanId, + span_id: SHORT_UUID_MATCHER, + start_timestamp: expect.any(Number), + status: 'ok', + timestamp: expect.any(Number), + trace_id: traceId, + origin: 'manual', + }, ]); }) .start(signal); diff --git a/packages/cloudflare/test/opentelemetry.test.ts b/packages/cloudflare/test/opentelemetry.test.ts index 8e98eddefcf9..68726f824139 100644 --- a/packages/cloudflare/test/opentelemetry.test.ts +++ b/packages/cloudflare/test/opentelemetry.test.ts @@ -1,6 +1,6 @@ import { trace } from '@opentelemetry/api'; import type { TransactionEvent } from '@sentry/core'; -import { startSpan } from '@sentry/core'; +import { getActiveSpan, spanToJSON, startSpan } from '@sentry/core'; import { beforeEach, describe, expect, test } from 'vitest'; import { init } from '../src/sdk'; import { resetSdk } from './testUtils'; @@ -202,4 +202,106 @@ describe('opentelemetry compatibility', () => { expect(transactionEvent?.transaction).toBe('prisma:client:operation'); }); + + test('startActiveSpan does not leave the span active after the callback returns', async () => { + const transactionEvents: TransactionEvent[] = []; + + const client = init({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + traceLifecycle: 'static', + enableOpenTelemetrySetup: true, + beforeSendTransaction: event => { + transactionEvents.push(event); + return null; + }, + }); + + const tracer = trace.getTracer('test'); + + tracer.startActiveSpan('otel span', span => { + expect(getActiveSpan()).toBe(span); + span.end(); + }); + + expect(getActiveSpan()).toBeUndefined(); + + startSpan({ name: 'sentry span' }, () => {}); + + await client!.flush(); + + expect(transactionEvents).toHaveLength(2); + const [otelEvent, sentryEvent] = transactionEvents; + + expect(otelEvent?.transaction).toBe('otel span'); + expect(sentryEvent?.transaction).toBe('sentry span'); + expect(sentryEvent?.contexts?.trace?.parent_span_id).toBeUndefined(); + }); + + test('startActiveSpan restores the previously active span after the callback returns', async () => { + const transactionEvents: TransactionEvent[] = []; + + const client = init({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + traceLifecycle: 'static', + enableOpenTelemetrySetup: true, + beforeSendTransaction: event => { + transactionEvents.push(event); + return null; + }, + }); + + const tracer = trace.getTracer('test'); + + startSpan({ name: 'sentry span' }, parent => { + tracer.startActiveSpan('otel span', span => { + span.end(); + }); + + expect(getActiveSpan()).toBe(parent); + + startSpan({ name: 'sentry child' }, () => {}); + }); + + await client!.flush(); + + expect(transactionEvents).toHaveLength(1); + const [transactionEvent] = transactionEvents; + + expect(transactionEvent?.transaction).toBe('sentry span'); + expect(transactionEvent?.spans).toHaveLength(2); + const rootSpanId = transactionEvent?.contexts?.trace?.span_id; + expect(transactionEvent?.spans?.map(span => [span.description, span.parent_span_id])).toEqual([ + ['otel span', rootSpanId], + ['sentry child', rootSpanId], + ]); + }); + + test('ignored startActiveSpan child does not become active', () => { + init({ + dsn: 'https://username@domain/123', + tracesSampleRate: 1, + traceLifecycle: 'stream', + enableOpenTelemetrySetup: true, + ignoreSpans: ['ignored span'], + }); + + const tracer = trace.getTracer('test'); + + startSpan({ name: 'sentry span' }, parent => { + tracer.startActiveSpan('ignored span', span => { + expect(span.isRecording()).toBe(false); + expect(getActiveSpan()).toBe(parent); + + const child = tracer.startSpan('child span'); + expect(child.isRecording()).toBe(true); + expect(spanToJSON(child).parent_span_id).toBe(parent.spanContext().spanId); + child.end(); + span.end(); + }); + + expect(getActiveSpan()).toBe(parent); + }); + }); }); diff --git a/packages/opentelemetry/src/tracer.ts b/packages/opentelemetry/src/tracer.ts index f72aac2f200f..4d41f205b4ac 100644 --- a/packages/opentelemetry/src/tracer.ts +++ b/packages/opentelemetry/src/tracer.ts @@ -5,6 +5,7 @@ import { _INTERNAL_setSpanForScope, startInactiveSpan, addChildSpanToSpan, + getActiveSpan, getCapturedScopesOnSpan, getCurrentScope, getIsolationScope, @@ -14,6 +15,7 @@ import { spanIsIgnored, spanKindToName, startNewTrace, + withScope, } from '@sentry/core'; import type { Span, SpanAttributes } from '@sentry/core'; import { SENTRY_FORK_SET_ISOLATION_SCOPE_CONTEXT_KEY } from './constants'; @@ -90,16 +92,39 @@ export class SentryTracer implements Tracer { // along with it (cascading the drop down the whole subtree). Leaving the parent active lets the // children attach to it and get re-parented instead. An ignored root span has no parent and still // becomes active, so its subtree is dropped as intended. - if (spanIsIgnored(span) && trace.getSpan(ctx)) { + if (spanIsIgnored(span) && this._hasParentSpan(options, explicitCtx)) { return context.with(withCapturedIsolationScope(ctx), () => callback(span)) as ReturnType; } return context.with(withCapturedIsolationScope(trace.setSpan(ctx, span)), () => { + // Without an OTel context manager (e.g. Cloudflare) `context.with` runs the callback directly and + // nothing forks the scope, so setting the span on the current scope would leak it past the callback. + // Fork explicitly in that case so the previously active span is restored afterwards. + if (trace.getSpan(context.active()) !== span) { + return withScope(scope => { + _INTERNAL_setSpanForScope(scope, span); + return callback(span) as ReturnType; + }); + } + _INTERNAL_setSpanForScope(getCurrentScope(), span); return callback(span) as ReturnType; }); } + /** + * Whether a span started with these arguments gets a parent. Mirrors the parent lookup in `startSpan` + * plus core's fallback to the scope's active span, which is what parents the span on runtimes without an + * OTel context manager. + */ + private _hasParentSpan(options: SpanOptions, explicitCtx: Context | undefined): boolean { + if (options.root) { + return false; + } + const parentSpan = explicitCtx ? trace.getSpan(explicitCtx) : getActiveSpan(); + return !!parentSpan && isSpanContextValid(parentSpan.spanContext()); + } + private _startSentrySpan( name: string, options: SpanOptions,