ref(tracing): Rework tracestate internals to allow for third-party data (#3266)

Author: lobsterkatie

packages/core/src/request.ts

@@ -67,7 +67,7 @@ export function eventToSentryRequest(event: Event, api: API): SentryRequest {
 export function transactionToSentryRequest(event: Event, api: API): SentryRequest {
   const sdkInfo = getSdkMetadataForEnvelopeHeader(api);
 
-  const { transactionSampling, tracestate: encodedTracestate, ...metadata } = event.debug_meta || {};
+  const { transactionSampling, tracestate, ...metadata } = event.debug_meta || {};
   const { method: samplingMethod, rate: sampleRate } = transactionSampling || {};
   if (Object.keys(metadata).length === 0) {
     delete event.debug_meta;
@@ -77,9 +77,14 @@ export function transactionToSentryRequest(event: Event, api: API): SentryReques
 
   // the tracestate is stored in bas64-encoded JSON, but envelope header values are expected to be full JS values,
   // so we have to decode and reinflate it
-  let tracestate;
+  let reinflatedTracestate;
   try {
-    tracestate = JSON.parse(base64ToUnicode(encodedTracestate as string));
+    // Because transaction metadata passes through a number of locations (transactionContext, transaction, event during
+    // processing, event as sent), each with different requirements, all of the parts are typed as optional. That said,
+    // if we get to this point and either `tracestate` or `tracestate.sentry` are undefined, something's gone very wrong.
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    const encodedSentryValue = tracestate!.sentry!.replace('sentry=', '');
+    reinflatedTracestate = JSON.parse(base64ToUnicode(encodedSentryValue));
   } catch (err) {
     logger.warn(err);
   }
@@ -88,7 +93,7 @@ export function transactionToSentryRequest(event: Event, api: API): SentryReques
     event_id: event.event_id,
     sent_at: new Date().toISOString(),
     ...(sdkInfo && { sdk: sdkInfo }),
-    ...(tracestate && { trace: tracestate }), // trace context for dynamic sampling on relay
+    ...(reinflatedTracestate && { trace: reinflatedTracestate }), // trace context for dynamic sampling on relay
   });
 
   const itemHeaders = JSON.stringify({

packages/core/test/lib/request.test.ts

@@ -68,9 +68,11 @@ describe('eventToSentryRequest', () => {
           //   release: 'off.leash.park',
           //   public_key: 'dogsarebadatkeepingsecrets',
           // }),
-          tracestate:
-            'eyJ0cmFjZV9pZCI6IjEyMzEyMDEyMTEyMTIwMTIiLCJlbnZpcm9ubWVudCI6ImRvZ3BhcmsiLCJyZWxlYXNlIjoib2ZmLmxlYXNo' +
-            'LnBhcmsiLCJwdWJsaWNfa2V5IjoiZG9nc2FyZWJhZGF0a2VlcGluZ3NlY3JldHMifQ',
+          tracestate: {
+            sentry:
+              'sentry=eyJ0cmFjZV9pZCI6IjEyMzEyMDEyMTEyMTIwMTIiLCJlbnZpcm9ubWVudCI6ImRvZ3BhcmsiLCJyZWxlYXNlIjoib2ZmLmxlYXNo' +
+              'LnBhcmsiLCJwdWJsaWNfa2V5IjoiZG9nc2FyZWJhZGF0a2VlcGluZ3NlY3JldHMifQ',
+          },
         },
         spans: [],
         transaction: '/dogs/are/great/',

packages/tracing/src/browser/request.ts

@@ -1,7 +1,7 @@
 import { getCurrentHub } from '@sentry/hub';
+import { Span } from '@sentry/types';
 import { addInstrumentationHandler, isInstanceOf, isMatchingPattern } from '@sentry/utils';
 
-import { Span } from '../span';
 import { getActiveTransaction, hasTracingEnabled } from '../utils';
 
 export const DEFAULT_TRACING_ORIGINS = ['localhost', /^\//];

packages/tracing/src/span.ts

@@ -1,8 +1,10 @@
 /* eslint-disable max-lines */
-import { Primitive, Span as SpanInterface, SpanContext, TraceHeaders, Transaction } from '@sentry/types';
+import { getCurrentHub } from '@sentry/hub';
+import { Hub, Primitive, Span as SpanInterface, SpanContext, TraceHeaders, Transaction } from '@sentry/types';
 import { dropUndefinedKeys, timestampWithMs, uuid4 } from '@sentry/utils';
 
 import { SpanStatus } from './spanstatus';
+import { computeTracestateValue } from './utils';
 
 /**
  * Keeps track of finished spans for a given transaction
@@ -289,7 +291,12 @@ export class Span implements SpanInterface {
    */
   public getTraceHeaders(): TraceHeaders {
     // tracestates live on the transaction, so if this is a free-floating span, there won't be one
-    const tracestate = this.transaction && `sentry=${this.transaction.tracestate}`; // TODO kmclb
+    let tracestate;
+    if (this.transaction) {
+      tracestate = this.transaction.metadata?.tracestate?.sentry;
+    } else {
+      tracestate = this._getNewTracestate();
+    }
 
     return {
       'sentry-trace': this.toTraceparent(),
@@ -352,4 +359,32 @@ export class Span implements SpanInterface {
       trace_id: this.traceId,
     });
   }
+
+  /**
+   * Create a new Sentry tracestate header entry (i.e. `sentry=xxxxxx`)
+   *
+   * @returns The new Sentry tracestate entry, or undefined if there's no client or no dsn
+   */
+  protected _getNewTracestate(hub: Hub = getCurrentHub()): string | undefined {
+    const client = hub.getClient();
+    const dsn = client?.getDsn();
+
+    if (!client || !dsn) {
+      return;
+    }
+
+    const { environment, release } = client.getOptions() || {};
+
+    // TODO - the only reason we need the non-null assertion on `dsn.publicKey` (below) is because `dsn.publicKey` has
+    // to be optional while we transition from `dsn.user` -> `dsn.publicKey`. Once `dsn.user` is removed, we can make
+    // `dsn.publicKey` required and remove the `!`.
+
+    return `sentry=${computeTracestateValue({
+      trace_id: this.traceId,
+      environment,
+      release,
+      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+      public_key: dsn.publicKey!,
+    })}`;
+  }
 }

packages/tracing/src/transaction.ts

@@ -1,19 +1,20 @@
 import { getCurrentHub, Hub } from '@sentry/hub';
-import { DebugMeta, Event, Measurements, Transaction as TransactionInterface, TransactionContext } from '@sentry/types';
+import {
+  Event,
+  Measurements,
+  Transaction as TransactionInterface,
+  TransactionContext,
+  TransactionMetadata,
+} from '@sentry/types';
 import { dropUndefinedKeys, isInstanceOf, logger } from '@sentry/utils';
 
 import { Span as SpanClass, SpanRecorder } from './span';
-import { computeTracestateValue } from './utils';
-
-type TransactionMetadata = Pick<DebugMeta, 'transactionSampling' | 'tracestate'>;
 
 /** JSDoc */
 export class Transaction extends SpanClass implements TransactionInterface {
   public name: string;
 
-  public readonly tracestate: string;
-
-  private _metadata: TransactionMetadata = {};
+  public metadata: TransactionMetadata;
 
   private _measurements: Measurements = {};
 
@@ -42,9 +43,9 @@ export class Transaction extends SpanClass implements TransactionInterface {
 
     this._trimEnd = transactionContext.trimEnd;
 
-    // _getNewTracestate only returns undefined in the absence of a client or dsn, in which case it doesn't matter what
-    // the header values are - nothing can be sent anyway - so the third alternative here is just to make TS happy
-    this.tracestate = transactionContext.tracestate || this._getNewTracestate() || 'things are broken';
+    this.metadata = transactionContext.metadata || {};
+    this.metadata.tracestate = this.metadata.tracestate || {};
+    this.metadata.tracestate.sentry = this.metadata.tracestate.sentry || this._getNewTracestate(this._hub);
 
     // this is because transactions are also spans, and spans have a transaction pointer
     this.transaction = this;
@@ -81,7 +82,7 @@ export class Transaction extends SpanClass implements TransactionInterface {
    * @hidden
    */
   public setMetadata(newMetadata: TransactionMetadata): void {
-    this._metadata = { ...this._metadata, ...newMetadata };
+    this.metadata = { ...this.metadata, ...newMetadata };
   }
 
   /**
@@ -118,8 +119,6 @@ export class Transaction extends SpanClass implements TransactionInterface {
       }).endTimestamp;
     }
 
-    this._metadata.tracestate = this.tracestate;
-
     const transaction: Event = {
       contexts: {
         trace: this.getTraceContext(),
@@ -130,7 +129,7 @@ export class Transaction extends SpanClass implements TransactionInterface {
       timestamp: this.endTimestamp,
       transaction: this.name,
       type: 'transaction',
-      debug_meta: this._metadata,
+      debug_meta: this.metadata,
     };
 
     const hasMeasurements = Object.keys(this._measurements).length > 0;
@@ -170,27 +169,4 @@ export class Transaction extends SpanClass implements TransactionInterface {
 
     return this;
   }
-
-  /**
-   * Create a new tracestate header value
-   *
-   * @returns The new tracestate value, or undefined if there's no client or no dsn
-   */
-  private _getNewTracestate(): string | undefined {
-    const client = this._hub.getClient();
-    const dsn = client?.getDsn();
-
-    if (!client || !dsn) {
-      return;
-    }
-
-    const { environment, release } = client.getOptions() || {};
-
-    // TODO - the only reason we need the non-null assertion on `dsn.publicKey` (below) is because `dsn.publicKey` has
-    // to be optional while we transition from `dsn.user` -> `dsn.publicKey`. Once `dsn.user` is removed, we can make
-    // `dsn.publicKey` required and remove the `!`.
-
-    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-    return computeTracestateValue({ trace_id: this.traceId, environment, release, public_key: dsn.publicKey! });
-  }
 }

packages/tracing/src/utils.ts

@@ -68,22 +68,23 @@ export function secToMs(time: number): number {
 // so it can be used in manual instrumentation without necessitating a hard dependency on @sentry/utils
 export { stripUrlQueryAndFragment } from '@sentry/utils';
 
+type TracestateData = {
+  trace_id: string;
+  environment: string | undefined | null;
+  release: string | undefined | null;
+  public_key: string;
+};
+
 /**
- * Compute the value of a tracestate header.
+ * Compute the value of a Sentry tracestate header.
  *
  * @throws SentryError (because using the logger creates a circular dependency)
  * @returns the base64-encoded header value
  */
-// Note: this is here instead of in the tracing package since @sentry/core tests rely on it
-export function computeTracestateValue(tracestateData: {
-  trace_id: string;
-  environment: string | undefined | null;
-  release: string | undefined | null;
-  public_key: string;
-}): string {
+export function computeTracestateValue(data: TracestateData): string {
   // `JSON.stringify` will drop keys with undefined values, but not ones with null values
-  tracestateData.environment = tracestateData.environment || null;
-  tracestateData.release = tracestateData.release || null;
+  data.environment = data.environment || null;
+  data.release = data.release || null;
 
   // See https://www.w3.org/TR/trace-context/#tracestate-header-field-values
   // The spec for tracestate header values calls for a string of the form
@@ -94,8 +95,8 @@ export function computeTracestateValue(tracestateData: {
   // used to pad the end of base64 values though, so to avoid confusion, we strip them off. (Most languages' base64
   // decoding functions (including those in JS) are able to function without the padding.)
   try {
-    return unicodeToBase64(JSON.stringify(tracestateData)).replace(/={1,2}$/, '');
+    return unicodeToBase64(JSON.stringify(data)).replace(/={1,2}$/, '');
   } catch (err) {
-    throw new SentryError(`[Tracing] Error creating tracestate header: ${err}`);
+    throw new SentryError(`[Tracing] Error computing tracestate value from data: ${err}\nData: ${data}`);
   }
 }

packages/tracing/test/hub.test.ts

@@ -35,12 +35,19 @@ describe('Hub', () => {
         name: 'dogpark',
         traceId: '12312012123120121231201212312012',
         parentSpanId: '1121201211212012',
-        tracestate: 'doGsaREgReaT',
+        metadata: { tracestate: { sentry: 'sentry=doGsaREgReaT' } },
       };
       const hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
       const transaction = hub.startTransaction(transactionContext);
 
-      expect(transaction).toEqual(expect.objectContaining(transactionContext));
+      expect(transaction).toEqual(
+        expect.objectContaining({
+          name: 'dogpark',
+          traceId: '12312012123120121231201212312012',
+          parentSpanId: '1121201211212012',
+          metadata: expect.objectContaining({ tracestate: { sentry: 'sentry=doGsaREgReaT' } }),
+        }),
+      );
     });
 
     it('creates a new tracestate value if not given one in transaction context', () => {
@@ -62,7 +69,7 @@ describe('Hub', () => {
         public_key: 'dogsarebadatkeepingsecrets',
       });
 
-      expect(transaction.tracestate).toEqual(b64Value);
+      expect(transaction.metadata?.tracestate?.sentry).toEqual(`sentry=${b64Value}`);
     });
 
     it('uses default environment if none given', () => {
@@ -80,7 +87,7 @@ describe('Hub', () => {
         public_key: 'dogsarebadatkeepingsecrets',
       });
 
-      expect(transaction.tracestate).toEqual(b64Value);
+      expect(transaction.metadata?.tracestate?.sentry).toEqual(`sentry=${b64Value}`);
     });
   });
 

packages/tracing/test/span.test.ts

@@ -117,7 +117,7 @@ describe('Span', () => {
       const headers = span.getTraceHeaders();
 
       expect(headers['sentry-trace']).toEqual(span.toTraceparent());
-      expect(headers.tracestate).toEqual(`sentry=${transaction.tracestate}`);
+      expect(headers.tracestate).toEqual(transaction.metadata?.tracestate?.sentry);
     });
   });
 

packages/types/src/debugMeta.ts

@@ -1,13 +1,11 @@
+import { TransactionMetadata } from './transaction';
+
 /**
  * Holds meta information to customize the behavior of sentry's event processing.
  **/
-export interface DebugMeta {
+export type DebugMeta = {
   images?: Array<DebugImage>;
-  transactionSampling?: { rate?: number; method?: string };
-
-  /** The Sentry portion of a transaction's tracestate header, used for dynamic sampling */
-  tracestate?: string;
-}
+} & TransactionMetadata;
 
 /**
  * Possible choices for debug images.

packages/types/src/index.ts

@@ -34,6 +34,7 @@ export {
   TraceparentData,
   Transaction,
   TransactionContext,
+  TransactionMetadata,
   TransactionSamplingMethod,
 } from './transaction';
 export { Thread } from './thread';