Skip to content

Commit 8a9a722

Browse files
committed
fix(browser): Ensure suppressTracing does not leak when async
1 parent a72ca10 commit 8a9a722

File tree

4 files changed

+84
-1
lines changed

4 files changed

+84
-1
lines changed

packages/cloudflare/src/async.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,16 @@ export function setAsyncLocalStorageAsyncContextStrategy(): void {
6464
});
6565
}
6666

67+
// In contrast to the browser, we can rely on async context isolation here
68+
function suppressTracing<T>(callback: () => T): T {
69+
return withScope(scope => {
70+
scope.setSDKProcessingMetadata({ __SENTRY_SUPPRESS_TRACING__: true });
71+
return callback();
72+
});
73+
}
74+
6775
setAsyncContextStrategy({
76+
suppressTracing,
6877
withScope,
6978
withSetScope,
7079
withIsolationScope,

packages/core/src/tracing/trace.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,8 +253,15 @@ export function suppressTracing<T>(callback: () => T): T {
253253
}
254254

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

packages/core/test/lib/tracing/trace.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1975,6 +1975,40 @@ describe('suppressTracing', () => {
19751975
expect(spanIsSampled(child)).toBe(false);
19761976
});
19771977
});
1978+
1979+
it('works with parallel processes', async () => {
1980+
const span = suppressTracing(() => {
1981+
return startInactiveSpan({ name: 'span' });
1982+
});
1983+
1984+
// Note: This is unintuitive, but it is the expected behavior
1985+
// because we only suppress tracing synchronously in the browser
1986+
const span2Promise = suppressTracing(async () => {
1987+
await new Promise(resolve => setTimeout(resolve, 100));
1988+
return startInactiveSpan({ name: 'span2' });
1989+
});
1990+
1991+
const span3Promise = suppressTracing(async () => {
1992+
const span = startInactiveSpan({ name: 'span3' });
1993+
await new Promise(resolve => setTimeout(resolve, 100));
1994+
return span;
1995+
});
1996+
1997+
const span4 = suppressTracing(() => {
1998+
return startInactiveSpan({ name: 'span' });
1999+
});
2000+
2001+
const span5 = startInactiveSpan({ name: 'span5' });
2002+
2003+
const span2 = await span2Promise;
2004+
const span3 = await span3Promise;
2005+
2006+
expect(spanIsSampled(span)).toBe(false);
2007+
expect(spanIsSampled(span2)).toBe(true);
2008+
expect(spanIsSampled(span3)).toBe(false);
2009+
expect(spanIsSampled(span4)).toBe(false);
2010+
expect(spanIsSampled(span5)).toBe(true);
2011+
});
19782012
});
19792013

19802014
describe('startNewTrace', () => {

packages/opentelemetry/test/trace.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1921,6 +1921,39 @@ describe('suppressTracing', () => {
19211921
expect(spanIsSampled(child)).toBe(false);
19221922
});
19231923
});
1924+
1925+
it('works with parallel processes', async () => {
1926+
const span = suppressTracing(() => {
1927+
return startInactiveSpan({ name: 'span' });
1928+
});
1929+
1930+
1931+
const span2Promise = suppressTracing(async () => {
1932+
await new Promise(resolve => setTimeout(resolve, 100));
1933+
return startInactiveSpan({ name: 'span2' });
1934+
});
1935+
1936+
const span3Promise = suppressTracing(async () => {
1937+
const span = startInactiveSpan({ name: 'span3' });
1938+
await new Promise(resolve => setTimeout(resolve, 100));
1939+
return span;
1940+
});
1941+
1942+
const span4 = suppressTracing(() => {
1943+
return startInactiveSpan({ name: 'span' });
1944+
});
1945+
1946+
const span5 = startInactiveSpan({ name: 'span5' });
1947+
1948+
const span2 = await span2Promise;
1949+
const span3 = await span3Promise;
1950+
1951+
expect(spanIsSampled(span)).toBe(false);
1952+
expect(spanIsSampled(span2)).toBe(false);
1953+
expect(spanIsSampled(span3)).toBe(false);
1954+
expect(spanIsSampled(span4)).toBe(false);
1955+
expect(spanIsSampled(span5)).toBe(true);
1956+
});
19241957
});
19251958

19261959
function getSpanName(span: AbstractSpan): string | undefined {

0 commit comments

Comments
 (0)