ref(tracing): Move getBaggage() from Span to Transaction class (#5299)

Move the getBaggage() method from the Span class, to the Transaction class. This change is good for two reasons:

* It conceptually makes more sense to store DSC on the transaction rather than the span because we're always talking about transactions in DS
* We can get rid of a sketchy type cast and avoid a circular dependency situation (transaction -> span -> transaction) entirely

Author: Lms24

packages/node/src/integrations/http.ts

@@ -123,12 +123,13 @@ function _createWrappedRequestMethodFactory(
               `[Tracing] Adding sentry-trace header ${sentryTraceHeader} to outgoing request to ${requestUrl}: `,
             );
 
+          const baggage = parentSpan.transaction && parentSpan.transaction.getBaggage();
           const headerBaggageString = requestOptions.headers && requestOptions.headers.baggage;
 
           requestOptions.headers = {
             ...requestOptions.headers,
             'sentry-trace': sentryTraceHeader,
-            baggage: mergeAndSerializeBaggage(span.getBaggage(), headerBaggageString),
+            baggage: mergeAndSerializeBaggage(baggage, headerBaggageString),
           };
         }
       }

packages/tracing/src/browser/request.ts

@@ -1,5 +1,5 @@
 /* eslint-disable max-lines */
-import type { Span } from '@sentry/types';
+import type { Baggage, Span } from '@sentry/types';
 import {
   addInstrumentationHandler,
   BAGGAGE_HEADER_NAME,
@@ -198,12 +198,13 @@ export function fetchCallback(
     const request = (handlerData.args[0] = handlerData.args[0] as string | Request);
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     const options = (handlerData.args[1] = (handlerData.args[1] as { [key: string]: any }) || {});
-    options.headers = addTracingHeaders(request, span, options);
+    options.headers = addTracingHeaders(request, activeTransaction.getBaggage(), span, options);
   }
 }
 
 function addTracingHeaders(
   request: string | Request,
+  incomingBaggage: Baggage | undefined,
   span: Span,
   options: { [key: string]: any },
 ): PolymorphicRequestHeaders {
@@ -212,7 +213,6 @@ function addTracingHeaders(
   if (isInstanceOf(request, Request)) {
     headers = (request as Request).headers;
   }
-  const incomingBaggage = span.getBaggage();
 
   if (headers) {
     // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
@@ -302,7 +302,7 @@ export function xhrCallback(
 
         handlerData.xhr.setRequestHeader(
           BAGGAGE_HEADER_NAME,
-          mergeAndSerializeBaggage(span.getBaggage(), headerBaggageString),
+          mergeAndSerializeBaggage(activeTransaction.getBaggage(), headerBaggageString),
         );
       } catch (_) {
         // Error: InvalidStateError: Failed to execute 'setRequestHeader' on 'XMLHttpRequest': The object's state must be OPENED.

packages/tracing/src/span.ts

@@ -1,16 +1,6 @@
 /* eslint-disable max-lines */
-import { getCurrentHub } from '@sentry/hub';
-import { Baggage, Hub, Primitive, Span as SpanInterface, SpanContext, Transaction } from '@sentry/types';
-import {
-  createBaggage,
-  dropUndefinedKeys,
-  isBaggageMutable,
-  isSentryBaggageEmpty,
-  setBaggageImmutable,
-  setBaggageValue,
-  timestampWithMs,
-  uuid4,
-} from '@sentry/utils';
+import { Primitive, Span as SpanInterface, SpanContext, Transaction } from '@sentry/types';
+import { dropUndefinedKeys, timestampWithMs, uuid4 } from '@sentry/utils';
 
 /**
  * Keeps track of finished spans for a given transaction
@@ -308,29 +298,6 @@ export class Span implements SpanInterface {
     });
   }
 
-  /**
-   * @inheritdoc
-   */
-  public getBaggage(): Baggage {
-    const existingBaggage = this.transaction && this.transaction.metadata.baggage;
-
-    // Only add Sentry baggage items to baggage, if baggage does not exist yet or it is still
-    // empty and mutable
-    // TODO: we might want to ditch the isSentryBaggageEmpty condition because it prevents
-    //       custom sentry-values in DSC (added by users in the future)
-    const finalBaggage =
-      !existingBaggage || (isBaggageMutable(existingBaggage) && isSentryBaggageEmpty(existingBaggage))
-        ? this._populateBaggageWithSentryValues(existingBaggage)
-        : existingBaggage;
-
-    // In case, we poulated the DSC, we have update the stored one on the transaction.
-    if (existingBaggage !== finalBaggage && this.transaction) {
-      this.transaction.metadata.baggage = finalBaggage;
-    }
-
-    return finalBaggage;
-  }
-
   /**
    * @inheritDoc
    */
@@ -360,65 +327,6 @@ export class Span implements SpanInterface {
       trace_id: this.traceId,
     });
   }
-
-  /**
-   * Collects and adds data to the passed baggage object.
-   *
-   * Note: This function does not explicitly check if the passed baggage object is allowed
-   * to be modified. Implicitly, `setBaggageValue` will not make modification to the object
-   * if it was already set immutable.
-   *
-   * After adding the data, the baggage object is set immutable to prevent further modifications.
-   *
-   * @param baggage
-   *
-   * @returns modified and immutable baggage object
-   */
-  private _populateBaggageWithSentryValues(baggage: Baggage = createBaggage({})): Baggage {
-    // Because of a cicular dependency, we cannot import the Transaction class here, hence the type casts
-    // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-explicit-any
-    const hub: Hub = ((this.transaction as any) && (this.transaction as any)._hub) || getCurrentHub();
-    const client = hub && hub.getClient();
-
-    const { environment, release } = (client && client.getOptions()) || {};
-    const { publicKey } = (client && client.getDsn()) || {};
-
-    const metadata = this.transaction && this.transaction.metadata;
-    const sampleRate = metadata && metadata.transactionSampling && metadata.transactionSampling.rate;
-
-    const traceId = this.traceId;
-
-    const transactionName = this.transaction && this.transaction.name;
-
-    let userId, userSegment;
-    hub.withScope(scope => {
-      const user = scope.getUser();
-      userId = user && user.id;
-      userSegment = user && user.segment;
-    });
-
-    environment && setBaggageValue(baggage, 'environment', environment);
-    release && setBaggageValue(baggage, 'release', release);
-    transactionName && setBaggageValue(baggage, 'transaction', transactionName);
-    userId && setBaggageValue(baggage, 'userid', userId);
-    userSegment && setBaggageValue(baggage, 'usersegment', userSegment);
-    sampleRate &&
-      setBaggageValue(
-        baggage,
-        'samplerate',
-        // This will make sure that expnent notation (e.g. 1.45e-14) is converted to simple decimal representation
-        // Another edge case would be something like Number.NEGATIVE_INFINITY in which case we could still
-        // add something like .replace(/-?∞/, '0'). For the sake of saving bytes, I'll not add this until
-        // it becomes a problem
-        sampleRate.toLocaleString('fullwide', { useGrouping: false, maximumFractionDigits: 16 }),
-      );
-    publicKey && setBaggageValue(baggage, 'publickey', publicKey);
-    traceId && setBaggageValue(baggage, 'traceid', traceId);
-
-    setBaggageImmutable(baggage);
-
-    return baggage;
-  }
 }
 
 export type SpanStatusType =

packages/tracing/src/transaction.ts

@@ -1,12 +1,21 @@
 import { getCurrentHub, Hub } from '@sentry/hub';
 import {
+  Baggage,
   Event,
   Measurements,
   Transaction as TransactionInterface,
   TransactionContext,
   TransactionMetadata,
 } from '@sentry/types';
-import { dropUndefinedKeys, logger } from '@sentry/utils';
+import {
+  createBaggage,
+  dropUndefinedKeys,
+  isBaggageMutable,
+  isSentryBaggageEmpty,
+  logger,
+  setBaggageImmutable,
+  setBaggageValue,
+} from '@sentry/utils';
 
 import { Span as SpanClass, SpanRecorder } from './span';
 
@@ -176,4 +185,83 @@ export class Transaction extends SpanClass implements TransactionInterface {
 
     return this;
   }
+
+  /**
+   * @inheritdoc
+   *
+   * @experimental
+   */
+  public getBaggage(): Baggage {
+    const existingBaggage = this.metadata.baggage;
+
+    // Only add Sentry baggage items to baggage, if baggage does not exist yet or it is still
+    // empty and mutable
+    // TODO: we might want to ditch the isSentryBaggageEmpty condition because it prevents
+    //       custom sentry-values in DSC (added by users in the future)
+    const finalBaggage =
+      !existingBaggage || (isBaggageMutable(existingBaggage) && isSentryBaggageEmpty(existingBaggage))
+        ? this._populateBaggageWithSentryValues(existingBaggage)
+        : existingBaggage;
+
+    // In case, we poulated the DSC, we have update the stored one on the transaction.
+    if (existingBaggage !== finalBaggage) {
+      this.metadata.baggage = finalBaggage;
+    }
+
+    return finalBaggage;
+  }
+
+  /**
+   * Collects and adds data to the passed baggage object.
+   *
+   * Note: This function does not explicitly check if the passed baggage object is allowed
+   * to be modified. Implicitly, `setBaggageValue` will not make modification to the object
+   * if it was already set immutable.
+   *
+   * After adding the data, the baggage object is set immutable to prevent further modifications.
+   *
+   * @param baggage
+   *
+   * @returns modified and immutable baggage object
+   */
+  private _populateBaggageWithSentryValues(baggage: Baggage = createBaggage({})): Baggage {
+    const hub: Hub = this._hub || getCurrentHub();
+    const client = hub && hub.getClient();
+
+    const { environment, release } = (client && client.getOptions()) || {};
+    const { publicKey } = (client && client.getDsn()) || {};
+
+    const sampleRate = this.metadata && this.metadata.transactionSampling && this.metadata.transactionSampling.rate;
+    const traceId = this.traceId;
+    const transactionName = this.name;
+
+    let userId, userSegment;
+    hub.configureScope(scope => {
+      const { id, segment } = scope.getUser() || {};
+      userId = id;
+      userSegment = segment;
+    });
+
+    environment && setBaggageValue(baggage, 'environment', environment);
+    release && setBaggageValue(baggage, 'release', release);
+    transactionName && setBaggageValue(baggage, 'transaction', transactionName);
+    userId && setBaggageValue(baggage, 'userid', userId);
+    userSegment && setBaggageValue(baggage, 'usersegment', userSegment);
+    sampleRate &&
+      setBaggageValue(
+        baggage,
+        'samplerate',
+        // This will make sure that expnent notation (e.g. 1.45e-14) is converted to simple decimal representation
+        // Another edge case would be something like Number.NEGATIVE_INFINITY in which case we could still
+        // add something like .replace(/-?∞/, '0'). For the sake of saving bytes, I'll not add this until
+        // it becomes a problem
+        sampleRate.toLocaleString('fullwide', { useGrouping: false, maximumFractionDigits: 16 }),
+      );
+    publicKey && setBaggageValue(baggage, 'publickey', publicKey);
+    traceId && setBaggageValue(baggage, 'traceid', traceId);
+
+    setBaggageImmutable(baggage);
+
+    return baggage;
+  }
 }

