feat(v8): Remove deprecated span id fields (getsentry#11180)

ref getsentry#10100 Removes `span.spanId`, `span.traceId`, and `span.parentSpanId`.
cadesalaberry · Apr 19, 2024 · af3c928 · af3c928
1 parent a444591
commit af3c928
Show file tree

Hide file tree

Showing 8 changed files with 45 additions and 98 deletions.
diff --git a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/server.ts b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/server.ts
@@ -28,12 +28,11 @@ app.use(cors());
 app.get('/test/express', (_req, res) => {
   // eslint-disable-next-line deprecation/deprecation
   const transaction = Sentry.getCurrentScope().getTransaction();
-  if (transaction) {
-    // eslint-disable-next-line deprecation/deprecation
-    transaction.traceId = '86f39e84263a4de99c326acab3bfe3bd';
-  }
+  const traceId = transaction?.spanContext().traceId;
   const headers = http.get('http://somewhere.not.sentry/').getHeaders();
-
+  if (traceId) {
+    headers['baggage'] = (headers['baggage'] as string).replace(traceId, '__SENTRY_TRACE_ID__');
+  }
   // Responding with the headers outgoing request headers back to the assertions.
   res.send({ test_data: headers });
 });

diff --git a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts
@@ -16,7 +16,7 @@ test('should attach a baggage header to an outgoing request.', async () => {
       host: 'somewhere.not.sentry',
       baggage:
         'sentry-environment=prod,sentry-release=1.0,sentry-public_key=public' +
-        ',sentry-trace_id=86f39e84263a4de99c326acab3bfe3bd,sentry-sample_rate=1,sentry-transaction=GET%20%2Ftest%2Fexpress' +
+        ',sentry-trace_id=__SENTRY_TRACE_ID__,sentry-sample_rate=1,sentry-transaction=GET%20%2Ftest%2Fexpress' +
         ',sentry-sampled=true',
     },
   });

diff --git a/...ges/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/server.ts b/...ges/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/server.ts
@@ -32,13 +32,9 @@ app.use(cors());
 app.get('/test/express', (_req, res) => {
   // eslint-disable-next-line deprecation/deprecation
   const transaction = Sentry.getCurrentScope().getTransaction();
-  if (transaction) {
-    // eslint-disable-next-line deprecation/deprecation
-    transaction.traceId = '86f39e84263a4de99c326acab3bfe3bd';
-    transaction.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route');
-  }
-  const headers = http.get('http://somewhere.not.sentry/').getHeaders();
+  transaction?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route');
 
+  const headers = http.get('http://somewhere.not.sentry/').getHeaders();
   // Responding with the headers outgoing request headers back to the assertions.
   res.send({ test_data: headers });
 });

diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts
@@ -97,56 +97,6 @@ export class SentrySpan implements Span {
   // This rule conflicts with another eslint rule :(
   /* eslint-disable @typescript-eslint/member-ordering */
 
-  /**
-   * The ID of the trace.
-   * @deprecated Use `spanContext().traceId` instead.
-   */
-  public get traceId(): string {
-    return this._traceId;
-  }
-
-  /**
-   * The ID of the trace.
-   * @deprecated You cannot update the traceId of a span after span creation.
-   */
-  public set traceId(traceId: string) {
-    this._traceId = traceId;
-  }
-
-  /**
-   * The ID of the span.
-   * @deprecated Use `spanContext().spanId` instead.
-   */
-  public get spanId(): string {
-    return this._spanId;
-  }
-
-  /**
-   * The ID of the span.
-   * @deprecated You cannot update the spanId of a span after span creation.
-   */
-  public set spanId(spanId: string) {
-    this._spanId = spanId;
-  }
-
-  /**
-   * @inheritDoc
-   *
-   * @deprecated Use `startSpan` functions instead.
-   */
-  public set parentSpanId(string) {
-    this._parentSpanId = string;
-  }
-
-  /**
-   * @inheritDoc
-   *
-   * @deprecated Use `spanToJSON(span).parent_span_id` instead.
-   */
-  public get parentSpanId(): string | undefined {
-    return this._parentSpanId;
-  }
-
   /**
    * Was this span chosen to be sent as part of the sample?
    * @deprecated Use `isRecording()` instead.

diff --git a/packages/core/test/lib/tracing/sentrySpan.test.ts b/packages/core/test/lib/tracing/sentrySpan.test.ts
@@ -1,7 +1,13 @@
 import { timestampInSeconds } from '@sentry/utils';
 import { SentrySpan } from '../../../src/tracing/sentrySpan';
 import { SPAN_STATUS_ERROR } from '../../../src/tracing/spanstatus';
-import { TRACE_FLAG_NONE, TRACE_FLAG_SAMPLED, spanToJSON, spanToTraceContext } from '../../../src/utils/spanUtils';
+import {
+  TRACE_FLAG_NONE,
+  TRACE_FLAG_SAMPLED,
+  spanIsSampled,
+  spanToJSON,
+  spanToTraceContext,
+} from '../../../src/utils/spanUtils';
 
 describe('SentrySpan', () => {
   describe('name', () => {
@@ -25,9 +31,9 @@ describe('SentrySpan', () => {
       const span = new SentrySpan({ sampled: true });
       // eslint-disable-next-line deprecation/deprecation
       const span2 = span.startChild();
-      expect((span2 as any).parentSpanId).toBe((span as any).spanId);
-      expect((span2 as any).traceId).toBe((span as any).traceId);
-      expect((span2 as any).sampled).toBe((span as any).sampled);
+      expect(spanToJSON(span2).parent_span_id).toBe(span.spanContext().spanId);
+      expect(span.spanContext().traceId).toBe(span.spanContext().traceId);
+      expect(spanIsSampled(span2)).toBe(spanIsSampled(span));
     });
   });
 
@@ -58,18 +64,23 @@ describe('SentrySpan', () => {
     });
 
     test('with parent', () => {
-      const spanA = new SentrySpan({ traceId: 'a', spanId: 'b' }) as any;
-      const spanB = new SentrySpan({ traceId: 'c', spanId: 'd', sampled: false, parentSpanId: spanA.spanId });
+      const spanA = new SentrySpan({ traceId: 'a', spanId: 'b' });
+      const spanB = new SentrySpan({
+        traceId: 'c',
+        spanId: 'd',
+        sampled: false,
+        parentSpanId: spanA.spanContext().spanId,
+      });
       const serialized = spanToJSON(spanB);
       expect(serialized).toHaveProperty('parent_span_id', 'b');
       expect(serialized).toHaveProperty('span_id', 'd');
       expect(serialized).toHaveProperty('trace_id', 'c');
     });
 
     test('should drop all `undefined` values', () => {
-      const spanA = new SentrySpan({ traceId: 'a', spanId: 'b' }) as any;
+      const spanA = new SentrySpan({ traceId: 'a', spanId: 'b' });
       const spanB = new SentrySpan({
-        parentSpanId: spanA.spanId,
+        parentSpanId: spanA.spanContext().spanId,
         spanId: 'd',
         traceId: 'c',
       });

diff --git a/packages/node/test/handlers.test.ts b/packages/node/test/handlers.test.ts
@@ -9,6 +9,7 @@ import {
   getMainCarrier,
   mergeScopeData,
   setCurrentClient,
+  spanIsSampled,
   spanToJSON,
   withScope,
 } from '@sentry/core';
@@ -289,7 +290,7 @@ describe('tracingHandler', () => {
 
     sentryTracingMiddleware(req, res, next);
 
-    const transaction = (res as any).__sentry_transaction;
+    const transaction = (res as any).__sentry_transaction as Transaction;
 
     expect(getPropagationContext()).toEqual({
       traceId: '12312012123120121231201212312012',
@@ -300,9 +301,10 @@ describe('tracingHandler', () => {
     });
 
     // since we have no tracesSampler defined, the default behavior (inherit if possible) applies
-    expect(transaction.traceId).toEqual('12312012123120121231201212312012');
-    expect(transaction.parentSpanId).toEqual('1121201211212012');
-    expect(transaction.sampled).toEqual(false);
+    expect(transaction.spanContext().traceId).toEqual('12312012123120121231201212312012');
+    expect(spanToJSON(transaction).parent_span_id).toEqual('1121201211212012');
+    expect(spanIsSampled(transaction)).toEqual(false);
+    // eslint-disable-next-line deprecation/deprecation
     expect(transaction.metadata?.dynamicSamplingContext).toStrictEqual({});
   });
 
@@ -322,12 +324,13 @@ describe('tracingHandler', () => {
       dsc: { version: '1.0', environment: 'production' },
     });
 
-    const transaction = (res as any).__sentry_transaction;
+    const transaction = (res as any).__sentry_transaction as Transaction;
 
     // since we have no tracesSampler defined, the default behavior (inherit if possible) applies
-    expect(transaction.traceId).toEqual('12312012123120121231201212312012');
-    expect(transaction.parentSpanId).toEqual('1121201211212012');
-    expect(transaction.sampled).toEqual(true);
+    expect(transaction.spanContext().traceId).toEqual('12312012123120121231201212312012');
+    expect(spanToJSON(transaction).parent_span_id).toEqual('1121201211212012');
+    expect(spanIsSampled(transaction)).toEqual(true);
+    // eslint-disable-next-line deprecation/deprecation
     expect(transaction.metadata?.dynamicSamplingContext).toStrictEqual({ version: '1.0', environment: 'production' });
   });
 
@@ -341,7 +344,8 @@ describe('tracingHandler', () => {
 
     expect(getPropagationContext().dsc).toEqual({ version: '1.0', environment: 'production' });
 
-    const transaction = (res as any).__sentry_transaction;
+    const transaction = (res as any).__sentry_transaction as Transaction;
+    // eslint-disable-next-line deprecation/deprecation
     expect(transaction.metadata?.dynamicSamplingContext).toStrictEqual({ version: '1.0', environment: 'production' });
   });
 
@@ -364,7 +368,7 @@ describe('tracingHandler', () => {
   it('puts its transaction on the response object', () => {
     sentryTracingMiddleware(req, res, next);
 
-    const transaction = (res as any).__sentry_transaction;
+    const transaction = (res as any).__sentry_transaction as Transaction;
 
     expect(transaction).toBeDefined();
 
@@ -396,7 +400,7 @@ describe('tracingHandler', () => {
 
     sentryTracingMiddleware(req, res, next);
 
-    const transaction = (res as any).__sentry_transaction;
+    const transaction = (res as any).__sentry_transaction as Transaction;
 
     expect(spanToJSON(transaction).description).toBe(`${method.toUpperCase()} ${path}`);
   });
@@ -406,7 +410,7 @@ describe('tracingHandler', () => {
 
     sentryTracingMiddleware(req, res, next);
 
-    const transaction = (res as any).__sentry_transaction;
+    const transaction = (res as any).__sentry_transaction as Transaction;
 
     expect(spanToJSON(transaction).description).toBe(`${method.toUpperCase()} ${path}`);
   });
@@ -416,7 +420,7 @@ describe('tracingHandler', () => {
 
     sentryTracingMiddleware(req, res, next);
 
-    const transaction = (res as any).__sentry_transaction;
+    const transaction = (res as any).__sentry_transaction as Transaction;
 
     expect(spanToJSON(transaction).description).toBe(`${method.toUpperCase()} ${path}`);
   });

diff --git a/packages/types/src/envelope.ts b/packages/types/src/envelope.ts
@@ -8,13 +8,12 @@ import type { Profile } from './profiling';
 import type { ReplayEvent, ReplayRecordingData } from './replay';
 import type { SdkInfo } from './sdkinfo';
 import type { SerializedSession, Session, SessionAggregates } from './session';
-import type { Transaction } from './transaction';
 
 // Based on: https://develop.sentry.dev/sdk/envelopes/
 
 // Based on https://github.com/getsentry/relay/blob/b23b8d3b2360a54aaa4d19ecae0231201f31df5e/relay-sampling/src/lib.rs#L685-L707
 export type DynamicSamplingContext = {
-  trace_id: Transaction['traceId'];
+  trace_id: string;
   public_key: DsnComponents['publicKey'];
   sample_rate?: string;
   release?: string;

diff --git a/packages/types/src/transaction.ts b/packages/types/src/transaction.ts
@@ -57,19 +57,7 @@ export interface TraceparentData {
 /**
  * Transaction "Class", inherits Span only has `setName`
  */
-export interface Transaction extends Omit<TransactionContext, 'name' | 'op'>, Span {
-  /**
-   * The ID of the transaction.
-   * @deprecated Use `spanContext().spanId` instead.
-   */
-  spanId: string;
-
-  /**
-   * The ID of the trace.
-   * @deprecated Use `spanContext().traceId` instead.
-   */
-  traceId: string;
-
+export interface Transaction extends Omit<TransactionContext, 'name' | 'op' | 'spanId' | 'traceId'>, Span {
   /**
    * Was this transaction chosen to be sent as part of the sample?
    * @deprecated Use `spanIsSampled(transaction)` instead.