From 135e613233ca063bff1713bea8f065a84b911f58 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 26 Feb 2024 11:15:54 +0000 Subject: [PATCH] feat(opentelemetry): Support `forceTransaction` in OTEL --- packages/opentelemetry/src/trace.ts | 60 +++- packages/opentelemetry/test/trace.test.ts | 320 +++++++++++++++++++++- 2 files changed, 372 insertions(+), 8 deletions(-) diff --git a/packages/opentelemetry/src/trace.ts b/packages/opentelemetry/src/trace.ts index 8bd4ae9766be..a5fd8e259bbf 100644 --- a/packages/opentelemetry/src/trace.ts +++ b/packages/opentelemetry/src/trace.ts @@ -1,13 +1,18 @@ -import type { Context, Span, SpanOptions, Tracer } from '@opentelemetry/api'; +import type { Context, Span, SpanContext, SpanOptions, Tracer } from '@opentelemetry/api'; +import { TraceFlags } from '@opentelemetry/api'; import { context } from '@opentelemetry/api'; import { SpanStatusCode, trace } from '@opentelemetry/api'; -import { suppressTracing } from '@opentelemetry/core'; +import { TraceState, suppressTracing } from '@opentelemetry/core'; import { SDK_VERSION, getClient, getCurrentScope, handleCallbackErrors } from '@sentry/core'; import type { Client, Scope } from '@sentry/types'; +import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils'; +import { SENTRY_TRACE_STATE_DSC } from './constants'; import { InternalSentrySemanticAttributes } from './semanticAttributes'; import type { OpenTelemetryClient, OpenTelemetrySpanContext } from './types'; import { getContextFromScope } from './utils/contextData'; +import { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext'; +import { getRootSpan } from './utils/getActiveSpan'; import { setSpanMetadata } from './utils/spanData'; /** @@ -24,7 +29,7 @@ export function startSpan(options: OpenTelemetrySpanContext, callback: (span: const { name } = options; - const activeCtx = getContext(options.scope); + const activeCtx = getContext(options.scope, options.forceTransaction); const shouldSkipSpan = options.onlyIfParent && !trace.getSpan(activeCtx); const ctx = shouldSkipSpan ? suppressTracing(activeCtx) : activeCtx; @@ -57,7 +62,7 @@ export function startSpanManual(options: OpenTelemetrySpanContext, callback: const { name } = options; - const activeCtx = getContext(options.scope); + const activeCtx = getContext(options.scope, options.forceTransaction); const shouldSkipSpan = options.onlyIfParent && !trace.getSpan(activeCtx); const ctx = shouldSkipSpan ? suppressTracing(activeCtx) : activeCtx; @@ -95,7 +100,7 @@ export function startInactiveSpan(options: OpenTelemetrySpanContext): Span { const { name } = options; - const activeCtx = getContext(options.scope); + const activeCtx = getContext(options.scope, options.forceTransaction); const shouldSkipSpan = options.onlyIfParent && !trace.getSpan(activeCtx); const ctx = shouldSkipSpan ? suppressTracing(activeCtx) : activeCtx; @@ -166,7 +171,50 @@ function ensureTimestampInMilliseconds(timestamp: number): number { return isMs ? timestamp * 1000 : timestamp; } -function getContext(scope?: Scope): Context { +function getContext(scope: Scope | undefined, forceTransaction: boolean | undefined): Context { + const ctx = getContextForScope(scope); + + if (!forceTransaction) { + return ctx; + } + + // Else we need to "fix" the context to have no parent span + const parentSpan = trace.getSpan(ctx); + + // If there is no parent span, all good, nothing to do! + if (!parentSpan) { + return ctx; + } + + // Else, we need to do two things: + // 1. Unset the parent span from the context, so we'll create a new root span + // 2. Ensure the propagation context is correct, so we'll continue from the parent span + const ctxWithoutSpan = trace.deleteSpan(ctx); + + const { spanId, traceId, traceFlags } = parentSpan.spanContext(); + // eslint-disable-next-line no-bitwise + const sampled = Boolean(traceFlags & TraceFlags.SAMPLED); + + const rootSpan = getRootSpan(parentSpan); + const dsc = getDynamicSamplingContextFromSpan(rootSpan); + const dscString = dynamicSamplingContextToSentryBaggageHeader(dsc); + + const traceState = dscString ? new TraceState().set(SENTRY_TRACE_STATE_DSC, dscString) : undefined; + + const spanContext: SpanContext = { + traceId, + spanId, + isRemote: true, + traceFlags: sampled ? TraceFlags.SAMPLED : TraceFlags.NONE, + traceState, + }; + + const ctxWithSpanContext = trace.setSpanContext(ctxWithoutSpan, spanContext); + + return ctxWithSpanContext; +} + +function getContextForScope(scope?: Scope): Context { if (scope) { const ctx = getContextFromScope(scope); if (ctx) { diff --git a/packages/opentelemetry/test/trace.test.ts b/packages/opentelemetry/test/trace.test.ts index 969df0643da3..7cc845232ec8 100644 --- a/packages/opentelemetry/test/trace.test.ts +++ b/packages/opentelemetry/test/trace.test.ts @@ -3,8 +3,8 @@ import { SpanKind } from '@opentelemetry/api'; import { TraceFlags, context, trace } from '@opentelemetry/api'; import type { ReadableSpan } from '@opentelemetry/sdk-trace-base'; import { Span as SpanClass } from '@opentelemetry/sdk-trace-base'; -import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, getClient, getCurrentScope } from '@sentry/core'; -import type { PropagationContext, Scope } from '@sentry/types'; +import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, getClient, getCurrentScope, spanToJSON } from '@sentry/core'; +import type { Event, PropagationContext, Scope } from '@sentry/types'; import { InternalSentrySemanticAttributes } from '../src/semanticAttributes'; import { startInactiveSpan, startSpan, startSpanManual } from '../src/trace'; @@ -300,6 +300,111 @@ describe('trace', () => { expect(getActiveSpan()).toBe(undefined); }); + it('allows to force a transaction with forceTransaction=true', async () => { + const client = getClient()!; + const transactionEvents: Event[] = []; + + client.getOptions().beforeSendTransaction = event => { + transactionEvents.push({ + ...event, + sdkProcessingMetadata: { + dynamicSamplingContext: event.sdkProcessingMetadata?.dynamicSamplingContext, + }, + }); + 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 })), + }; + }); + + 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?.trace).toEqual({ + data: { + 'sentry.source': 'custom', + 'sentry.sample_rate': 1, + 'sentry.origin': 'manual', + 'otel.kind': 'INTERNAL', + }, + span_id: expect.any(String), + trace_id: expect.any(String), + origin: 'manual', + status: 'ok', + }); + 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', + public_key: 'username', + trace_id: outerTraceId, + sample_rate: '1', + transaction: 'outer transaction', + sampled: 'true', + }, + }); + + expect(innerTransaction?.contexts?.trace).toEqual({ + data: { + 'sentry.source': 'custom', + 'sentry.origin': 'manual', + 'otel.kind': 'INTERNAL', + 'sentry.sample_rate': 1, + }, + parent_span_id: innerParentSpanId, + span_id: expect.any(String), + trace_id: outerTraceId, + origin: 'manual', + status: 'ok', + }); + 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', + public_key: 'username', + trace_id: outerTraceId, + sample_rate: '1', + transaction: 'outer transaction', + sampled: 'true', + }, + }); + }); + // TODO: propagation scope is not picked up by spans... describe('onlyIfParent', () => { @@ -444,6 +549,108 @@ describe('trace', () => { expect(getActiveSpan()).toBe(undefined); }); + it('allows to force a transaction with forceTransaction=true', async () => { + const client = getClient()!; + const transactionEvents: Event[] = []; + + client.getOptions().beforeSendTransaction = event => { + transactionEvents.push({ + ...event, + sdkProcessingMetadata: { + dynamicSamplingContext: event.sdkProcessingMetadata?.dynamicSamplingContext, + }, + }); + 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 })), + }; + }); + + 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?.trace).toEqual({ + data: { + 'sentry.source': 'custom', + 'sentry.sample_rate': 1, + 'sentry.origin': 'manual', + 'otel.kind': 'INTERNAL', + }, + span_id: expect.any(String), + trace_id: expect.any(String), + origin: 'manual', + status: 'ok', + }); + 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', + public_key: 'username', + trace_id: outerTraceId, + sample_rate: '1', + transaction: 'outer transaction', + sampled: 'true', + }, + }); + + expect(innerTransaction?.contexts?.trace).toEqual({ + data: { + 'sentry.source': 'custom', + 'sentry.origin': 'manual', + 'otel.kind': 'INTERNAL', + 'sentry.sample_rate': 1, + }, + parent_span_id: innerParentSpanId, + span_id: expect.any(String), + trace_id: outerTraceId, + origin: 'manual', + status: 'ok', + }); + expect(innerTransaction?.spans).toEqual([]); + expect(innerTransaction?.tags).toEqual({ + transaction: 'inner transaction', + }); + expect(innerTransaction?.sdkProcessingMetadata).toEqual({ + dynamicSamplingContext: { + environment: 'production', + public_key: 'username', + trace_id: outerTraceId, + sample_rate: '1', + transaction: 'outer transaction', + sampled: 'true', + }, + }); + }); + describe('onlyIfParent', () => { it('does not create a span if there is no parent', () => { const span = startInactiveSpan({ name: 'test span', onlyIfParent: true }); @@ -572,6 +779,115 @@ describe('trace', () => { expect(getActiveSpan()).toBe(undefined); }); + it('allows to force a transaction with forceTransaction=true', async () => { + const client = getClient()!; + const transactionEvents: Event[] = []; + + client.getOptions().beforeSendTransaction = event => { + transactionEvents.push({ + ...event, + sdkProcessingMetadata: { + dynamicSamplingContext: event.sdkProcessingMetadata?.dynamicSamplingContext, + }, + }); + 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 })), + }; + }); + + 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?.trace).toEqual({ + data: { + 'sentry.source': 'custom', + 'sentry.sample_rate': 1, + 'sentry.origin': 'manual', + 'otel.kind': 'INTERNAL', + }, + span_id: expect.any(String), + trace_id: expect.any(String), + origin: 'manual', + status: 'ok', + }); + 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', + public_key: 'username', + trace_id: outerTraceId, + sample_rate: '1', + transaction: 'outer transaction', + sampled: 'true', + }, + }); + + expect(innerTransaction?.contexts?.trace).toEqual({ + data: { + 'sentry.source': 'custom', + 'sentry.origin': 'manual', + 'otel.kind': 'INTERNAL', + 'sentry.sample_rate': 1, + }, + parent_span_id: innerParentSpanId, + span_id: expect.any(String), + trace_id: outerTraceId, + origin: 'manual', + status: 'ok', + }); + 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', + public_key: 'username', + trace_id: outerTraceId, + sample_rate: '1', + transaction: 'outer transaction', + sampled: 'true', + }, + }); + }); + describe('onlyIfParent', () => { it('does not create a span if there is no parent', () => { const span = startSpanManual({ name: 'test span', onlyIfParent: true }, span => {