diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index dff11499285b..c227dda5057a 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -1,13 +1,14 @@ import type { Hub, Scope, Span, SpanTimeInput, StartSpanOptions, TransactionContext } from '@sentry/types'; import { addNonEnumerableProperty, dropUndefinedKeys, logger, tracingContextFromHeaders } from '@sentry/utils'; +import { getDynamicSamplingContextFromSpan } from '.'; import { getCurrentScope, getIsolationScope, withScope } from '../currentScopes'; import { DEBUG_BUILD } from '../debug-build'; import { getCurrentHub } from '../hub'; import { handleCallbackErrors } from '../utils/handleCallbackErrors'; import { hasTracingEnabled } from '../utils/hasTracingEnabled'; -import { spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils'; +import { spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils'; /** * Wraps a function with a transaction/span and finishes the span after the function is done. @@ -21,7 +22,7 @@ import { spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils'; * and the `span` returned from the callback will be undefined. */ export function startSpan(context: StartSpanOptions, callback: (span: Span | undefined) => T): T { - const ctx = normalizeContext(context); + const spanContext = normalizeContext(context); return withScope(context.scope, scope => { // eslint-disable-next-line deprecation/deprecation @@ -30,10 +31,14 @@ export function startSpan(context: StartSpanOptions, callback: (span: Span | const parentSpan = scope.getSpan(); const shouldSkipSpan = context.onlyIfParent && !parentSpan; - const activeSpan = shouldSkipSpan ? undefined : createChildSpanOrTransaction(hub, parentSpan, ctx); - - // eslint-disable-next-line deprecation/deprecation - scope.setSpan(activeSpan); + const activeSpan = shouldSkipSpan + ? undefined + : createChildSpanOrTransaction(hub, { + parentSpan, + spanContext, + forceTransaction: context.forceTransaction, + scope, + }); return handleCallbackErrors( () => callback(activeSpan), @@ -66,7 +71,7 @@ export function startSpanManual( context: StartSpanOptions, callback: (span: Span | undefined, finish: () => void) => T, ): T { - const ctx = normalizeContext(context); + const spanContext = normalizeContext(context); return withScope(context.scope, scope => { // eslint-disable-next-line deprecation/deprecation @@ -75,10 +80,14 @@ export function startSpanManual( const parentSpan = scope.getSpan(); const shouldSkipSpan = context.onlyIfParent && !parentSpan; - const activeSpan = shouldSkipSpan ? undefined : createChildSpanOrTransaction(hub, parentSpan, ctx); - - // eslint-disable-next-line deprecation/deprecation - scope.setSpan(activeSpan); + const activeSpan = shouldSkipSpan + ? undefined + : createChildSpanOrTransaction(hub, { + parentSpan, + spanContext, + forceTransaction: context.forceTransaction, + scope, + }); function finishAndSetSpan(): void { activeSpan && activeSpan.end(); @@ -114,7 +123,7 @@ export function startInactiveSpan(context: StartSpanOptions): Span | undefined { return undefined; } - const ctx = normalizeContext(context); + const spanContext = normalizeContext(context); // eslint-disable-next-line deprecation/deprecation const hub = getCurrentHub(); const parentSpan = context.scope @@ -128,41 +137,19 @@ export function startInactiveSpan(context: StartSpanOptions): Span | undefined { return undefined; } - const isolationScope = getIsolationScope(); - const scope = getCurrentScope(); - - let span: Span | undefined; - - if (parentSpan) { - // eslint-disable-next-line deprecation/deprecation - span = parentSpan.startChild(ctx); - } else { - const { traceId, dsc, parentSpanId, sampled } = { - ...isolationScope.getPropagationContext(), - ...scope.getPropagationContext(), - }; - - // eslint-disable-next-line deprecation/deprecation - span = hub.startTransaction({ - traceId, - parentSpanId, - parentSampled: sampled, - ...ctx, - metadata: { - dynamicSamplingContext: dsc, - // eslint-disable-next-line deprecation/deprecation - ...ctx.metadata, - }, - }); - } - - if (parentSpan) { - addChildSpanToSpan(parentSpan, span); - } + const scope = context.scope || getCurrentScope(); - setCapturedScopesOnSpan(span, scope, isolationScope); + // Even though we don't actually want to make this span active on the current scope, + // we need to make it active on a temporary scope that we use for event processing + // as otherwise, it won't pick the correct span for the event when processing it + const temporaryScope = scope.clone(); - return span; + return createChildSpanOrTransaction(hub, { + parentSpan, + spanContext, + forceTransaction: context.forceTransaction, + scope: temporaryScope, + }); } /** @@ -277,20 +264,47 @@ export const continueTrace: ContinueTrace = ( function createChildSpanOrTransaction( hub: Hub, - parentSpan: Span | undefined, - ctx: TransactionContext, + { + parentSpan, + spanContext, + forceTransaction, + scope, + }: { + parentSpan: Span | undefined; + spanContext: TransactionContext; + forceTransaction?: boolean; + scope: Scope; + }, ): Span | undefined { if (!hasTracingEnabled()) { return undefined; } const isolationScope = getIsolationScope(); - const scope = getCurrentScope(); let span: Span | undefined; - if (parentSpan) { + if (parentSpan && !forceTransaction) { + // eslint-disable-next-line deprecation/deprecation + span = parentSpan.startChild(spanContext); + addChildSpanToSpan(parentSpan, span); + } else if (parentSpan) { + // If we forced a transaction but have a parent span, make sure to continue from the parent span, not the scope + const dsc = getDynamicSamplingContextFromSpan(parentSpan); + const { traceId, spanId: parentSpanId } = parentSpan.spanContext(); + const sampled = spanIsSampled(parentSpan); + // eslint-disable-next-line deprecation/deprecation - span = parentSpan.startChild(ctx); + span = hub.startTransaction({ + traceId, + parentSpanId, + parentSampled: sampled, + ...spanContext, + metadata: { + dynamicSamplingContext: dsc, + // eslint-disable-next-line deprecation/deprecation + ...spanContext.metadata, + }, + }); } else { const { traceId, dsc, parentSpanId, sampled } = { ...isolationScope.getPropagationContext(), @@ -302,18 +316,20 @@ function createChildSpanOrTransaction( traceId, parentSpanId, parentSampled: sampled, - ...ctx, + ...spanContext, metadata: { dynamicSamplingContext: dsc, // eslint-disable-next-line deprecation/deprecation - ...ctx.metadata, + ...spanContext.metadata, }, }); } - if (parentSpan) { - addChildSpanToSpan(parentSpan, span); - } + // We always set this as active span on the scope + // In the case of this being an inactive span, we ensure to pass a detached scope in here in the first place + // But by having this here, we can ensure that the lookup through `getCapturedScopesOnSpan` results in the correct scope & span combo + // eslint-disable-next-line deprecation/deprecation + scope.setSpan(span); setCapturedScopesOnSpan(span, scope, isolationScope); diff --git a/packages/core/src/tracing/transaction.ts b/packages/core/src/tracing/transaction.ts index d3d0c19718a0..bd71918cfc4c 100644 --- a/packages/core/src/tracing/transaction.ts +++ b/packages/core/src/tracing/transaction.ts @@ -301,7 +301,9 @@ export class Transaction extends SentrySpan implements TransactionInterface { ...metadata, capturedSpanScope, capturedSpanIsolationScope, - dynamicSamplingContext: getDynamicSamplingContextFromSpan(this), + ...dropUndefinedKeys({ + dynamicSamplingContext: getDynamicSamplingContextFromSpan(this), + }), }, _metrics_summary: getMetricSummaryJsonForSpan(this), ...(source && { diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index 8c18bea9afe9..b02a4d2bb0fc 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -1,4 +1,4 @@ -import type { Span as SpanType } from '@sentry/types'; +import type { Event, Span as SpanType } from '@sentry/types'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, addTracingExtensions, @@ -288,6 +288,112 @@ describe('startSpan', () => { expect(getActiveSpan()).toBe(undefined); }); + it('allows to force a transaction with forceTransaction=true', async () => { + const options = getDefaultTestClientOptions({ tracesSampleRate: 1.0 }); + client = new TestClient(options); + setCurrentClient(client); + client.init(); + + const transactionEvents: Event[] = []; + + client.addEventProcessor(event => { + if (event.type === 'transaction') { + transactionEvents.push(event); + } + return event; + }); + + startSpan({ name: 'outer transaction' }, () => { + startSpan({ name: 'inner span' }, () => { + startSpan({ name: 'inner transaction', forceTransaction: true }, () => { + startSpan({ name: 'inner span 2' }, () => { + // all good + }); + }); + }); + }); + + await client.flush(); + + const normalizedTransactionEvents = transactionEvents.map(event => { + return { + ...event, + spans: event.spans?.map(span => ({ name: spanToJSON(span).description, id: span.spanContext().spanId })), + sdkProcessingMetadata: { + dynamicSamplingContext: event.sdkProcessingMetadata?.dynamicSamplingContext, + }, + }; + }); + + expect(normalizedTransactionEvents).toHaveLength(2); + + const outerTransaction = normalizedTransactionEvents.find(event => event.transaction === 'outer transaction'); + const innerTransaction = normalizedTransactionEvents.find(event => event.transaction === 'inner transaction'); + + const outerTraceId = outerTransaction?.contexts?.trace?.trace_id; + // The inner transaction should be a child of the last span of the outer transaction + const innerParentSpanId = outerTransaction?.spans?.[0].id; + const innerSpanId = innerTransaction?.contexts?.trace?.span_id; + + expect(outerTraceId).toBeDefined(); + expect(innerParentSpanId).toBeDefined(); + expect(innerSpanId).toBeDefined(); + // inner span ID should _not_ be the parent span ID, but the id of the new span + expect(innerSpanId).not.toEqual(innerParentSpanId); + + expect(outerTransaction?.contexts).toEqual({ + trace: { + data: { + 'sentry.source': 'custom', + 'sentry.sample_rate': 1, + 'sentry.origin': 'manual', + }, + span_id: expect.any(String), + trace_id: expect.any(String), + origin: 'manual', + }, + }); + expect(outerTransaction?.spans).toEqual([{ name: 'inner span', id: expect.any(String) }]); + expect(outerTransaction?.tags).toEqual({ + transaction: 'outer transaction', + }); + expect(outerTransaction?.sdkProcessingMetadata).toEqual({ + dynamicSamplingContext: { + environment: 'production', + trace_id: outerTraceId, + sample_rate: '1', + transaction: 'outer transaction', + sampled: 'true', + }, + }); + + expect(innerTransaction?.contexts).toEqual({ + trace: { + data: { + 'sentry.source': 'custom', + 'sentry.origin': 'manual', + }, + parent_span_id: innerParentSpanId, + span_id: expect.any(String), + trace_id: outerTraceId, + origin: 'manual', + }, + }); + expect(innerTransaction?.spans).toEqual([{ name: 'inner span 2', id: expect.any(String) }]); + expect(innerTransaction?.tags).toEqual({ + transaction: 'inner transaction', + }); + expect(innerTransaction?.sdkProcessingMetadata).toEqual({ + dynamicSamplingContext: { + environment: 'production', + trace_id: outerTraceId, + sample_rate: '1', + transaction: 'outer transaction', + sampled: 'true', + }, + }); + }); + it("picks up the trace id off the parent scope's propagation context", () => { expect.assertions(1); withScope(scope => { @@ -463,6 +569,116 @@ describe('startSpanManual', () => { expect(getActiveSpan()).toBe(undefined); }); + it('allows to force a transaction with forceTransaction=true', async () => { + const options = getDefaultTestClientOptions({ tracesSampleRate: 1.0 }); + client = new TestClient(options); + setCurrentClient(client); + client.init(); + + const transactionEvents: Event[] = []; + + client.addEventProcessor(event => { + if (event.type === 'transaction') { + transactionEvents.push(event); + } + return event; + }); + + startSpanManual({ name: 'outer transaction' }, span => { + startSpanManual({ name: 'inner span' }, span => { + startSpanManual({ name: 'inner transaction', forceTransaction: true }, span => { + startSpanManual({ name: 'inner span 2' }, span => { + // all good + span?.end(); + }); + span?.end(); + }); + span?.end(); + }); + span?.end(); + }); + + await client.flush(); + + const normalizedTransactionEvents = transactionEvents.map(event => { + return { + ...event, + spans: event.spans?.map(span => ({ name: spanToJSON(span).description, id: span.spanContext().spanId })), + sdkProcessingMetadata: { + dynamicSamplingContext: event.sdkProcessingMetadata?.dynamicSamplingContext, + }, + }; + }); + + expect(normalizedTransactionEvents).toHaveLength(2); + + const outerTransaction = normalizedTransactionEvents.find(event => event.transaction === 'outer transaction'); + const innerTransaction = normalizedTransactionEvents.find(event => event.transaction === 'inner transaction'); + + const outerTraceId = outerTransaction?.contexts?.trace?.trace_id; + // The inner transaction should be a child of the last span of the outer transaction + const innerParentSpanId = outerTransaction?.spans?.[0].id; + const innerSpanId = innerTransaction?.contexts?.trace?.span_id; + + expect(outerTraceId).toBeDefined(); + expect(innerParentSpanId).toBeDefined(); + expect(innerSpanId).toBeDefined(); + // inner span ID should _not_ be the parent span ID, but the id of the new span + expect(innerSpanId).not.toEqual(innerParentSpanId); + + expect(outerTransaction?.contexts).toEqual({ + trace: { + data: { + 'sentry.source': 'custom', + 'sentry.sample_rate': 1, + 'sentry.origin': 'manual', + }, + span_id: expect.any(String), + trace_id: expect.any(String), + origin: 'manual', + }, + }); + expect(outerTransaction?.spans).toEqual([{ name: 'inner span', id: expect.any(String) }]); + expect(outerTransaction?.tags).toEqual({ + transaction: 'outer transaction', + }); + expect(outerTransaction?.sdkProcessingMetadata).toEqual({ + dynamicSamplingContext: { + environment: 'production', + trace_id: outerTraceId, + sample_rate: '1', + transaction: 'outer transaction', + sampled: 'true', + }, + }); + + expect(innerTransaction?.contexts).toEqual({ + trace: { + data: { + 'sentry.source': 'custom', + 'sentry.origin': 'manual', + }, + parent_span_id: innerParentSpanId, + span_id: expect.any(String), + trace_id: outerTraceId, + origin: 'manual', + }, + }); + expect(innerTransaction?.spans).toEqual([{ name: 'inner span 2', id: expect.any(String) }]); + expect(innerTransaction?.tags).toEqual({ + transaction: 'inner transaction', + }); + expect(innerTransaction?.sdkProcessingMetadata).toEqual({ + dynamicSamplingContext: { + environment: 'production', + trace_id: outerTraceId, + sample_rate: '1', + transaction: 'outer transaction', + sampled: 'true', + }, + }); + }); + it('allows to pass a `startTime`', () => { const start = startSpanManual({ name: 'outer', startTime: [1234, 0] }, span => { span?.end(); @@ -573,6 +789,109 @@ describe('startInactiveSpan', () => { expect(getActiveSpan()).toBeUndefined(); }); + it('allows to force a transaction with forceTransaction=true', async () => { + const options = getDefaultTestClientOptions({ tracesSampleRate: 1.0 }); + client = new TestClient(options); + setCurrentClient(client); + client.init(); + + const transactionEvents: Event[] = []; + + client.addEventProcessor(event => { + if (event.type === 'transaction') { + transactionEvents.push(event); + } + return event; + }); + + startSpan({ name: 'outer transaction' }, () => { + startSpan({ name: 'inner span' }, () => { + const innerTransaction = startInactiveSpan({ name: 'inner transaction', forceTransaction: true }); + innerTransaction?.end(); + }); + }); + + await client.flush(); + + const normalizedTransactionEvents = transactionEvents.map(event => { + return { + ...event, + spans: event.spans?.map(span => ({ name: spanToJSON(span).description, id: span.spanContext().spanId })), + sdkProcessingMetadata: { + dynamicSamplingContext: event.sdkProcessingMetadata?.dynamicSamplingContext, + }, + }; + }); + + expect(normalizedTransactionEvents).toHaveLength(2); + + const outerTransaction = normalizedTransactionEvents.find(event => event.transaction === 'outer transaction'); + const innerTransaction = normalizedTransactionEvents.find(event => event.transaction === 'inner transaction'); + + const outerTraceId = outerTransaction?.contexts?.trace?.trace_id; + // The inner transaction should be a child of the last span of the outer transaction + const innerParentSpanId = outerTransaction?.spans?.[0].id; + const innerSpanId = innerTransaction?.contexts?.trace?.span_id; + + expect(outerTraceId).toBeDefined(); + expect(innerParentSpanId).toBeDefined(); + expect(innerSpanId).toBeDefined(); + // inner span ID should _not_ be the parent span ID, but the id of the new span + expect(innerSpanId).not.toEqual(innerParentSpanId); + + expect(outerTransaction?.contexts).toEqual({ + trace: { + data: { + 'sentry.source': 'custom', + 'sentry.sample_rate': 1, + 'sentry.origin': 'manual', + }, + span_id: expect.any(String), + trace_id: expect.any(String), + origin: 'manual', + }, + }); + expect(outerTransaction?.spans).toEqual([{ name: 'inner span', id: expect.any(String) }]); + expect(outerTransaction?.tags).toEqual({ + transaction: 'outer transaction', + }); + expect(outerTransaction?.sdkProcessingMetadata).toEqual({ + dynamicSamplingContext: { + environment: 'production', + trace_id: outerTraceId, + sample_rate: '1', + transaction: 'outer transaction', + sampled: 'true', + }, + }); + + expect(innerTransaction?.contexts).toEqual({ + trace: { + data: { + 'sentry.source': 'custom', + 'sentry.origin': 'manual', + }, + parent_span_id: innerParentSpanId, + span_id: expect.any(String), + trace_id: outerTraceId, + origin: 'manual', + }, + }); + expect(innerTransaction?.spans).toEqual([]); + expect(innerTransaction?.tags).toEqual({ + transaction: 'inner transaction', + }); + expect(innerTransaction?.sdkProcessingMetadata).toEqual({ + dynamicSamplingContext: { + environment: 'production', + trace_id: outerTraceId, + sample_rate: '1', + transaction: 'outer transaction', + sampled: 'true', + }, + }); + }); + it('allows to pass a `startTime`', () => { const span = startInactiveSpan({ name: 'outer', startTime: [1234, 0] }); expect(spanToJSON(span!).start_timestamp).toEqual(1234); diff --git a/packages/node/test/integrations/undici.test.ts b/packages/node/test/integrations/undici.test.ts index 3284e13436d8..2409515c08cb 100644 --- a/packages/node/test/integrations/undici.test.ts +++ b/packages/node/test/integrations/undici.test.ts @@ -154,7 +154,7 @@ conditionalTest({ min: 16 })('Undici integration', () => { }); }); - it('creates a span for invalid looking urls xxx', async () => { + it('creates a span for invalid looking urls', async () => { await startSpan({ name: 'outer-span' }, async outerSpan => { try { // Intentionally add // to the url diff --git a/packages/types/src/startSpanOptions.ts b/packages/types/src/startSpanOptions.ts index 44be5bb1c699..01293e129c74 100644 --- a/packages/types/src/startSpanOptions.ts +++ b/packages/types/src/startSpanOptions.ts @@ -19,6 +19,13 @@ export interface StartSpanOptions extends TransactionContext { /** An op for the span. This is a categorization for spans. */ op?: string; + /** + * If set to true, this span will be forced to be treated as a transaction in the Sentry UI, if possible and applicable. + * Note that it is up to the SDK to decide how exactly the span will be sent, which may change in future SDK versions. + * It is not guaranteed that a span started with this flag set to `true` will be sent as a transaction. + */ + forceTransaction?: boolean; + /** * The origin of the span - if it comes from auto instrumentation or manual instrumentation. *