feat(core): Add spanGetMetadata() & spanSetMetadata() APIs

To replace `transaction.setMetadata()`.

Author: mydea

MIGRATION.md

@@ -17,6 +17,8 @@ In v8, the Span class is heavily reworked. The following properties & methods ar
 * `span.setName(newName)`: Use `span.updateName(newName)` instead.
 * `span.toTraceparent()`: use `spanToTraceHeader(span)` util instead.
 * `span.getTraceContext()`: Use `spanToTraceContext(span)` utility function instead.
+* `transaction.setMetadata(metadata)`: Use `spanSetMetadata(span, metadata)` utility function instead.
+* `transaction.metadata`: Use `spanGetMetadata(span)` utility function instead.
 
 ## Deprecate `pushScope` & `popScope` in favor of `withScope`
 

dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/server.ts

@@ -1,4 +1,5 @@
 import http from 'http';
+import { spanSetMetadata } from '@sentry/core';
 import * as Sentry from '@sentry/node';
 import * as Tracing from '@sentry/tracing';
 import cors from 'cors';
@@ -32,7 +33,7 @@ app.get('/test/express', (_req, res) => {
   const transaction = Sentry.getCurrentHub().getScope().getTransaction();
   if (transaction) {
     transaction.traceId = '86f39e84263a4de99c326acab3bfe3bd';
-    transaction.setMetadata({ source: 'route' });
+    spanSetMetadata(transaction, { source: 'route' });
   }
   const headers = http.get('http://somewhere.not.sentry/').getHeaders();
 

packages/angular-ivy/ng-package.json

@@ -5,9 +5,10 @@
     "entryFile": "src/index.ts",
     "umdModuleIds": {
       "@sentry/browser": "Sentry",
-      "@sentry/utils": "Sentry.util"
+      "@sentry/utils": "Sentry.util",
+      "@sentry/core": "Sentry.core"
     }
   },
-  "allowedNonPeerDependencies": ["@sentry/browser", "@sentry/utils", "@sentry/types", "tslib"],
+  "allowedNonPeerDependencies": ["@sentry/browser", "@sentry/core", "@sentry/utils", "@sentry/types", "tslib"],
   "assets": ["README.md", "LICENSE"]
 }

packages/angular-ivy/package.json

@@ -22,6 +22,7 @@
   },
   "dependencies": {
     "@sentry/browser": "7.91.0",
+    "@sentry/core": "7.91.0",
     "@sentry/types": "7.91.0",
     "@sentry/utils": "7.91.0",
     "tslib": "^2.4.1"

packages/angular/ng-package.json

@@ -5,9 +5,10 @@
     "entryFile": "src/index.ts",
     "umdModuleIds": {
       "@sentry/browser": "Sentry",
-      "@sentry/utils": "Sentry.util"
+      "@sentry/utils": "Sentry.util",
+      "@sentry/core": "Sentry.core"
     }
   },
-  "whitelistedNonPeerDependencies": ["@sentry/browser", "@sentry/utils", "@sentry/types", "tslib"],
+  "whitelistedNonPeerDependencies": ["@sentry/browser", "@sentry/core", "@sentry/utils", "@sentry/types", "tslib"],
   "assets": ["README.md", "LICENSE"]
 }

packages/angular/package.json

@@ -22,6 +22,7 @@
   },
   "dependencies": {
     "@sentry/browser": "7.91.0",
+    "@sentry/core": "7.91.0",
     "@sentry/types": "7.91.0",
     "@sentry/utils": "7.91.0",
     "tslib": "^2.4.1"

packages/angular/src/tracing.ts

@@ -8,6 +8,7 @@ import type { ActivatedRouteSnapshot, Event, RouterState } from '@angular/router
 import { NavigationCancel, NavigationError, Router } from '@angular/router';
 import { NavigationEnd, NavigationStart, ResolveEnd } from '@angular/router';
 import { WINDOW, getCurrentScope } from '@sentry/browser';
+import { spanSetMetadata } from '@sentry/core';
 import type { Span, Transaction, TransactionContext } from '@sentry/types';
 import { logger, stripUrlQueryAndFragment, timestampInSeconds } from '@sentry/utils';
 import type { Observable } from 'rxjs';
@@ -119,7 +120,7 @@ export class TraceService implements OnDestroy {
       // TODO (v8 / #5416): revisit the source condition. Do we want to make the parameterized route the default?
       if (transaction && transaction.metadata.source === 'url') {
         transaction.updateName(route);
-        transaction.setMetadata({ source: 'route' });
+        spanSetMetadata(transaction, { source: 'route' });
       }
     }),
   );

