ref(tracing): Add necessary helpers for using propagation context on …

…outgoing headers (#8434) This PR adds: 1. A getter for `PropagationContext` on the scope 2. `generateSentryTraceHeader`, which will be used to dynamically create `sentry-trace` headers, regardless of if there is a span or not 3. `getDynamicSamplingContextFromClient`, which is used to generate dynamic sampling context from a client directly (instead of having to go through a transaction) This PR also updates `extractTraceparentData` and `dynamicSamplingContextToSentryBaggageHeader` to be more liberal about the values it takes so we can better accommodate the new flows we are going to add. Co-authored-by: Francesco Novy <[email protected]>
getsentry · Jul 1, 2023 · 9006287 · 9006287
1 parent 3625fb1
commit 9006287
Show file tree

Hide file tree

Showing 18 changed files with 120 additions and 63 deletions.
diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts
@@ -30,7 +30,6 @@ import {
   addItemToEnvelope,
   checkOrSetAlreadyCaught,
   createAttachmentEnvelopeItem,
-  dropUndefinedKeys,
   isPlainObject,
   isPrimitive,
   isThenable,
@@ -43,12 +42,12 @@ import {
 } from '@sentry/utils';
 
 import { getEnvelopeEndpointWithUrlEncodedAuth } from './api';
-import { DEFAULT_ENVIRONMENT } from './constants';
 import { createEventEnvelope, createSessionEnvelope } from './envelope';
 import type { IntegrationIndex } from './integration';
 import { setupIntegration, setupIntegrations } from './integration';
 import type { Scope } from './scope';
 import { updateSession } from './session';
+import { getDynamicSamplingContextFromClient } from './tracing/dynamicSamplingContext';
 import { prepareEvent } from './utils/prepareEvent';
 
 const ALREADY_SEEN_ERROR = "Not capturing exception because it's already been captured.";
@@ -531,20 +530,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
           ...evt.contexts,
         };
 
-        const { publicKey: public_key } = this.getDsn() || {};
-        const { segment: user_segment } = (scope && scope.getUser()) || {};
-
-        let dynamicSamplingContext = dsc;
-        if (!dsc) {
-          dynamicSamplingContext = dropUndefinedKeys({
-            environment: options.environment || DEFAULT_ENVIRONMENT,
-            release: options.release,
-            user_segment,
-            public_key,
-            trace_id,
-          });
-          this.emit && this.emit('createDsc', dynamicSamplingContext);
-        }
+        const dynamicSamplingContext = dsc ? dsc : getDynamicSamplingContextFromClient(trace_id, this, scope);
 
         evt.sdkProcessingMetadata = {
           dynamicSamplingContext,

diff --git a/packages/core/src/scope.ts b/packages/core/src/scope.ts
@@ -533,13 +533,20 @@ export class Scope implements ScopeInterface {
   }
 
   /**
-   * @inheritdoc
+   * @inheritDoc
    */
   public setPropagationContext(context: PropagationContext): this {
     this._propagationContext = context;
     return this;
   }
 
+  /**
+   * @inheritDoc
+   */
+  public getPropagationContext(): PropagationContext {
+    return this._propagationContext;
+  }
+
   /**
    * This will be called after {@link applyToEvent} is finished.
    */

diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts
@@ -0,0 +1,32 @@
+import type { Client, DynamicSamplingContext, Scope } from '@sentry/types';
+import { dropUndefinedKeys } from '@sentry/utils';
+
+import { DEFAULT_ENVIRONMENT } from '../constants';
+
+/**
+ * Creates a dynamic sampling context from a client.
+ *
+ * Dispatchs the `createDsc` lifecycle hook as a side effect.
+ */
+export function getDynamicSamplingContextFromClient(
+  trace_id: string,
+  client: Client,
+  scope?: Scope,
+): DynamicSamplingContext {
+  const options = client.getOptions();
+
+  const { publicKey: public_key } = client.getDsn() || {};
+  const { segment: user_segment } = (scope && scope.getUser()) || {};
+
+  const dsc = dropUndefinedKeys({
+    environment: options.environment || DEFAULT_ENVIRONMENT,
+    release: options.release,
+    user_segment,
+    public_key,
+    trace_id,
+  }) as DynamicSamplingContext;
+
+  client.emit && client.emit('createDsc', dsc);
+
+  return dsc;
+}
diff --git a/packages/core/src/tracing/index.ts b/packages/core/src/tracing/index.ts
@@ -7,3 +7,4 @@ export { extractTraceparentData, getActiveTransaction } from './utils';
 export { SpanStatus } from './spanstatus';
 export type { SpanStatusType } from './span';
 export { trace } from './trace';
+export { getDynamicSamplingContextFromClient } from './dynamicSamplingContext';
diff --git a/packages/core/src/tracing/span.ts b/packages/core/src/tracing/span.ts
@@ -7,7 +7,7 @@ import type {
   TraceContext,
   Transaction,
 } from '@sentry/types';
-import { dropUndefinedKeys, logger, timestampInSeconds, uuid4 } from '@sentry/utils';
+import { dropUndefinedKeys, generateSentryTraceHeader, logger, timestampInSeconds, uuid4 } from '@sentry/utils';
 
 /**
  * Keeps track of finished spans for a given transaction
@@ -265,11 +265,7 @@ export class Span implements SpanInterface {
    * @inheritDoc
    */
   public toTraceparent(): string {
-    let sampledString = '';
-    if (this.sampled !== undefined) {
-      sampledString = this.sampled ? '-1' : '-0';
-    }
-    return `${this.traceId}-${this.spanId}${sampledString}`;
+    return generateSentryTraceHeader(this.traceId, this.spanId, this.sampled);
   }
 
   /**

diff --git a/packages/core/src/tracing/transaction.ts b/packages/core/src/tracing/transaction.ts
@@ -11,9 +11,9 @@ import type {
 } from '@sentry/types';
 import { dropUndefinedKeys, logger } from '@sentry/utils';
 
-import { DEFAULT_ENVIRONMENT } from '../constants';
 import type { Hub } from '../hub';
 import { getCurrentHub } from '../hub';
+import { getDynamicSamplingContextFromClient } from './dynamicSamplingContext';
 import { Span as SpanClass, SpanRecorder } from './span';
 
 /** JSDoc */
@@ -245,33 +245,24 @@ export class Transaction extends SpanClass implements TransactionInterface {
       return this._frozenDynamicSamplingContext;
     }
 
-    const hub: Hub = this._hub || getCurrentHub();
-    const client = hub && hub.getClient();
+    const hub = this._hub || getCurrentHub();
+    const client = hub.getClient();
 
     if (!client) return {};
 
-    const { environment, release } = client.getOptions() || {};
-    const { publicKey: public_key } = client.getDsn() || {};
+    const scope = hub.getScope();
+    const dsc = getDynamicSamplingContextFromClient(this.traceId, client, scope);
 
     const maybeSampleRate = this.metadata.sampleRate;
-    const sample_rate = maybeSampleRate !== undefined ? maybeSampleRate.toString() : undefined;
-
-    const { segment: user_segment } = hub.getScope().getUser() || {};
-
-    const source = this.metadata.source;
+    if (maybeSampleRate !== undefined) {
+      dsc.sample_rate = `${maybeSampleRate}`;
+    }
 
     // We don't want to have a transaction name in the DSC if the source is "url" because URLs might contain PII
-    const transaction = source && source !== 'url' ? this.name : undefined;
-
-    const dsc = dropUndefinedKeys({
-      environment: environment || DEFAULT_ENVIRONMENT,
-      release,
-      transaction,
-      user_segment,
-      public_key,
-      trace_id: this.traceId,
-      sample_rate,
-    });
+    const source = this.metadata.source;
+    if (source && source !== 'url') {
+      dsc.transaction = this.name;
+    }
 
     // Uncomment if we want to make DSC immutable
     // this._frozenDynamicSamplingContext = dsc;

diff --git a/packages/hub/test/scope.test.ts b/packages/hub/test/scope.test.ts
@@ -152,6 +152,22 @@ describe('Scope', () => {
       expect((scope as any)._sdkProcessingMetadata.dogs).toEqual('are great!');
     });
 
+    test('set and get propagation context', () => {
+      const scope = new Scope();
+      const oldPropagationContext = scope.getPropagationContext();
+      scope.setPropagationContext({
+        traceId: '86f39e84263a4de99c326acab3bfe3bd',
+        spanId: '6e0c63257de34c92',
+        sampled: true,
+      });
+      expect(scope.getPropagationContext()).not.toEqual(oldPropagationContext);
+      expect(scope.getPropagationContext()).toEqual({
+        traceId: '86f39e84263a4de99c326acab3bfe3bd',
+        spanId: '6e0c63257de34c92',
+        sampled: true,
+      });
+    });
+
     test('chaining', () => {
       const scope = new Scope();
       scope.setLevel('fatal').setUser({ id: '1' });

diff --git a/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts b/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-assign/test.ts
@@ -76,8 +76,8 @@ test('Should populate and propagate sentry baggage if sentry-trace header does n
     test_data: {
       host: 'somewhere.not.sentry',
       // TraceId changes, hence we only expect that the string contains the traceid key
-      baggage: expect.stringContaining(
-        'sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,sentry-public_key=public,sentry-trace_id=',
+      baggage: expect.stringMatching(
+        /sentry-environment=prod,sentry-release=1.0,sentry-public_key=public,sentry-trace_id=[\S]*,sentry-sample_rate=1,sentry-transaction=GET%20%2Ftest%2Fexpress/,
       ),
     },
   });
@@ -95,8 +95,8 @@ test('Should populate Sentry and ignore 3rd party content if sentry-trace header
     test_data: {
       host: 'somewhere.not.sentry',
       // TraceId changes, hence we only expect that the string contains the traceid key
-      baggage: expect.stringContaining(
-        'sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,sentry-public_key=public,sentry-trace_id=',
+      baggage: expect.stringMatching(
+        /sentry-environment=prod,sentry-release=1.0,sentry-public_key=public,sentry-trace_id=[\S]*,sentry-sample_rate=1,sentry-transaction=GET%20%2Ftest%2Fexpress/,
       ),
     },
   });

diff --git a/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts b/packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts
@@ -13,8 +13,8 @@ test('should attach a `baggage` header to an outgoing request.', async () => {
     test_data: {
       host: 'somewhere.not.sentry',
       baggage:
-        'sentry-environment=prod,sentry-release=1.0,sentry-transaction=GET%20%2Ftest%2Fexpress,sentry-user_segment=SegmentA' +
-        ',sentry-public_key=public,sentry-trace_id=86f39e84263a4de99c326acab3bfe3bd,sentry-sample_rate=1',
+        'sentry-environment=prod,sentry-release=1.0,sentry-user_segment=SegmentA,sentry-public_key=public' +
+        ',sentry-trace_id=86f39e84263a4de99c326acab3bfe3bd,sentry-sample_rate=1,sentry-transaction=GET%20%2Ftest%2Fexpress',
     },
   });
 });
diff --git a/...ation-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts b/...ation-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts
@@ -35,7 +35,7 @@ test('should ignore sentry-values in `baggage` header of a third party vendor an
       baggage: [
         'other=vendor,foo=bar,third=party,sentry-release=9.9.9,sentry-environment=staging,sentry-sample_rate=0.54,last=item',
         expect.stringMatching(
-          /sentry-environment=prod,sentry-release=1\.0,sentry-transaction=GET%20%2Ftest%2Fexpress,sentry-public_key=public,sentry-trace_id=[0-9a-f]{32},sentry-sample_rate=1/,
+          /sentry-environment=prod,sentry-release=1\.0,sentry-public_key=public,sentry-trace_id=[0-9a-f]{32},sentry-sample_rate=1,sentry-transaction=GET%20%2Ftest%2Fexpress/,
         ),
       ],
     },

diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts
@@ -114,9 +114,9 @@ describe('tracing', () => {
     const baggageHeader = request.getHeader('baggage') as string;
 
     expect(baggageHeader).toEqual(
-      'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,' +
+      'sentry-environment=production,sentry-release=1.0.0,' +
         'sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,' +
-        'sentry-trace_id=12312012123120121231201212312012,sentry-sample_rate=1',
+        'sentry-trace_id=12312012123120121231201212312012,sentry-sample_rate=1,sentry-transaction=dogpark',
     );
   });
 
@@ -130,7 +130,7 @@ describe('tracing', () => {
 
     expect(baggageHeader).toEqual([
       'dog=great',
-      'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,sentry-trace_id=12312012123120121231201212312012,sentry-sample_rate=1',
+      'sentry-environment=production,sentry-release=1.0.0,sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,sentry-trace_id=12312012123120121231201212312012,sentry-sample_rate=1,sentry-transaction=dogpark',
     ]);
   });
 
@@ -144,7 +144,7 @@ describe('tracing', () => {
 
     expect(baggageHeader).toEqual([
       'dog=great',
-      'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dogpark,sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,sentry-trace_id=12312012123120121231201212312012,sentry-sample_rate=1',
+      'sentry-environment=production,sentry-release=1.0.0,sentry-user_segment=segmentA,sentry-public_key=dogsarebadatkeepingsecrets,sentry-trace_id=12312012123120121231201212312012,sentry-sample_rate=1,sentry-transaction=dogpark',
     ]);
   });
 

diff --git a/packages/node/test/integrations/undici.test.ts b/packages/node/test/integrations/undici.test.ts
@@ -204,7 +204,7 @@ conditionalTest({ min: 16 })('Undici integration', () => {
 
     expect(requestHeaders['sentry-trace']).toEqual(span?.toTraceparent());
     expect(requestHeaders['baggage']).toEqual(
-      `sentry-environment=production,sentry-transaction=test-transaction,sentry-public_key=0,sentry-trace_id=${transaction.traceId},sentry-sample_rate=1`,
+      `sentry-environment=production,sentry-public_key=0,sentry-trace_id=${transaction.traceId},sentry-sample_rate=1,sentry-transaction=test-transaction`,
     );
   });
 

diff --git a/packages/opentelemetry-node/test/propagator.test.ts b/packages/opentelemetry-node/test/propagator.test.ts
@@ -85,7 +85,7 @@ describe('SentryPropagator', () => {
               spanId: '6e0c63257de34c92',
               sampled: true,
             },
-            'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=sampled-transaction,sentry-public_key=abc,sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b',
+            'sentry-environment=production,sentry-release=1.0.0,sentry-public_key=abc,sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b,sentry-transaction=sampled-transaction',
             'd4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-1',
           ],
           [
@@ -101,7 +101,7 @@ describe('SentryPropagator', () => {
               spanId: '6e0c63257de34c92',
               sampled: false,
             },
-            'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=not-sampled-transaction,sentry-public_key=abc,sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b',
+            'sentry-environment=production,sentry-release=1.0.0,sentry-public_key=abc,sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b,sentry-transaction=not-sampled-transaction',
             'd4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-0',
           ],
           [
@@ -161,7 +161,7 @@ describe('SentryPropagator', () => {
           const baggage = propagation.createBaggage({ foo: { value: 'bar' } });
           propagator.inject(propagation.setBaggage(context, baggage), carrier, defaultTextMapSetter);
           expect(carrier[SENTRY_BAGGAGE_HEADER]).toBe(
-            'foo=bar,sentry-environment=production,sentry-release=1.0.0,sentry-transaction=sampled-transaction,sentry-public_key=abc,sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b',
+            'foo=bar,sentry-environment=production,sentry-release=1.0.0,sentry-public_key=abc,sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b,sentry-transaction=sampled-transaction',
           );
         });
 
@@ -232,7 +232,7 @@ describe('SentryPropagator', () => {
 
     it('sets defined dynamic sampling context on context', () => {
       const baggage =
-        'sentry-environment=production,sentry-release=1.0.0,sentry-transaction=dsc-transaction,sentry-public_key=abc,sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b';
+        'sentry-environment=production,sentry-release=1.0.0,sentry-public_key=abc,sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b,sentry-transaction=dsc-transaction';
       carrier[SENTRY_BAGGAGE_HEADER] = baggage;
       const context = propagator.extract(ROOT_CONTEXT, carrier, defaultTextMapGetter);
       expect(context.getValue(SENTRY_DYNAMIC_SAMPLING_CONTEXT_KEY)).toEqual({

diff --git a/packages/tracing/test/utils.test.ts b/packages/tracing/test/utils.test.ts
@@ -77,6 +77,9 @@ describe('extractTraceparentData', () => {
   });
 
   test('invalid', () => {
+    // undefined
+    expect(extractTraceparentData(undefined)).toBeUndefined();
+
     // empty string
     expect(extractTraceparentData('')).toBeUndefined();
 

diff --git a/packages/types/src/scope.ts b/packages/types/src/scope.ts
@@ -192,4 +192,9 @@ export interface Scope {
    * Add propagation context to the scope, used for distributed tracing
    */
   setPropagationContext(context: PropagationContext): this;
+
+  /**
+   * Get propagation context from the scope, used for distributed tracing
+   */
+  getPropagationContext(): PropagationContext;
 }
diff --git a/packages/utils/src/baggage.ts b/packages/utils/src/baggage.ts
@@ -83,8 +83,12 @@ export function baggageHeaderToDynamicSamplingContext(
  */
 export function dynamicSamplingContextToSentryBaggageHeader(
   // this also takes undefined for convenience and bundle size in other places
-  dynamicSamplingContext: Partial<DynamicSamplingContext>,
+  dynamicSamplingContext?: Partial<DynamicSamplingContext>,
 ): string | undefined {
+  if (!dynamicSamplingContext) {
+    return undefined;
+  }
+
   // Prefix all DSC keys with "sentry-" and put them into a new object
   const sentryPrefixedDSC = Object.entries(dynamicSamplingContext).reduce<Record<string, string>>(
     (acc, [dscKey, dscValue]) => {

diff --git a/packages/utils/src/tracing.ts b/packages/utils/src/tracing.ts
@@ -46,8 +46,8 @@ export function extractTraceparentData(traceparent?: string): TraceparentData |
  * Create tracing context from incoming headers.
  */
 export function tracingContextFromHeaders(
-  sentryTrace: Parameters<typeof extractTraceparentData>[0] = '',
-  baggage: Parameters<typeof baggageHeaderToDynamicSamplingContext>[0] = '',
+  sentryTrace: Parameters<typeof extractTraceparentData>[0],
+  baggage: Parameters<typeof baggageHeaderToDynamicSamplingContext>[0],
 ): {
   traceparentData: ReturnType<typeof extractTraceparentData>;
   dynamicSamplingContext: ReturnType<typeof baggageHeaderToDynamicSamplingContext>;
@@ -78,3 +78,18 @@ export function tracingContextFromHeaders(
     propagationContext,
   };
 }
+
+/**
+ * Create sentry-trace header from span context values.
+ */
+export function generateSentryTraceHeader(
+  traceId: string = uuid4(),
+  spanId: string = uuid4().substring(16),
+  sampled?: boolean,
+): string {
+  let sampledString = '';
+  if (sampled !== undefined) {
+    sampledString = sampled ? '-1' : '-0';
+  }
+  return `${traceId}-${spanId}${sampledString}`;
+}
diff --git a/packages/utils/test/baggage.test.ts b/packages/utils/test/baggage.test.ts
@@ -28,6 +28,7 @@ test.each([
 });
 
 test.each([
+  [undefined, undefined],
   [{}, undefined],
   [{ release: 'abcdf' }, 'sentry-release=abcdf'],
   [{ release: 'abcdf', environment: '1234' }, 'sentry-release=abcdf,sentry-environment=1234'],