packages/tracing/test/span.test.ts

@@ -414,9 +414,7 @@ describe('Span', () => {
 
       const hubSpy = jest.spyOn(hub.getClient()!, 'getOptions');
 
-      const span = transaction.startChild();
-
-      const baggage = span.getBaggage();
+      const baggage = transaction.getBaggage();
 
       expect(hubSpy).toHaveBeenCalledTimes(0);
       expect(baggage && isSentryBaggageEmpty(baggage)).toBe(false);
@@ -438,9 +436,7 @@ describe('Span', () => {
 
       const hubSpy = jest.spyOn(hub.getClient()!, 'getOptions');
 
-      const span = transaction.startChild();
-
-      const baggage = span.getBaggage();
+      const baggage = transaction.getBaggage();
 
       expect(hubSpy).toHaveBeenCalledTimes(1);
       expect(baggage && isSentryBaggageEmpty(baggage)).toBe(false);

packages/types/src/span.ts

@@ -1,4 +1,3 @@
-import { Baggage } from './baggage';
 import { Primitive } from './misc';
 import { Transaction } from './transaction';
 
@@ -162,6 +161,7 @@ export interface Span extends SpanContext {
     tags?: { [key: string]: Primitive };
     trace_id: string;
   };
+
   /** Convert the object to JSON */
   toJSON(): {
     data?: { [key: string]: any };
@@ -175,7 +175,4 @@ export interface Span extends SpanContext {
     timestamp?: number;
     trace_id: string;
   };
-
-  /** return the baggage for dynamic sampling and trace propagation */
-  getBaggage(): Baggage | undefined;
 }

packages/types/src/transaction.ts

@@ -87,6 +87,9 @@ export interface Transaction extends TransactionContext, Span {
 
   /** Updates the current transaction with a new `TransactionContext` */
   updateWithContext(transactionContext: TransactionContext): this;
+
+  /** return the baggage for dynamic sampling and trace propagation */
+  getBaggage(): Baggage;
 }
 
 /**

packages/utils/src/baggage.ts

@@ -1,5 +1,4 @@
-import { Baggage, BaggageObj, TraceparentData } from '@sentry/types';
-import { HttpHeaderValue } from '@sentry/types';
+import { Baggage, BaggageObj, HttpHeaderValue, TraceparentData } from '@sentry/types';
 
 import { isString } from './is';
 import { logger } from './logger';