packages/angular/test/tracing.test.ts

@@ -1,6 +1,7 @@
 import { Component } from '@angular/core';
 import type { ActivatedRouteSnapshot, CanActivate, RouterStateSnapshot } from '@angular/router';
 
+import * as SentryCore from '@sentry/core';
 import { TraceClassDecorator, TraceDirective, TraceMethodDecorator, instrumentAngularRouting } from '../src';
 import { getParameterizedRouteFromSnapshot } from '../src/tracing';
 import { AppComponent, TestEnv } from './utils/index';
@@ -11,7 +12,6 @@ const defaultStartTransaction = (ctx: any) => {
   transaction = {
     ...ctx,
     updateName: jest.fn(name => (transaction.name = name)),
-    setMetadata: jest.fn(),
   };
 
   return transaction;
@@ -31,9 +31,12 @@ jest.mock('@sentry/browser', () => {
   };
 });
 
+const mockSpanSetMetadata = jest.spyOn(SentryCore, 'spanSetMetadata');
+
 describe('Angular Tracing', () => {
   beforeEach(() => {
     transaction = undefined;
+    mockSpanSetMetadata.mockClear();
   });
 
   describe('instrumentAngularRouting', () => {
@@ -329,7 +332,7 @@ describe('Angular Tracing', () => {
           metadata: { source: 'url' },
         });
         expect(transaction.updateName).toHaveBeenCalledWith(result);
-        expect(transaction.setMetadata).toHaveBeenCalledWith({ source: 'route' });
+        expect(mockSpanSetMetadata).toHaveBeenCalledWith(expect.anything(), { source: 'route' });
 
         env.destroy();
       });

packages/core/src/index.ts

@@ -69,7 +69,7 @@ export { prepareEvent } from './utils/prepareEvent';
 export { createCheckInEnvelope } from './checkin';
 export { hasTracingEnabled } from './utils/hasTracingEnabled';
 export { isSentryRequestUrl } from './utils/isSentryRequestUrl';
-export { spanToTraceHeader } from './utils/spanUtils';
+export { spanToTraceHeader, spanSetMetadata, spanGetMetadata } from './utils/spanUtils';
 export { DEFAULT_ENVIRONMENT } from './constants';
 export { ModuleMetadata } from './integrations/metadata';
 export { RequestData } from './integrations/requestdata';

packages/core/src/tracing/sampling.ts

@@ -3,6 +3,7 @@ import { isNaN, logger } from '@sentry/utils';
 
 import { DEBUG_BUILD } from '../debug-build';
 import { hasTracingEnabled } from '../utils/hasTracingEnabled';
+import { spanSetMetadata } from '../utils/spanUtils';
 import type { Transaction } from './transaction';
 
 /**
@@ -27,7 +28,7 @@ export function sampleTransaction<T extends Transaction>(
 
   // if the user has forced a sampling decision by passing a `sampled` value in their transaction context, go with that
   if (transaction.sampled !== undefined) {
-    transaction.setMetadata({
+    spanSetMetadata(transaction, {
       sampleRate: Number(transaction.sampled),
     });
     return transaction;
@@ -38,20 +39,20 @@ export function sampleTransaction<T extends Transaction>(
   let sampleRate;
   if (typeof options.tracesSampler === 'function') {
     sampleRate = options.tracesSampler(samplingContext);
-    transaction.setMetadata({
+    spanSetMetadata(transaction, {
       sampleRate: Number(sampleRate),
     });
   } else if (samplingContext.parentSampled !== undefined) {
     sampleRate = samplingContext.parentSampled;
   } else if (typeof options.tracesSampleRate !== 'undefined') {
     sampleRate = options.tracesSampleRate;
-    transaction.setMetadata({
-      sampleRate: Number(sampleRate),
+    spanSetMetadata(transaction, {
+      sampleRate,
     });
   } else {
     // When `enableTracing === true`, we use a sample rate of 100%
     sampleRate = 1;
-    transaction.setMetadata({
+    spanSetMetadata(transaction, {
       sampleRate,
     });
   }

packages/core/src/tracing/transaction.ts

@@ -14,15 +14,13 @@ import { dropUndefinedKeys, logger, timestampInSeconds } from '@sentry/utils';
 import { DEBUG_BUILD } from '../debug-build';
 import type { Hub } from '../hub';
 import { getCurrentHub } from '../hub';
-import { spanToTraceContext } from '../utils/spanUtils';
+import { spanGetMetadata, spanSetMetadata, spanToTraceContext } from '../utils/spanUtils';
 import { getDynamicSamplingContextFromClient } from './dynamicSamplingContext';
 import { Span as SpanClass, SpanRecorder } from './span';
 import { ensureTimestampInSeconds } from './utils';
 
 /** JSDoc */
 export class Transaction extends SpanClass implements TransactionInterface {
-  public metadata: TransactionMetadata;
-
   /**
    * The reference to the current hub.
    */
@@ -58,11 +56,11 @@ export class Transaction extends SpanClass implements TransactionInterface {
 
     this._name = transactionContext.name || '';
 
-    this.metadata = {
+    spanSetMetadata(this, {
       source: 'custom',
       ...transactionContext.metadata,
       spanMetadata: {},
-    };
+    });
 
     this._trimEnd = transactionContext.trimEnd;
 
@@ -71,13 +69,18 @@ export class Transaction extends SpanClass implements TransactionInterface {
 
     // If Dynamic Sampling Context is provided during the creation of the transaction, we freeze it as it usually means
     // there is incoming Dynamic Sampling Context. (Either through an incoming request, a baggage meta-tag, or other means)
-    const incomingDynamicSamplingContext = this.metadata.dynamicSamplingContext;
+    const incomingDynamicSamplingContext = (
+      spanGetMetadata(this) as { dynamicSamplingContext: DynamicSamplingContext | undefined }
+    ).dynamicSamplingContext;
     if (incomingDynamicSamplingContext) {
       // We shallow copy this in case anything writes to the original reference of the passed in `dynamicSamplingContext`
       this._frozenDynamicSamplingContext = { ...incomingDynamicSamplingContext };
     }
   }
 
+  // This sadly conflicts with the getter/setter ordering :(
+  /* eslint-disable @typescript-eslint/member-ordering */
+
   /** Getter for `name` property */
   public get name(): string {
     return this._name;
@@ -91,14 +94,32 @@ export class Transaction extends SpanClass implements TransactionInterface {
     this.setName(newName);
   }
 
+  /**
+   * Get the metadata for this transaction.
+   * @deprecated Use `spanGetMetadata(transaction)` instead.
+   */
+  public get metadata(): TransactionMetadata {
+    return spanGetMetadata(this) as TransactionMetadata;
+  }
+
+  /**
+   * Update the metadata for this transaction.
+   * @deprecated Use `spanGetMetadata(transaction)` instead.
+   */
+  public set metadata(metadata: TransactionMetadata) {
+    spanSetMetadata(this, metadata);
+  }
+
+  /* eslint-enable @typescript-eslint/member-ordering */
+
   /**
    * Setter for `name` property, which also sets `source` on the metadata.
    *
    * @deprecated Use `updateName()` and `setMetadata()` instead.
    */
   public setName(name: string, source: TransactionMetadata['source'] = 'custom'): void {
     this._name = name;
-    this.metadata.source = source;
+    spanSetMetadata(this, { source });
   }
 
   /** @inheritdoc */
@@ -138,10 +159,11 @@ export class Transaction extends SpanClass implements TransactionInterface {
   }
 
   /**
-   * @inheritDoc
+   * Set metadata for this transaction.
+   * @deprecated Use `spanSetMetadata(span, metadata)` instead.
    */
   public setMetadata(newMetadata: Partial<TransactionMetadata>): void {
-    this.metadata = { ...this.metadata, ...newMetadata };
+    spanSetMetadata(this, newMetadata);
   }
 
   /**
@@ -203,13 +225,15 @@ export class Transaction extends SpanClass implements TransactionInterface {
     const scope = hub.getScope();
     const dsc = getDynamicSamplingContextFromClient(this.traceId, client, scope);
 
-    const maybeSampleRate = this.metadata.sampleRate;
+    const metadata = spanGetMetadata(this) as TransactionMetadata;
+
+    const maybeSampleRate = metadata.sampleRate;
     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 source = this.metadata.source;
+    const source = metadata.source;
     if (source && source !== 'url') {
       dsc.transaction = this.name;
     }
@@ -278,7 +302,7 @@ export class Transaction extends SpanClass implements TransactionInterface {
       }).endTimestamp;
     }
 
-    const metadata = this.metadata;
+    const metadata = spanGetMetadata(this) as TransactionMetadata;
 
     const transaction: TransactionEvent = {
       contexts: {

packages/core/src/utils/spanUtils.ts

@@ -1,6 +1,8 @@
-import type { Span, TraceContext } from '@sentry/types';
+import type { Span, SpanMetadata, TraceContext } from '@sentry/types';
 import { dropUndefinedKeys, generateSentryTraceHeader } from '@sentry/utils';
 
+const SPAN_METADATA = new WeakMap<Span, SpanMetadata>();
+
 /**
  * Convert a span to a trace context, which can be sent as the `trace` context in an event.
  */
@@ -26,3 +28,19 @@ export function spanToTraceContext(span: Span): TraceContext {
 export function spanToTraceHeader(span: Span): string {
   return generateSentryTraceHeader(span.traceId, span.spanId, span.sampled);
 }
+
+/**
+ * Get the metadata for a span.
+ */
+export function spanGetMetadata(span: Span): SpanMetadata {
+  return SPAN_METADATA.get(span) || {};
+}
+
+/**
+ * Update metadata for a span.
+ * This will merge the given new metadata with existing metadata.
+ */
+export function spanSetMetadata(span: Span, newMetadata: SpanMetadata): void {
+  const existingMetadata = spanGetMetadata(span);
+  SPAN_METADATA.set(span, { ...existingMetadata, ...newMetadata });
+}

packages/core/test/lib/tracing/transaction.test.ts

@@ -1,4 +1,5 @@
 import { Transaction } from '../../../src';
+import { spanSetMetadata } from '../../../src/utils/spanUtils';
 
 describe('transaction', () => {
   it('works with name', () => {
@@ -8,37 +9,38 @@ describe('transaction', () => {
 
   it('allows to update the name via setter', () => {
     const transaction = new Transaction({ name: 'span name' });
-    transaction.setMetadata({ source: 'route' });
+    spanSetMetadata(transaction, { source: 'route' });
     expect(transaction.name).toEqual('span name');
 
     transaction.name = 'new name';
 
     expect(transaction.name).toEqual('new name');
+    // eslint-disable-next-line deprecation/deprecation
     expect(transaction.metadata.source).toEqual('custom');
   });
 
   it('allows to update the name via setName', () => {
     const transaction = new Transaction({ name: 'span name' });
-    transaction.setMetadata({ source: 'route' });
+    spanSetMetadata(transaction, { source: 'route' });
     expect(transaction.name).toEqual('span name');
 
-    transaction.setMetadata({ source: 'route' });
-
     // eslint-disable-next-line deprecation/deprecation
     transaction.setName('new name');
 
     expect(transaction.name).toEqual('new name');
+    // eslint-disable-next-line deprecation/deprecation
     expect(transaction.metadata.source).toEqual('custom');
   });
 
   it('allows to update the name via updateName', () => {
     const transaction = new Transaction({ name: 'span name' });
-    transaction.setMetadata({ source: 'route' });
+    spanSetMetadata(transaction, { source: 'route' });
     expect(transaction.name).toEqual('span name');
 
     transaction.updateName('new name');
 
     expect(transaction.name).toEqual('new name');
+    // eslint-disable-next-line deprecation/deprecation
     expect(transaction.metadata.source).toEqual('route');
   });
 });