Skip to content

fix(browser): Ensure suppressTracing does not leak when async #16545

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/cloudflare/src/async.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,16 @@ export function setAsyncLocalStorageAsyncContextStrategy(): void {
});
}

// In contrast to the browser, we can rely on async context isolation here
function suppressTracing<T>(callback: () => T): T {
return withScope(scope => {
scope.setSDKProcessingMetadata({ __SENTRY_SUPPRESS_TRACING__: true });
return callback();
});
}

setAsyncContextStrategy({
suppressTracing,
withScope,
withSetScope,
withIsolationScope,
Expand Down
9 changes: 8 additions & 1 deletion packages/core/src/tracing/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -253,8 +253,15 @@ export function suppressTracing<T>(callback: () => T): T {
}

return withScope(scope => {
// Note: We do not wait for the callback to finish before we reset the metadata
// the reason for this is that otherwise, in the browser this can lead to very weird behavior
// as there is only a single top scope, if the callback takes longer to finish,
// other, unrelated spans may also be suppressed, which we do not want
// so instead, we only suppress tracing synchronoysly in the browser
scope.setSDKProcessingMetadata({ [SUPPRESS_TRACING_KEY]: true });
return callback();
const res = callback();
scope.setSDKProcessingMetadata({ [SUPPRESS_TRACING_KEY]: undefined });
return res;
});
}

Expand Down
34 changes: 34 additions & 0 deletions packages/core/test/lib/tracing/trace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1975,6 +1975,40 @@ describe('suppressTracing', () => {
expect(spanIsSampled(child)).toBe(false);
});
});

it('works with parallel processes', async () => {
const span = suppressTracing(() => {
return startInactiveSpan({ name: 'span' });
});

// Note: This is unintuitive, but it is the expected behavior
// because we only suppress tracing synchronously in the browser
const span2Promise = suppressTracing(async () => {
await new Promise(resolve => setTimeout(resolve, 100));
return startInactiveSpan({ name: 'span2' });
});

const span3Promise = suppressTracing(async () => {
const span = startInactiveSpan({ name: 'span3' });
await new Promise(resolve => setTimeout(resolve, 100));
return span;
});

const span4 = suppressTracing(() => {
return startInactiveSpan({ name: 'span' });
});

const span5 = startInactiveSpan({ name: 'span5' });

const span2 = await span2Promise;
const span3 = await span3Promise;

expect(spanIsSampled(span)).toBe(false);
expect(spanIsSampled(span2)).toBe(true);
expect(spanIsSampled(span3)).toBe(false);
expect(spanIsSampled(span4)).toBe(false);
expect(spanIsSampled(span5)).toBe(true);
});
});

describe('startNewTrace', () => {
Expand Down
32 changes: 32 additions & 0 deletions packages/opentelemetry/test/trace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1921,6 +1921,38 @@ describe('suppressTracing', () => {
expect(spanIsSampled(child)).toBe(false);
});
});

it('works with parallel processes', async () => {
const span = suppressTracing(() => {
return startInactiveSpan({ name: 'span' });
});

const span2Promise = suppressTracing(async () => {
await new Promise(resolve => setTimeout(resolve, 100));
return startInactiveSpan({ name: 'span2' });
});

const span3Promise = suppressTracing(async () => {
const span = startInactiveSpan({ name: 'span3' });
await new Promise(resolve => setTimeout(resolve, 100));
return span;
});

const span4 = suppressTracing(() => {
return startInactiveSpan({ name: 'span' });
});

const span5 = startInactiveSpan({ name: 'span5' });

const span2 = await span2Promise;
const span3 = await span3Promise;

expect(spanIsSampled(span)).toBe(false);
expect(spanIsSampled(span2)).toBe(false);
expect(spanIsSampled(span3)).toBe(false);
expect(spanIsSampled(span4)).toBe(false);
expect(spanIsSampled(span5)).toBe(true);
});
});

function getSpanName(span: AbstractSpan): string | undefined {
Expand Down
Loading