feat(opentelemetry): Ensure DSC & attributes are correctly set

Author: mydea

packages/core/src/tracing/dynamicSamplingContext.ts

@@ -47,7 +47,6 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly<Partial<
     return {};
   }
 
-  // passing emit=false here to only emit later once the DSC is actually populated
   const dsc = getDynamicSamplingContextFromClient(spanToJSON(span).trace_id || '', client);
 
   // TODO (v8): Remove v7FrozenDsc as a Transaction will no longer have _frozenDynamicSamplingContext

packages/opentelemetry/src/constants.ts

@@ -2,6 +2,7 @@ import { createContextKey } from '@opentelemetry/api';
 
 export const SENTRY_TRACE_HEADER = 'sentry-trace';
 export const SENTRY_BAGGAGE_HEADER = 'baggage';
+export const SENTRY_TRACE_STATE_DSC = 'sentry.trace';
 
 /** Context Key to hold a PropagationContext. */
 export const SENTRY_PROPAGATION_CONTEXT_CONTEXT_KEY = createContextKey('SENTRY_PROPAGATION_CONTEXT_CONTEXT_KEY');

packages/opentelemetry/src/propagator.ts

@@ -1,11 +1,16 @@
 import type { Baggage, Context, SpanContext, TextMapGetter, TextMapSetter } from '@opentelemetry/api';
 import { TraceFlags, propagation, trace } from '@opentelemetry/api';
-import { W3CBaggagePropagator, isTracingSuppressed } from '@opentelemetry/core';
+import { TraceState, W3CBaggagePropagator, isTracingSuppressed } from '@opentelemetry/core';
 import { getClient, getDynamicSamplingContextFromClient } from '@sentry/core';
 import type { DynamicSamplingContext, PropagationContext } from '@sentry/types';
