feat(v8): Remove usage of span.description and span.name (#10697)

ref #10677 Removes all usage of setting/getting `span.description` and `span.name`, and removes the `span.setName` method.
getsentry · Feb 21, 2024 · c2baeac · c2baeac
1 parent 18f7107
commit c2baeac
Show file tree

Hide file tree

Showing 54 changed files with 180 additions and 430 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -1056,7 +1056,8 @@ jobs:
             'sveltekit-2',
             'generic-ts3.8',
             'node-experimental-fastify-app',
-            'node-hapi-app',
+            # TODO(v8): Re-enable hapi tests
+            # 'node-hapi-app',
             'node-exports-test-app',
             'vue-3'
           ]

diff --git a/...r-integration-tests/suites/tracing/browserTracingIntegration/backgroundtab-custom/test.ts b/...r-integration-tests/suites/tracing/browserTracingIntegration/backgroundtab-custom/test.ts
@@ -23,23 +23,23 @@ sentryTest('should finish a custom transaction when the page goes background', a
   const transactionHandle = await page.evaluateHandle('window.transaction');
 
   const id_before = await getPropertyValue(transactionHandle, 'span_id');
-  const name_before = await getPropertyValue(transactionHandle, 'name');
+  const nameBefore = JSON.parse(await transactionHandle.evaluate((t: any) => JSON.stringify(t))).description;
   const status_before = await getPropertyValue(transactionHandle, 'status');
   const tags_before = await getPropertyValue(transactionHandle, 'tags');
 
-  expect(name_before).toBe('test-transaction');
+  expect(nameBefore).toBe('test-transaction');
   expect(status_before).toBeUndefined();
   expect(tags_before).toStrictEqual({});
 
   await page.locator('#go-background').click();
 
   const id_after = await getPropertyValue(transactionHandle, 'span_id');
-  const name_after = await getPropertyValue(transactionHandle, 'name');
+  const nameAfter = JSON.parse(await transactionHandle.evaluate((t: any) => JSON.stringify(t))).description;
   const status_after = await getPropertyValue(transactionHandle, 'status');
   const tags_after = await getPropertyValue(transactionHandle, 'tags');
 
   expect(id_before).toBe(id_after);
-  expect(name_after).toBe(name_before);
+  expect(nameAfter).toBe('test-transaction');
   expect(status_after).toBe('cancelled');
   expect(tags_after).toStrictEqual({ visibilitychange: 'document.hidden' });
 });
diff --git a/...ation-tests/suites/tracing/browserTracingIntegration/pageloadWithHeartbeatTimeout/test.ts b/...ation-tests/suites/tracing/browserTracingIntegration/pageloadWithHeartbeatTimeout/test.ts
@@ -1,5 +1,5 @@
 import { expect } from '@playwright/test';
-import type { Event } from '@sentry/types';
+import type { SerializedEvent } from '@sentry/types';
 
 import { sentryTest } from '../../../../utils/fixtures';
 import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers';
@@ -16,11 +16,10 @@ sentryTest(
 
     const url = await getLocalTestPath({ testDir: __dirname });
 
-    const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
+    const eventData = await getFirstSentryEnvelopeRequest<SerializedEvent>(page, url);
 
     expect(eventData.contexts?.trace?.op).toBe('pageload');
     expect(
-      // eslint-disable-next-line deprecation/deprecation
       eventData.spans?.find(span => span.description === 'pageload-child-span' && span.status === 'cancelled'),
     ).toBeDefined();
   },

diff --git a/...ages/browser-integration-tests/suites/tracing/browsertracing/backgroundtab-custom/test.ts b/...ages/browser-integration-tests/suites/tracing/browsertracing/backgroundtab-custom/test.ts
@@ -23,23 +23,23 @@ sentryTest('should finish a custom transaction when the page goes background', a
   const transactionHandle = await page.evaluateHandle('window.transaction');
 
   const id_before = await getPropertyValue(transactionHandle, 'span_id');
-  const name_before = await getPropertyValue(transactionHandle, 'name');
+  const nameBefore = JSON.parse(await transactionHandle.evaluate((t: any) => JSON.stringify(t))).description;
   const status_before = await getPropertyValue(transactionHandle, 'status');
   const tags_before = await getPropertyValue(transactionHandle, 'tags');
 
-  expect(name_before).toBe('test-transaction');
+  expect(nameBefore).toBe('test-transaction');
   expect(status_before).toBeUndefined();
   expect(tags_before).toStrictEqual({});
 
   await page.locator('#go-background').click();
 
   const id_after = await getPropertyValue(transactionHandle, 'span_id');
-  const name_after = await getPropertyValue(transactionHandle, 'name');
+  const nameAfter = JSON.parse(await transactionHandle.evaluate((t: any) => JSON.stringify(t))).description;
   const status_after = await getPropertyValue(transactionHandle, 'status');
   const tags_after = await getPropertyValue(transactionHandle, 'tags');
 
   expect(id_before).toBe(id_after);
-  expect(name_after).toBe(name_before);
+  expect(nameAfter).toBe('test-transaction');
   expect(status_after).toBe('cancelled');
   expect(tags_after).toStrictEqual({ visibilitychange: 'document.hidden' });
 });
