Skip to content
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: 0 additions & 9 deletions packages/cloudflare/src/async.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,7 @@ 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
18 changes: 18 additions & 0 deletions packages/core/src/asyncContext/stackStrategy.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { Client } from '../client';
import { getDefaultCurrentScope, getDefaultIsolationScope } from '../defaultScopes';
import { Scope } from '../scope';
import { SUPPRESS_TRACING_KEY } from '../tracing/constants';
import { chainAndCopyPromiseLike } from '../utils/chain-and-copy-promiselike';
import { isThenable } from '../utils/is';
import { getMainCarrier, getSentryCarrier } from './../carrier';
Expand Down Expand Up @@ -148,6 +149,7 @@ function withIsolationScope<T>(callback: (isolationScope: Scope) => T): T {
*/
export function getStackAsyncContextStrategy(): AsyncContextStrategy {
return {
suppressTracing: suppressTracingInStack,
withIsolationScope,
withScope,
withSetScope,
Expand All @@ -158,3 +160,19 @@ export function getStackAsyncContextStrategy(): AsyncContextStrategy {
getIsolationScope: () => getAsyncContextStack().getIsolationScope(),
};
}

/**
* In stack-based ACS, we do not wait for the callback to finish before we reset the metadata
* the reason for this is that otherwise, in the stack 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 stack.
*/
function suppressTracingInStack<T>(callback: () => T): T {
return withScope(scope => {
scope.setSDKProcessingMetadata({ [SUPPRESS_TRACING_KEY]: true });
const res = callback();
scope.setSDKProcessingMetadata({ [SUPPRESS_TRACING_KEY]: undefined });
return res;
});
}
6 changes: 3 additions & 3 deletions packages/core/src/integrations/http/client-subscriptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ import { addOutgoingRequestBreadcrumb } from './add-outgoing-request-breadcrumb'
import {
bindScopeToEmitter,
getSpanStatusFromHttpCode,
isTracingSuppressed,
SPAN_STATUS_ERROR,
SPAN_STATUS_UNSET,
startInactiveSpan,
SUPPRESS_TRACING_KEY,
withActiveSpan,
} from '../../tracing';
import { debug } from '../../utils/debug-logger';
Expand All @@ -33,7 +33,7 @@ import type { HttpInstrumentationOptions, HttpClientRequest, HttpIncomingMessage
import { DEBUG_BUILD } from '../../debug-build';
import { LOG_PREFIX, HTTP_ON_CLIENT_REQUEST } from './constants';
import type { ClientSubscriptionName } from './constants';
import { getClient, getCurrentScope } from '../../currentScopes';
import { getClient } from '../../currentScopes';
import { hasSpansEnabled } from '../../utils/hasSpansEnabled';
import { doubleWrapWarning } from './double-wrap-warning';

Expand All @@ -48,7 +48,7 @@ export function getHttpClientSubscriptions(options: HttpInstrumentationOptions):
const onHttpClientRequestCreated: ChannelListener = (data: unknown): void => {
// Skip all instrumentation if tracing is suppressed
// (e.g., Sentry's own transport uses this to avoid self-instrumentation)
if (getCurrentScope().getScopeData().sdkProcessingMetadata[SUPPRESS_TRACING_KEY] === true) {
if (isTracingSuppressed()) {
return;
}

Expand Down
1 change: 1 addition & 0 deletions packages/core/src/tracing/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const SUPPRESS_TRACING_KEY = '__SENTRY_SUPPRESS_TRACING__';
1 change: 0 additions & 1 deletion packages/core/src/tracing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ export {
isTracingSuppressed,
startNewTrace,
spanIsIgnored,
SUPPRESS_TRACING_KEY,
} from './trace';
export { bindScopeToEmitter } from './bindScopeToEmitter';
export {
Expand Down
12 changes: 2 additions & 10 deletions packages/core/src/tracing/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ import { SentrySpan } from './sentrySpan';
import { SPAN_STATUS_ERROR } from './spanstatus';
import { setCapturedScopesOnSpan } from './utils';
import type { Client } from '../client';

export const SUPPRESS_TRACING_KEY = '__SENTRY_SUPPRESS_TRACING__';
import { SUPPRESS_TRACING_KEY } from './constants';

/**
* Wraps a function with a transaction/span and finishes the span after the function is done.
Expand Down Expand Up @@ -293,15 +292,8 @@ 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 });
const res = callback();
scope.setSDKProcessingMetadata({ [SUPPRESS_TRACING_KEY]: undefined });
return res;
return callback();
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as breadcrumbModule from '../../../../src/integrations/http/add-outgoin
import { HTTP_ON_CLIENT_REQUEST } from '../../../../src/integrations/http/constants';
import { getHttpClientSubscriptions } from '../../../../src/integrations/http/client-subscriptions';
import type { HttpClientRequest, HttpIncomingMessage } from '../../../../src/integrations/http/types';
import { SUPPRESS_TRACING_KEY } from '../../../../src/tracing';
import { SUPPRESS_TRACING_KEY } from '../../../../src/tracing/constants';
import { getCurrentScope, withScope } from '../../../../src/currentScopes';

function makeMockRequest(): HttpClientRequest & {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/lib/tracing/trace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import {
startInactiveSpan,
startSpan,
startSpanManual,
SUPPRESS_TRACING_KEY,
suppressTracing,
withActiveSpan,
} from '../../../src/tracing';
Expand All @@ -37,6 +36,7 @@ import type { StartSpanOptions } from '../../../src/types/startSpanOptions';
import { _setSpanForScope } from '../../../src/utils/spanOnScope';
import { getActiveSpan, getRootSpan, getSpanDescendants, spanIsSampled } from '../../../src/utils/spanUtils';
import { getDefaultTestClientOptions, TestClient } from '../../mocks/client';
import { SUPPRESS_TRACING_KEY } from '../../../src/tracing/constants';

const enum Type {
Sync = 'sync',
Expand Down
9 changes: 0 additions & 9 deletions packages/deno/src/async.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,16 +77,7 @@ 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
5 changes: 4 additions & 1 deletion packages/node/src/integrations/fs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ export const fsIntegration = defineIntegration((options: FsInstrumentationConfig
return {
name: INTEGRATION_NAME,
setupOnce() {
enableFsInstrumentation(options);
// We only run this in the next tick to avoid instrumenting the `fs` module while the SDK is initializating
// especially, at the point when this runs the Async Context Manager may not be set up yet,
// which could lead to weird outcomes - so we wait until everything is settled before we instrument.
setImmediate(() => enableFsInstrumentation(options));
},
};
});
3 changes: 0 additions & 3 deletions packages/opentelemetry/src/asyncContextStrategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import type { CurrentScopes } from './types';
import { getContextFromScope, getScopesFromContext } from './utils/contextData';
import { getActiveSpan } from './utils/getActiveSpan';
import { getTraceData } from './utils/getTraceData';
import { suppressTracing, isTracingSuppressed } from './utils/suppressTracing';

/**
* Sets the async context strategy to use follow the OTEL context under the hood.
Expand Down Expand Up @@ -103,8 +102,6 @@ export function setOpenTelemetryContextAsyncContextStrategy(options?: {
startSpanManual,
startInactiveSpan,
getActiveSpan,
suppressTracing,
isTracingSuppressed,
Comment thread
cursor[bot] marked this conversation as resolved.
getTraceData,
continueTrace,
startNewTrace,
Comment on lines 102 to 107

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Removing the custom suppressTracing implementation for OTEL causes Sentry.suppressTracing() to no longer suppress OTEL-native span creation, as it now only sets Sentry-scope metadata.
Severity: HIGH

Suggested Fix

Reinstate the custom suppressTracing implementation in the OTEL async context strategy (asyncContextStrategy.ts). This implementation should call the native OpenTelemetry suppressTracing() function from @opentelemetry/core to ensure the suppression signal is correctly propagated within the OTEL context, which is what the OTEL tracer and propagator check.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/opentelemetry/src/asyncContextStrategy.ts#L102-L107

Potential issue: In OpenTelemetry environments, the removal of the custom
`suppressTracing` implementation in `asyncContextStrategy.ts` causes a fallback to the
default Sentry implementation. This default only sets a `SUPPRESS_TRACING_KEY` in the
Sentry scope metadata. However, the OTEL tracer and propagator check for suppression
using OTEL's native context mechanism from `@opentelemetry/core`. Because the Sentry
metadata is not checked by OTEL, calling `Sentry.suppressTracing()` will fail to prevent
the creation of OTEL-native spans and the propagation of trace headers.

Did we get this right? 👍 / 👎 to inform future reviews.

Expand Down
2 changes: 0 additions & 2 deletions packages/opentelemetry/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ export { enhanceDscWithOpenTelemetryRootSpanName } from './utils/enhanceDscWithO

export { getTraceContextForScope } from './trace';

export { suppressTracing } from './utils/suppressTracing';
Comment thread
cursor[bot] marked this conversation as resolved.

export { setupEventContextTrace } from './setupEventContextTrace';

// eslint-disable-next-line typescript/no-deprecated
Expand Down
18 changes: 0 additions & 18 deletions packages/opentelemetry/src/utils/suppressTracing.ts

This file was deleted.

Loading