feat(core): Deprecate span tags, data, context & setters

Also deprecate direct access to `span.attributes` (instead use `spanGetAttributes(span)`).

There are a few usages that we still need to figure out how we will replace them...!

Author: mydea

MIGRATION.md

@@ -17,6 +17,11 @@ 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.
+* `span.tags`: Set tags on the surrounding scope instead, or use attributes.
+* `span.data`: Use `spanGetAttributes(span)` instead.
+* `span.setTag()`: Use `span.setAttribute()` instead or set tags on the surrounding scope.
+* `span.setData()`: Use `span.setAttribute()` instead.
+* `transaction.setContext()`: Set context on the surrounding scope instead.
 
 ## Deprecate `pushScope` & `popScope` in favor of `withScope`
 

dev-packages/browser-integration-tests/suites/public-api/startTransaction/basic_usage/test.ts

@@ -31,6 +31,7 @@ sentryTest('should report finished spans as children of the root transaction', a
   const span_1 = transaction.spans?.[0];
   expect(span_1?.op).toBe('span_1');
   expect(span_1?.parentSpanId).toEqual(rootSpanId);
+  // eslint-disable-next-line deprecation/deprecation
   expect(span_1?.data).toMatchObject({ foo: 'bar', baz: [1, 2, 3] });
 
   const span_3 = transaction.spans?.[1];

dev-packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/withoutCallback/scenario.ts

@@ -30,10 +30,10 @@ const query = connection.query('SELECT 1 + 1 AS solution');
 const query2 = connection.query('SELECT NOW()', ['1', '2']);
 
 query.on('end', () => {
-  transaction.setTag('result_done', 'yes');
+  transaction.setAttribute('result_done', 'yes');
 
   query2.on('end', () => {
-    transaction.setTag('result_done2', 'yes');
+    transaction.setAttribute('result_done2', 'yes');
 
     // Wait a bit to ensure the queries completed
     setTimeout(() => {

dev-packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/withoutCallback/test.ts

@@ -8,7 +8,7 @@ test('should auto-instrument `mysql` package when using query without callback',
 
   assertSentryTransaction(envelope[2], {
     transaction: 'Test Transaction',
-    tags: {
+    data: {
       result_done: 'yes',
       result_done2: 'yes',
     },

packages/browser/src/profiling/hubextensions.ts

@@ -159,6 +159,8 @@ export function startProfileForTransaction(transaction: Transaction): Transactio
     // Always call onProfileHandler to ensure stopProfiling is called and the timeout is cleared.
     void onProfileHandler().then(
       () => {
+        // TODO: Can we rewrite this to use attributes?
+        // eslint-disable-next-line deprecation/deprecation
         transaction.setContext('profile', { profile_id: profileId, start_timestamp: startTimestamp });
         originalEnd();
       },

packages/bun/src/integrations/bunserver.ts

@@ -3,6 +3,7 @@ import {
   captureException,
   continueTrace,
   convertIntegrationFnToClass,
+  getCurrentScope,
   runWithAsyncContext,
   startSpan,
 } from '@sentry/core';
@@ -88,9 +89,10 @@ function instrumentBunServeOptions(serveOptions: Parameters<typeof Bun.serve>[0]
                   >);
                   if (response && response.status) {
                     span?.setHttpStatus(response.status);
-                    span?.setData('http.response.status_code', response.status);
+                    span?.setAttribute('http.response.status_code', response.status);
                     if (span instanceof Transaction) {
-                      span.setContext('response', {
+                      const scope = getCurrentScope();
+                      scope.setContext('response', {
                         headers: response.headers.toJSON(),
                         status_code: response.status,
                       });

packages/bun/test/integrations/bunserver.test.ts

@@ -26,6 +26,7 @@ describe('Bun Serve Integration', () => {
   test('generates a transaction around a request', async () => {
     client.on('finishTransaction', transaction => {
       expect(transaction.status).toBe('ok');
+      // eslint-disable-next-line deprecation/deprecation
       expect(transaction.tags).toEqual({
         'http.status_code': '200',
       });
@@ -48,6 +49,7 @@ describe('Bun Serve Integration', () => {
   test('generates a post transaction', async () => {
     client.on('finishTransaction', transaction => {
       expect(transaction.status).toBe('ok');
+      // eslint-disable-next-line deprecation/deprecation
       expect(transaction.tags).toEqual({
         'http.status_code': '200',
       });

packages/core/src/index.ts

@@ -71,7 +71,7 @@ export { createCheckInEnvelope } from './checkin';
 export { hasTracingEnabled } from './utils/hasTracingEnabled';
 export { isSentryRequestUrl } from './utils/isSentryRequestUrl';
 export { handleCallbackErrors } from './utils/handleCallbackErrors';
-export { spanToTraceHeader } from './utils/spanUtils';
+export { spanToTraceHeader, spanGetAttributes } from './utils/spanUtils';
 export { DEFAULT_ENVIRONMENT } from './constants';
 export { ModuleMetadata } from './integrations/metadata';
 export { RequestData } from './integrations/requestdata';

packages/core/src/tracing/idletransaction.ts

@@ -145,6 +145,7 @@ export class IdleTransaction extends Transaction {
     this.activities = {};
 
     if (this.op === 'ui.action.click') {
+      // eslint-disable-next-line deprecation/deprecation
       this.setTag(FINISH_REASON_TAG, this._finishReason);
     }
 

packages/core/src/tracing/span.ts

@@ -13,7 +13,7 @@ import type {
 import { dropUndefinedKeys, logger, timestampInSeconds, uuid4 } from '@sentry/utils';
 
 import { DEBUG_BUILD } from '../debug-build';
-import { spanToTraceContext, spanToTraceHeader } from '../utils/spanUtils';
+import { spanGetAttributes, spanToTraceContext, spanToTraceHeader } from '../utils/spanUtils';
 import { ensureTimestampInSeconds } from './utils';
 
 /**
@@ -97,21 +97,18 @@ export class Span implements SpanInterface {
   public description?: string;
 
   /**
-   * @inheritDoc
+   * Tags for the span.
+   * @deprecated Use `getSpanAttributes(span)` instead.
    */
   public tags: { [key: string]: Primitive };
 
   /**
-   * @inheritDoc
+   * Data for the span.
+   * @deprecated Use `getSpanAttributes(span)` instead.
    */
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   public data: { [key: string]: any };
 
-  /**
-   * @inheritDoc
-   */
-  public attributes: SpanAttributes;
-
   /**
    * List of spans that were finalized
    */
@@ -132,6 +129,8 @@ export class Span implements SpanInterface {
    */
   public origin?: SpanOrigin;
 
+  protected _attributes: SpanAttributes;
+
   /**
    * You should never call the constructor manually, always use `Sentry.startTransaction()`
    * or call `startChild()` on an existing span.
@@ -143,9 +142,11 @@ export class Span implements SpanInterface {
     this.traceId = spanContext.traceId || uuid4();
     this.spanId = spanContext.spanId || uuid4().substring(16);
     this.startTimestamp = spanContext.startTimestamp || timestampInSeconds();
+    // eslint-disable-next-line deprecation/deprecation
     this.tags = spanContext.tags || {};
+    // eslint-disable-next-line deprecation/deprecation
     this.data = spanContext.data || {};
-    this.attributes = spanContext.attributes || {};
+    this._attributes = spanContext.attributes || {};
     this.instrumenter = spanContext.instrumenter || 'sentry';
     this.origin = spanContext.origin || 'manual';
 
@@ -173,6 +174,25 @@ export class Span implements SpanInterface {
     }
   }
 
+  // This rule conflicts with another eslint rule :(
+  /* eslint-disable @typescript-eslint/member-ordering */
+
+  /**
+   * Attributes for the span.
+   * @deprecated Use `getSpanAttributes(span)` instead.
+   */
+  public get attributes(): SpanAttributes {
+    return this._attributes;
+  }
+
+  /**
+   * Attributes for the span.
+   * @deprecated Use `setAttributes()` instead.
+   */
+  public set attributes(attributes: SpanAttributes) {
+    this._attributes = attributes;
+  }
+
   /** An alias for `description` of the Span. */
   public get name(): string {
     return this.description || '';
@@ -184,6 +204,8 @@ export class Span implements SpanInterface {
     this.updateName(name);
   }
 
+  /* eslint-enable @typescript-eslint/member-ordering */
+
   /**
    * @inheritDoc
    */
@@ -218,18 +240,29 @@ export class Span implements SpanInterface {
   }
 
   /**
-   * @inheritDoc
+   * Sets the tag attribute on the current span.
+   *
+   * Can also be used to unset a tag, by passing `undefined`.
+   *
+   * @param key Tag key
+   * @param value Tag value
+   * @deprecated Use `setAttribute()` instead.
    */
   public setTag(key: string, value: Primitive): this {
+    // eslint-disable-next-line deprecation/deprecation
     this.tags = { ...this.tags, [key]: value };
     return this;
   }
 
   /**
-   * @inheritDoc
+   * Sets the data attribute on the current span
+   * @param key Data key
+   * @param value Data value
+   * @deprecated Use `setAttribute()` instead.
    */
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   public setData(key: string, value: any): this {
+    // eslint-disable-next-line deprecation/deprecation
     this.data = { ...this.data, [key]: value };
     return this;
   }
@@ -238,9 +271,9 @@ export class Span implements SpanInterface {
   public setAttribute(key: string, value: SpanAttributeValue | undefined): void {
     if (value === undefined) {
       // eslint-disable-next-line @typescript-eslint/no-dynamic-delete
-      delete this.attributes[key];
+      delete this._attributes[key];
     } else {
-      this.attributes[key] = value;
+      this._attributes[key] = value;
     }
   }
 
@@ -261,7 +294,9 @@ export class Span implements SpanInterface {
    * @inheritDoc
    */
   public setHttpStatus(httpStatus: number): this {
+    // eslint-disable-next-line deprecation/deprecation
     this.setTag('http.status_code', String(httpStatus));
+    // eslint-disable-next-line deprecation/deprecation
     this.setData('http.response.status_code', httpStatus);
     const spanStatus = spanStatusfromHttpCode(httpStatus);
     if (spanStatus !== 'unknown_error') {
@@ -329,7 +364,7 @@ export class Span implements SpanInterface {
    */
   public toContext(): SpanContext {
     return dropUndefinedKeys({
-      data: this._getData(),
+      data: spanGetAttributes(this),
       description: this.description,
       endTimestamp: this.endTimestamp,
       op: this.op,
@@ -338,6 +373,7 @@ export class Span implements SpanInterface {
       spanId: this.spanId,
       startTimestamp: this.startTimestamp,
       status: this.status,
+      // eslint-disable-next-line deprecation/deprecation
       tags: this.tags,
       traceId: this.traceId,
     });
@@ -347,6 +383,7 @@ export class Span implements SpanInterface {
    * @inheritDoc
    */
   public updateWithContext(spanContext: SpanContext): this {
+    // eslint-disable-next-line deprecation/deprecation
     this.data = spanContext.data || {};
     this.description = spanContext.description;
     this.endTimestamp = spanContext.endTimestamp;
@@ -356,6 +393,7 @@ export class Span implements SpanInterface {
     this.spanId = spanContext.spanId || this.spanId;
     this.startTimestamp = spanContext.startTimestamp || this.startTimestamp;
     this.status = spanContext.status;
+    // eslint-disable-next-line deprecation/deprecation
     this.tags = spanContext.tags || {};
     this.traceId = spanContext.traceId || this.traceId;
 
@@ -387,49 +425,20 @@ export class Span implements SpanInterface {
     origin?: SpanOrigin;
   } {
     return dropUndefinedKeys({
-      data: this._getData(),
+      data: spanGetAttributes(this),
       description: this.description,
       op: this.op,
       parent_span_id: this.parentSpanId,
       span_id: this.spanId,
       start_timestamp: this.startTimestamp,
       status: this.status,
+      // eslint-disable-next-line deprecation/deprecation
       tags: Object.keys(this.tags).length > 0 ? this.tags : undefined,
       timestamp: this.endTimestamp,
       trace_id: this.traceId,
       origin: this.origin,
     });
   }
-
-  /**
-   * Get the merged data for this span.
-   * For now, this combines `data` and `attributes` together,
-   * until eventually we can ingest `attributes` directly.
-   */
-  private _getData():
-    | {
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        [key: string]: any;
-      }
-    | undefined {
-    const { data, attributes } = this;
-
-    const hasData = Object.keys(data).length > 0;
-    const hasAttributes = Object.keys(attributes).length > 0;
-
-    if (!hasData && !hasAttributes) {
-      return undefined;
-    }
-
-    if (hasData && hasAttributes) {
-      return {
-        ...data,
-        ...attributes,
-      };
-    }
-
-    return hasData ? data : attributes;
-  }
 }
 
 export type SpanStatusType =

packages/core/src/tracing/transaction.ts

@@ -119,7 +119,8 @@ export class Transaction extends SpanClass implements TransactionInterface {
   }
 
   /**
-   * @inheritDoc
+   * Set the context of a transaction event.
+   * @deprecated Use either `.setAttribute()`, or set the context on the scope before creating the transaction.
    */
   public setContext(key: string, context: Context | null): void {
     if (context === null) {
@@ -288,6 +289,7 @@ export class Transaction extends SpanClass implements TransactionInterface {
       },
       spans: finishedSpans,
       start_timestamp: this.startTimestamp,
+      // eslint-disable-next-line deprecation/deprecation
       tags: this.tags,
       timestamp: this.endTimestamp,
       transaction: this.name,

packages/core/src/utils/prepareEvent.ts

@@ -15,6 +15,7 @@ import { DEFAULT_ENVIRONMENT } from '../constants';
 import { getGlobalEventProcessors, notifyEventProcessors } from '../eventProcessors';
 import { Scope, getGlobalScope } from '../scope';
 import { applyScopeDataToEvent, mergeScopeData } from './applyScopeDataToEvent';
+import { spanGetAttributes } from './spanUtils';
 
 /**
  * This type makes sure that we get either a CaptureContext, OR an EventHint.
@@ -326,10 +327,14 @@ function normalizeEvent(event: Event | null, depth: number, maxBreadth: number):
   // event.spans[].data may contain circular/dangerous data so we need to normalize it
   if (event.spans) {
     normalized.spans = event.spans.map(span => {
-      // We cannot use the spread operator here because `toJSON` on `span` is non-enumerable
-      if (span.data) {
-        span.data = normalize(span.data, depth, maxBreadth);
+      const data = spanGetAttributes(span);
+
+      if (data) {
+        // This is a bit weird, as we generally have `Span` instances here, but to be safe we do not assume so
+        // eslint-disable-next-line deprecation/deprecation
+        span.data = data;
       }
+
       return span;
     });
   }