diff --git a/...wser-integration-tests/suites/tracing/browsertracing/pageloadWithHeartbeatTimeout/test.ts b/...wser-integration-tests/suites/tracing/browsertracing/pageloadWithHeartbeatTimeout/test.ts
@@ -1,5 +1,5 @@
 import { expect } from '@playwright/test';
-import type { Event } from '@sentry/types';
+import type { SerializedEvent } from '@sentry/types';
 
 import { sentryTest } from '../../../../utils/fixtures';
 import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers';
@@ -16,11 +16,10 @@ sentryTest(
 
     const url = await getLocalTestPath({ testDir: __dirname });
 
-    const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
+    const eventData = await getFirstSentryEnvelopeRequest<SerializedEvent>(page, url);
 
     expect(eventData.contexts?.trace?.op).toBe('pageload');
     expect(
-      // eslint-disable-next-line deprecation/deprecation
       eventData.spans?.find(span => span.description === 'pageload-child-span' && span.status === 'cancelled'),
     ).toBeDefined();
   },

diff --git a/packages/angular/src/tracing.ts b/packages/angular/src/tracing.ts
@@ -173,7 +173,7 @@ export class TraceService implements OnDestroy {
       if (activeTransaction) {
         // eslint-disable-next-line deprecation/deprecation
         this._routingSpan = activeTransaction.startChild({
-          description: `${navigationEvent.url}`,
+          name: `${navigationEvent.url}`,
           op: ANGULAR_ROUTING_OP,
           origin: 'auto.ui.angular',
           tags: {
@@ -327,7 +327,7 @@ export class TraceDirective implements OnInit, AfterViewInit {
     if (activeTransaction) {
       // eslint-disable-next-line deprecation/deprecation
       this._tracingSpan = activeTransaction.startChild({
-        description: `<${this.componentName}>`,
+        name: `<${this.componentName}>`,
         op: ANGULAR_INIT_OP,
         origin: 'auto.ui.angular.trace_directive',
       });
@@ -371,7 +371,7 @@ export function TraceClassDecorator(): ClassDecorator {
       if (activeTransaction) {
         // eslint-disable-next-line deprecation/deprecation
         tracingSpan = activeTransaction.startChild({
-          description: `<${target.name}>`,
+          name: `<${target.name}>`,
           op: ANGULAR_INIT_OP,
           origin: 'auto.ui.angular.trace_class_decorator',
         });
@@ -410,7 +410,7 @@ export function TraceMethodDecorator(): MethodDecorator {
       if (activeTransaction) {
         // eslint-disable-next-line deprecation/deprecation
         activeTransaction.startChild({
-          description: `<${target.constructor.name}>`,
+          name: `<${target.constructor.name}>`,
           endTimestamp: now,
           op: `${ANGULAR_OP}.${String(propertyKey)}`,
           origin: 'auto.ui.angular.trace_method_decorator',

diff --git a/packages/angular/test/tracing.test.ts b/packages/angular/test/tracing.test.ts
@@ -373,13 +373,13 @@ describe('Angular Tracing', () => {
       expect(transaction.startChild).toHaveBeenCalledWith({
         op: 'ui.angular.init',
         origin: 'auto.ui.angular.trace_directive',
-        description: '<unknown>',
+        name: '<unknown>',
       });
 
       env.destroy();
     });
 
-    it('should use component name as span description', async () => {
+    it('should use component name as span name', async () => {
       const directive = new TraceDirective();
       const finishMock = jest.fn();
       const customStartTransaction = jest.fn(defaultStartTransaction);
@@ -400,7 +400,7 @@ describe('Angular Tracing', () => {
       expect(transaction.startChild).toHaveBeenCalledWith({
         op: 'ui.angular.init',
         origin: 'auto.ui.angular.trace_directive',
-        description: '<test-component>',
+        name: '<test-component>',
       });
 
       env.destroy();
@@ -472,7 +472,7 @@ describe('Angular Tracing', () => {
       });
 
       expect(transaction.startChild).toHaveBeenCalledWith({
-        description: '<DecoratedComponent>',
+        name: '<DecoratedComponent>',
         op: 'ui.angular.init',
         origin: 'auto.ui.angular.trace_class_decorator',
       });
@@ -526,15 +526,15 @@ describe('Angular Tracing', () => {
 
       expect(transaction.startChild).toHaveBeenCalledTimes(2);
       expect(transaction.startChild.mock.calls[0][0]).toEqual({
-        description: '<DecoratedComponent>',
+        name: '<DecoratedComponent>',
         op: 'ui.angular.ngOnInit',
         origin: 'auto.ui.angular.trace_method_decorator',
         startTimestamp: expect.any(Number),
         endTimestamp: expect.any(Number),
       });
 
       expect(transaction.startChild.mock.calls[1][0]).toEqual({
-        description: '<DecoratedComponent>',
+        name: '<DecoratedComponent>',
         op: 'ui.angular.ngAfterViewInit',
         origin: 'auto.ui.angular.trace_method_decorator',
         startTimestamp: expect.any(Number),

diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts
@@ -144,8 +144,7 @@ export class SentrySpan implements SpanInterface {
       ...spanContext.attributes,
     });
 
-    // eslint-disable-next-line deprecation/deprecation
-    this._name = spanContext.name || spanContext.description;
+    this._name = spanContext.name;
 
     if (spanContext.parentSpanId) {
       this._parentSpanId = spanContext.parentSpanId;
@@ -165,38 +164,6 @@ export class SentrySpan implements SpanInterface {
   // This rule conflicts with another eslint rule :(
   /* eslint-disable @typescript-eslint/member-ordering */
 
-  /**
-   * An alias for `description` of the Span.
-   * @deprecated Use `spanToJSON(span).description` instead.
-   */
-  public get name(): string {
-    return this._name || '';
-  }
-
-  /**
-   * Update the name of the span.
-   * @deprecated Use `spanToJSON(span).description` instead.
-   */
-  public set name(name: string) {
-    this.updateName(name);
-  }
-
-  /**
-   * Get the description of the Span.
-   * @deprecated Use `spanToJSON(span).description` instead.
-   */
-  public get description(): string | undefined {
-    return this._name;
-  }
-
-  /**
-   * Get the description of the Span.
-   * @deprecated Use `spanToJSON(span).description` instead.
-   */
-  public set description(description: string | undefined) {
-    this._name = description;
-  }
-
   /**
    * The ID of the trace.
    * @deprecated Use `spanContext().traceId` instead.
@@ -486,15 +453,6 @@ export class SentrySpan implements SpanInterface {
     return this;
   }
 
-  /**
-   * @inheritdoc
-   *
-   * @deprecated Use `.updateName()` instead.
-   */
-  public setName(name: string): void {
-    this.updateName(name);
-  }
-
   /**
    * @inheritDoc
    */
@@ -551,7 +509,7 @@ export class SentrySpan implements SpanInterface {
   public toContext(): SpanContext {
     return dropUndefinedKeys({
       data: this._getData(),
-      description: this._name,
+      name: this._name,
       endTimestamp: this._endTime,
       // eslint-disable-next-line deprecation/deprecation
       op: this.op,
@@ -574,8 +532,7 @@ export class SentrySpan implements SpanInterface {
   public updateWithContext(spanContext: SpanContext): this {
     // eslint-disable-next-line deprecation/deprecation
     this.data = spanContext.data || {};
-    // eslint-disable-next-line deprecation/deprecation
-    this._name = spanContext.name || spanContext.description;
+    this._name = spanContext.name;
     this._endTime = spanContext.endTimestamp;
     // eslint-disable-next-line deprecation/deprecation
     this.op = spanContext.op;

diff --git a/packages/core/src/tracing/transaction.ts b/packages/core/src/tracing/transaction.ts
@@ -91,23 +91,6 @@ export class Transaction extends SentrySpan implements TransactionInterface {
   // This sadly conflicts with the getter/setter ordering :(
   /* eslint-disable @typescript-eslint/member-ordering */
 
-  /**
-   * Getter for `name` property.
-   * @deprecated Use `spanToJSON(span).description` instead.
-   */
-  public get name(): string {
-    return this._name;
-  }
-
-  /**
-   * Setter for `name` property, which also sets `source` as custom.
-   * @deprecated Use `updateName()` and `setMetadata()` instead.
-   */
-  public set name(newName: string) {
-    // eslint-disable-next-line deprecation/deprecation
-    this.setName(newName);
-  }
-
   /**
    * Get the metadata for this transaction.
    * @deprecated Use `spanGetMetadata(transaction)` instead.
@@ -138,19 +121,10 @@ export class Transaction extends SentrySpan implements TransactionInterface {
 
   /* eslint-enable @typescript-eslint/member-ordering */
 
-  /**
-   * Setter for `name` property, which also sets `source` on the metadata.
-   *
-   * @deprecated Use `.updateName()` and `.setAttribute()` instead.
-   */
-  public setName(name: string, source: TransactionSource = 'custom'): void {
-    this._name = name;
-    this.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source);
-  }
-
   /** @inheritdoc */
   public updateName(name: string): this {
     this._name = name;
+    this.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'custom');
     return this;
   }
 

diff --git a/packages/core/test/lib/tracing/span.test.ts b/packages/core/test/lib/tracing/span.test.ts
@@ -7,54 +7,16 @@ describe('span', () => {
     /* eslint-disable deprecation/deprecation */
     it('works with name', () => {
       const span = new SentrySpan({ name: 'span name' });
-      expect(span.name).toEqual('span name');
-      expect(span.description).toEqual('span name');
-    });
-
-    it('works with description', () => {
-      const span = new SentrySpan({ description: 'span name' });
-      expect(span.name).toEqual('span name');
-      expect(span.description).toEqual('span name');
-    });
-
-    it('works without name', () => {
-      const span = new SentrySpan({});
-      expect(span.name).toEqual('');
-      expect(span.description).toEqual(undefined);
-    });
-
-    it('allows to update the name via setter', () => {
-      const span = new SentrySpan({ name: 'span name' });
-      expect(span.name).toEqual('span name');
-      expect(span.description).toEqual('span name');
-
-      span.name = 'new name';
-
-      expect(span.name).toEqual('new name');
-      expect(span.description).toEqual('new name');
-    });
-
-    it('allows to update the name via setName', () => {
-      const span = new SentrySpan({ name: 'span name' });
-      expect(span.name).toEqual('span name');
-      expect(span.description).toEqual('span name');
-
-      // eslint-disable-next-line deprecation/deprecation
-      span.setName('new name');
-
-      expect(span.name).toEqual('new name');
-      expect(span.description).toEqual('new name');
+      expect(spanToJSON(span).description).toEqual('span name');
     });
 
     it('allows to update the name via updateName', () => {
       const span = new SentrySpan({ name: 'span name' });
-      expect(span.name).toEqual('span name');
-      expect(span.description).toEqual('span name');
+      expect(spanToJSON(span).description).toEqual('span name');
 
       span.updateName('new name');
 
-      expect(span.name).toEqual('new name');
-      expect(span.description).toEqual('new name');
+      expect(spanToJSON(span).description).toEqual('new name');
     });
   });
   /* eslint-enable deprecation/deprecation */

diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts
@@ -105,7 +105,7 @@ describe('startSpan', () => {
       }
       expect(ref).toBeDefined();
 
-      expect(ref.name).toEqual('GET users/[id]');
+      expect(spanToJSON(ref).description).toEqual('GET users/[id]');
       expect(ref.status).toEqual(isError ? 'internal_error' : undefined);
     });
 
@@ -192,7 +192,7 @@ describe('startSpan', () => {
       }
 
       expect(ref.spanRecorder.spans).toHaveLength(2);
-      expect(ref.spanRecorder.spans[1].description).toEqual('SELECT * from users');
+      expect(spanToJSON(ref.spanRecorder.spans[1]).description).toEqual('SELECT * from users');
       expect(ref.spanRecorder.spans[1].parentSpanId).toEqual(ref.spanId);
       expect(ref.spanRecorder.spans[1].status).toEqual(isError ? 'internal_error' : undefined);
     });