-import { SENTRY_BAGGAGE_KEY_PREFIX, generateSentryTraceHeader, propagationContextFromHeaders } from '@sentry/utils';
-
-import { SENTRY_BAGGAGE_HEADER, SENTRY_TRACE_HEADER } from './constants';
+import {
+  SENTRY_BAGGAGE_KEY_PREFIX,
+  dynamicSamplingContextToSentryBaggageHeader,
+  generateSentryTraceHeader,
+  propagationContextFromHeaders,
+} from '@sentry/utils';
+
+import { SENTRY_BAGGAGE_HEADER, SENTRY_TRACE_HEADER, SENTRY_TRACE_STATE_DSC } from './constants';
 import { getPropagationContextFromContext, setPropagationContextOnContext } from './utils/contextData';
 
 /**
@@ -24,7 +29,9 @@ export class SentryPropagator extends W3CBaggagePropagator {
 
     const propagationContext = getPropagationContextFromContext(context);
     const { spanId, traceId, sampled } = getSentryTraceData(context, propagationContext);
-    const dynamicSamplingContext = propagationContext ? getDsc(propagationContext, traceId) : undefined;
+    const dynamicSamplingContext = propagationContext
+      ? getDynamicSamplingContext(propagationContext, traceId)
+      : undefined;
 
     if (dynamicSamplingContext) {
       baggage = Object.entries(dynamicSamplingContext).reduce<Baggage>((b, [dscKey, dscValue]) => {
@@ -58,11 +65,19 @@ export class SentryPropagator extends W3CBaggagePropagator {
     // Add propagation context to context
     const contextWithPropagationContext = setPropagationContextOnContext(context, propagationContext);
 
+    // We store the DSC as OTEL trace state on the span context
+    const dscString = propagationContext.dsc
+      ? dynamicSamplingContextToSentryBaggageHeader(propagationContext.dsc)
+      : undefined;
+
+    const traceState = dscString ? new TraceState().set(SENTRY_TRACE_STATE_DSC, dscString) : undefined;
+
     const spanContext: SpanContext = {
       traceId: propagationContext.traceId,
       spanId: propagationContext.parentSpanId || '',
       isRemote: true,
       traceFlags: propagationContext.sampled === true ? TraceFlags.SAMPLED : TraceFlags.NONE,
+      traceState,
     };
 
     // Add remote parent span context
@@ -77,7 +92,8 @@ export class SentryPropagator extends W3CBaggagePropagator {
   }
 }
 
-function getDsc(
+/** Get the DSC. */
+function getDynamicSamplingContext(
   propagationContext: PropagationContext,
   traceId: string | undefined,
 ): Partial<DynamicSamplingContext> | undefined {
@@ -96,6 +112,7 @@ function getDsc(
   return undefined;
 }
 
+/** Get the trace data for propagation. */
 function getSentryTraceData(
   context: Context,
   propagationContext: PropagationContext | undefined,

packages/opentelemetry/src/semanticAttributes.ts

@@ -6,7 +6,6 @@ export const InternalSentrySemanticAttributes = {
   ORIGIN: 'sentry.origin',
   OP: 'sentry.op',
   SOURCE: 'sentry.source',
-  SAMPLE_RATE: 'sentry.sample_rate',
   PARENT_SAMPLED: 'sentry.parentSampled',
   BREADCRUMB_TYPE: 'sentry.breadcrumb.type',
   BREADCRUMB_LEVEL: 'sentry.breadcrumb.level',

packages/opentelemetry/src/setupEventContextTrace.ts

@@ -1,28 +1,37 @@
 import type { Client } from '@sentry/types';
+import { dropUndefinedKeys } from '@sentry/utils';
 
-import { getActiveSpan } from './utils/getActiveSpan';
-import { spanHasParentId } from './utils/spanTypes';
+import { getActiveSpan, getRootSpan } from './utils/getActiveSpan';
+import { spanHasName, spanHasParentId } from './utils/spanTypes';
 
 /** Ensure the `trace` context is set on all events. */
 export function setupEventContextTrace(client: Client): void {
   client.addEventProcessor(event => {
     const span = getActiveSpan();
-    if (!span) {
+    // For transaction events, this is handled separately
+    // Because the active span may not be the span that is actually the transaction event
+    if (!span || event.type === 'transaction') {
       return event;
     }
 
     const spanContext = span.spanContext();
 
     // If event has already set `trace` context, use that one.
     event.contexts = {
-      trace: {
+      trace: dropUndefinedKeys({
         trace_id: spanContext.traceId,
         span_id: spanContext.spanId,
         parent_span_id: spanHasParentId(span) ? span.parentSpanId : undefined,
-      },
+      }),
       ...event.contexts,
     };
 
+    const rootSpan = getRootSpan(span);
+    const transactionName = spanHasName(rootSpan) ? rootSpan.name : undefined;
+    if (transactionName) {
+      event.tags = { transaction: transactionName, ...event.tags };
+    }
+
     return event;
   });
 }

packages/opentelemetry/src/spanExporter.ts

@@ -7,12 +7,13 @@ import type { Transaction } from '@sentry/core';
 import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
 import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, flush, getCurrentHub } from '@sentry/core';
 import type { Scope, Span as SentrySpan, SpanOrigin, TransactionSource } from '@sentry/types';
-import { addNonEnumerableProperty, logger } from '@sentry/utils';
+import { addNonEnumerableProperty, dropUndefinedKeys, logger } from '@sentry/utils';
 import { startTransaction } from './custom/transaction';
 
 import { DEBUG_BUILD } from './debug-build';
 import { InternalSentrySemanticAttributes } from './semanticAttributes';
 import { convertOtelTimeToSeconds } from './utils/convertOtelTimeToSeconds';
+import { getDynamicSamplingContextFromSpan } from './utils/dynamicSamplingContext';
 import { getRequestSpanData } from './utils/getRequestSpanData';
 import type { SpanNode } from './utils/groupSpansWithParents';
 import { groupSpansWithParents } from './utils/groupSpansWithParents';
@@ -149,6 +150,17 @@ function createTransactionForOtelSpan(span: ReadableSpan): Transaction {
   const metadata = getSpanMetadata(span);
   const capturedSpanScopes = getSpanScopes(span);
 
+  const sampleRate = span.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as number | undefined;
+
+  const attributes = {
+    [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
+    [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate,
+    ...data,
+    ...removeSentryAttributes(span.attributes),
+  };
+
+  const dynamicSamplingContext = getDynamicSamplingContextFromSpan(span);
+
   const transaction = startTransaction(hub, {
     spanId,
     traceId,
@@ -159,13 +171,13 @@ function createTransactionForOtelSpan(span: ReadableSpan): Transaction {
     status: mapStatus(span),
     startTimestamp: convertOtelTimeToSeconds(span.startTime),
     metadata: {
-      sampleRate: span.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as number | undefined,
+      ...dropUndefinedKeys({
+        dynamicSamplingContext,
+        sampleRate,
+      }),
       ...metadata,
     },
-    attributes: {
-      [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
-    },
-    data: removeSentryAttributes(data),
+    attributes,
     origin,
     tags,
     sampled: true,
@@ -180,7 +192,14 @@ function createTransactionForOtelSpan(span: ReadableSpan): Transaction {
   });
 
   if (capturedSpanScopes) {
-    setCapturedScopesOnTransaction(transaction, capturedSpanScopes.scope, capturedSpanScopes.isolationScope);
+    // Ensure the `transaction` tag is correctly set on the transaction event
+    const scope = capturedSpanScopes.scope.clone();
+    scope.addEventProcessor(event => {
+      event.tags = { transaction: description, ...event.tags };
+      return event;
+    });
+
+    setCapturedScopesOnTransaction(transaction, scope, capturedSpanScopes.isolationScope);
   }
 
   return transaction;
@@ -264,6 +283,8 @@ function removeSentryAttributes(data: Record<string, unknown>): Record<string, u
   delete cleanedData[InternalSentrySemanticAttributes.ORIGIN];
   delete cleanedData[InternalSentrySemanticAttributes.OP];
   delete cleanedData[InternalSentrySemanticAttributes.SOURCE];
+  // We want to avoid having this on each span (as that is set by the Sampler)
+  // We only want this on the transaction, where we manually add it to `attributes`
   delete cleanedData[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE];
   /* eslint-enable @typescript-eslint/no-dynamic-delete */
 

packages/opentelemetry/src/utils/dynamicSamplingContext.ts

@@ -0,0 +1,61 @@
+import { TraceFlags } from '@opentelemetry/api';
+import {
+  SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE,
+  SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
+  getClient,
+  getDynamicSamplingContextFromClient,
+} from '@sentry/core';
+import type { DynamicSamplingContext } from '@sentry/types';
+import { baggageHeaderToDynamicSamplingContext } from '@sentry/utils';
+import { SENTRY_TRACE_STATE_DSC } from '../constants';
+import type { AbstractSpan } from '../types';
+import { spanHasAttributes, spanHasName } from './spanTypes';
+
+/**
+ * Creates a dynamic sampling context from a span (and client and scope)
+ *
+ * @param span the span from which a few values like the root span name and sample rate are extracted.
+ *
+ * @returns a dynamic sampling context
+ */
+export function getDynamicSamplingContextFromSpan(span: AbstractSpan): Readonly<Partial<DynamicSamplingContext>> {
+  const client = getClient();
+  if (!client) {
+    return {};
+  }
+
+  const dsc = getDynamicSamplingContextFromClient(span.spanContext().traceId, client);
+
+  const traceState = span.spanContext().traceState;
+  const traceStateDsc = traceState?.get(SENTRY_TRACE_STATE_DSC);
+
+  if (traceStateDsc) {
+    const dsc = baggageHeaderToDynamicSamplingContext(traceStateDsc);
+    if (dsc) {
+      return dsc;
+    }
+  }
+
+  const attributes = spanHasAttributes(span) ? span.attributes : {};
+
+  const sampleRate = attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE];
+  if (sampleRate != null) {
+    dsc.sample_rate = `${sampleRate}`;
+  }
+
+  // We don't want to have a transaction name in the DSC if the source is "url" because URLs might contain PII
+  const source = attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE];
+  const name = spanHasName(span) ? span.name : '';
+
+  if (source !== 'url' && name) {
+    dsc.transaction = name;
+  }
+
+  // eslint-disable-next-line no-bitwise
+  const sampled = Boolean(span.spanContext().traceFlags & TraceFlags.SAMPLED);
+  dsc.sampled = String(sampled);
+
+  client.emit('createDsc', dsc);
+
+  return dsc;
+}

packages/opentelemetry/test/integration/scope.test.ts

@@ -64,21 +64,33 @@ describe('Integration | Scope', () => {
       await client.flush();
 
       expect(beforeSend).toHaveBeenCalledTimes(1);
+
+      if (spanId) {
+        expect(beforeSend).toHaveBeenCalledWith(
+          expect.objectContaining({
+            contexts: {
+              trace: {
+                span_id: spanId,
+                trace_id: traceId,
+              },
+            },
+          }),
+          {
+            event_id: expect.any(String),
+            originalException: error,
+            syntheticException: expect.any(Error),
+          },
+        );
+      }
+
       expect(beforeSend).toHaveBeenCalledWith(
         expect.objectContaining({
-          contexts: expect.objectContaining({
-            trace: spanId
-              ? {
-                  span_id: spanId,
-                  trace_id: traceId,
-                }
-              : expect.any(Object),
-          }),
           tags: {
             tag1: 'val1',
             tag2: 'val2',
             tag3: 'val3',
             tag4: 'val4',
+            ...(enableTracing ? { transaction: 'outer' } : {}),
           },
         }),
         {
@@ -99,21 +111,22 @@ describe('Integration | Scope', () => {
                   'otel.kind': 'INTERNAL',
                   'sentry.origin': 'manual',
                   'sentry.source': 'custom',
+                  'sentry.sample_rate': 1,
                 },
                 span_id: spanId,
                 status: 'ok',
                 trace_id: traceId,
                 origin: 'manual',
               },
             }),
-
             spans: [],
             start_timestamp: expect.any(Number),
             tags: {
               tag1: 'val1',
               tag2: 'val2',
               tag3: 'val3',
               tag4: 'val4',
+              transaction: 'outer',
             },
             timestamp: expect.any(Number),
             transaction: 'outer',
@@ -207,6 +220,7 @@ describe('Integration | Scope', () => {
             tag2: 'val2a',
             tag3: 'val3a',
             tag4: 'val4a',
+            ...(enableTracing ? { transaction: 'outer' } : {}),
           },
         }),
         {
@@ -232,6 +246,7 @@ describe('Integration | Scope', () => {
             tag2: 'val2b',
             tag3: 'val3b',
             tag4: 'val4b',
+            ...(enableTracing ? { transaction: 'outer' } : {}),
           },
         }),
         {
@@ -331,6 +346,7 @@ describe('Integration | Scope', () => {
             tag4: 'val4a',
             isolationTag1: 'val1',
             isolationTag2: 'val2',
+            ...(enableTracing ? { transaction: 'outer' } : {}),
           },
         }),
         {
@@ -358,6 +374,7 @@ describe('Integration | Scope', () => {
             tag4: 'val4b',
             isolationTag1: 'val1',
             isolationTag2: 'val2b',
+            ...(enableTracing ? { transaction: 'outer' } : {}),
           },
         }),
         {