feat(core): Remove deprecated props from Span interface (#10854)

Instead, in places we need it we cast to a `SentrySpan` which still has
the things in place, for now.

With this, our span interface is _almost_ the same as for otel spans -
missing are only:
* Aligning `traceFlags` - this has apparently been updated in OTEL to
have type `number` only, which makes this a bit easier.
* Aligning `setStatus` which has a different signature in OTEL.

I'll do these in follow ups!

The biggest work here was fixing tests - I tried to rewrite tests to do
less mocking where possible, which IMHO should cover actual
functionality better than before (e.g. in svelte).

Author: mydea

packages/astro/test/server/meta.test.ts

@@ -1,25 +1,23 @@
 import * as SentryCore from '@sentry/core';
+import { SentrySpan } from '@sentry/core';
+import type { Transaction } from '@sentry/types';
 import { vi } from 'vitest';
 
 import { getTracingMetaTags, isValidBaggageString } from '../../src/server/meta';
 
 const TRACE_FLAG_SAMPLED = 0x1;
 
-const mockedSpan = {
-  isRecording: () => true,
-  spanContext: () => {
-    return {
-      traceId: '12345678901234567890123456789012',
-      spanId: '1234567890123456',
-      traceFlags: TRACE_FLAG_SAMPLED,
-    };
-  },
-  transaction: {
-    getDynamicSamplingContext: () => ({
-      environment: 'production',
-    }),
-  },
-} as any;
+const mockedSpan = new SentrySpan({
+  traceId: '12345678901234567890123456789012',
+  spanId: '1234567890123456',
+  sampled: true,
+});
+// eslint-disable-next-line deprecation/deprecation
+mockedSpan.transaction = {
+  getDynamicSamplingContext: () => ({
+    environment: 'production',
+  }),
+} as Transaction;
 
 const mockedClient = {} as any;
 

packages/core/src/scope.ts

@@ -26,6 +26,7 @@ import type {
 import { dateTimestampInSeconds, isPlainObject, logger, uuid4 } from '@sentry/utils';
 
 import { updateSession } from './session';
+import type { SentrySpan } from './tracing/sentrySpan';
 
 /**
  * Default value for maximum number of breadcrumbs added to an event.
@@ -329,10 +330,15 @@ export class Scope implements ScopeInterface {
     // Often, this span (if it exists at all) will be a transaction, but it's not guaranteed to be. Regardless, it will
     // have a pointer to the currently-active transaction.
     const span = this._span;
+
     // Cannot replace with getRootSpan because getRootSpan returns a span, not a transaction
     // Also, this method will be removed anyway.
     // eslint-disable-next-line deprecation/deprecation
-    return span && span.transaction;
+    if (span && (span as SentrySpan).transaction) {
+      // eslint-disable-next-line deprecation/deprecation
+      return (span as SentrySpan).transaction;
+    }
+    return undefined;
   }
 
   /**

packages/core/src/tracing/errors.ts

@@ -10,6 +10,11 @@ import { getActiveTransaction } from './utils';
 
 let errorsInstrumented = false;
 
+/**  Only exposed for testing */
+export function _resetErrorsInstrumented(): void {
+  errorsInstrumented = false;
+}
+
 /**
  * Configures global error listeners
  */

packages/core/src/tracing/sentrySpan.ts

@@ -1,6 +1,6 @@
 import type {
   Primitive,
-  Span as SpanInterface,
+  Span,
   SpanAttributeValue,
   SpanAttributes,
   SpanContext,
@@ -62,7 +62,7 @@ export class SpanRecorder {
 /**
  * Span contains all data about a span
  */
-export class SentrySpan implements SpanInterface {
+export class SentrySpan implements Span {
   /**
    * Tags for the span.
    * @deprecated Use `spanToJSON(span).atttributes` instead.
@@ -277,7 +277,7 @@ export class SentrySpan implements SpanInterface {
    */
   public startChild(
     spanContext?: Pick<SpanContext, Exclude<keyof SpanContext, 'sampled' | 'traceId' | 'parentSpanId'>>,
-  ): SpanInterface {
+  ): Span {
     const childSpan = new SentrySpan({
       ...spanContext,
       parentSpanId: this._spanId,

packages/core/src/tracing/trace.ts

@@ -1,7 +1,6 @@
 import type { Hub, Scope, Span, SpanTimeInput, StartSpanOptions, TransactionContext } from '@sentry/types';
 
 import { dropUndefinedKeys, logger, tracingContextFromHeaders } from '@sentry/utils';
-
 import { getCurrentScope, getIsolationScope, withScope } from '../currentScopes';
 
 import { DEBUG_BUILD } from '../debug-build';
@@ -10,6 +9,7 @@ import { handleCallbackErrors } from '../utils/handleCallbackErrors';
 import { hasTracingEnabled } from '../utils/hasTracingEnabled';
 import { spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils';
 import { getDynamicSamplingContextFromSpan } from './dynamicSamplingContext';
+import type { SentrySpan } from './sentrySpan';
 import { addChildSpanToSpan, getActiveSpan, setCapturedScopesOnSpan } from './utils';
 
 /**
@@ -30,7 +30,7 @@ export function startSpan<T>(context: StartSpanOptions, callback: (span: Span |
     // eslint-disable-next-line deprecation/deprecation
     const hub = getCurrentHub();
     // eslint-disable-next-line deprecation/deprecation
-    const parentSpan = scope.getSpan();
+    const parentSpan = scope.getSpan() as SentrySpan | undefined;
 
     const shouldSkipSpan = context.onlyIfParent && !parentSpan;
     const activeSpan = shouldSkipSpan
@@ -79,7 +79,7 @@ export function startSpanManual<T>(
     // eslint-disable-next-line deprecation/deprecation
     const hub = getCurrentHub();
     // eslint-disable-next-line deprecation/deprecation
-    const parentSpan = scope.getSpan();
+    const parentSpan = scope.getSpan() as SentrySpan | undefined;
 
     const shouldSkipSpan = context.onlyIfParent && !parentSpan;
     const activeSpan = shouldSkipSpan
@@ -130,8 +130,8 @@ export function startInactiveSpan(context: StartSpanOptions): Span | undefined {
   const hub = getCurrentHub();
   const parentSpan = context.scope
     ? // eslint-disable-next-line deprecation/deprecation
-      context.scope.getSpan()
-    : getActiveSpan();
+      (context.scope.getSpan() as SentrySpan | undefined)
+    : (getActiveSpan() as SentrySpan | undefined);
 
   const shouldSkipSpan = context.onlyIfParent && !parentSpan;
 
@@ -264,7 +264,7 @@ function createChildSpanOrTransaction(
     forceTransaction,
     scope,
   }: {
-    parentSpan: Span | undefined;
+    parentSpan: SentrySpan | undefined;
     spanContext: TransactionContext;
     forceTransaction?: boolean;
     scope: Scope;

packages/core/src/utils/getRootSpan.ts

@@ -1,4 +1,5 @@
 import type { Span } from '@sentry/types';
+import type { SentrySpan } from './../tracing/sentrySpan';
 
 /**
  * Returns the root span of a given span.
@@ -11,5 +12,5 @@ import type { Span } from '@sentry/types';
 export function getRootSpan(span: Span): Span | undefined {
   // TODO (v8): Remove this check and just return span
   // eslint-disable-next-line deprecation/deprecation
-  return span.transaction;
+  return (span as SentrySpan).transaction ? (span as SentrySpan).transaction : undefined;
 }

packages/core/src/utils/prepareEvent.ts

@@ -332,8 +332,7 @@ function normalizeEvent(event: Event | null, depth: number, maxBreadth: number):
 
       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 = normalize(data, depth, maxBreadth);
+        span.setAttributes(normalize(data, depth, maxBreadth));
       }
 
       return span;

packages/core/src/utils/spanUtils.ts

@@ -62,27 +62,28 @@ function ensureTimestampInSeconds(timestamp: number): number {
   return isMs ? timestamp / 1000 : timestamp;
 }
 
+type SpanWithToJSON = Span & { toJSON: () => SpanJSON };
+
 /**
  * Convert a span to a JSON representation.
  * Note that all fields returned here are optional and need to be guarded against.
  *
  * Note: Because of this, we currently have a circular type dependency (which we opted out of in package.json).
  * This is not avoidable as we need `spanToJSON` in `spanUtils.ts`, which in turn is needed by `span.ts` for backwards compatibility.
  * And `spanToJSON` needs the Span class from `span.ts` to check here.
- * TODO v8: When we remove the deprecated stuff from `span.ts`, we can remove the circular dependency again.
  */
 export function spanToJSON(span: Span): Partial<SpanJSON> {
   if (spanIsSentrySpan(span)) {
     return span.getSpanJSON();
   }
 
   // Fallback: We also check for `.toJSON()` here...
-  // eslint-disable-next-line deprecation/deprecation
-  if (typeof span.toJSON === 'function') {
-    // eslint-disable-next-line deprecation/deprecation
-    return span.toJSON();
+  if (typeof (span as SpanWithToJSON).toJSON === 'function') {
+    return (span as SpanWithToJSON).toJSON();
   }
 
+  // TODO: Also handle OTEL spans here!
+
   return {};
 }
 

packages/core/test/lib/hint.test.ts

@@ -1,6 +1,6 @@
-import { captureEvent, getCurrentScope } from '@sentry/core';
 import { GLOBAL_OBJ } from '@sentry/utils';
 
+import { captureEvent, getCurrentScope } from '../../src';
 import { initAndBind } from '../../src/sdk';
 import { TestClient, getDefaultTestClientOptions } from '../mocks/client';
 import { AddAttachmentTestIntegration } from '../mocks/integration';

packages/core/test/lib/integrations/requestdata.test.ts

@@ -1,9 +1,8 @@
 import type { IncomingMessage } from 'http';
-import type { RequestDataIntegrationOptions } from '@sentry/core';
-import { setCurrentClient } from '@sentry/core';
-import { RequestData } from '@sentry/core';
 import type { Event, EventProcessor } from '@sentry/types';
 import * as sentryUtils from '@sentry/utils';
+import type { RequestDataIntegrationOptions } from '../../../src';
+import { RequestData, setCurrentClient } from '../../../src';
 
 import { TestClient, getDefaultTestClientOptions } from '../../mocks/client';
 

packages/core/test/lib/scope.test.ts

@@ -961,18 +961,16 @@ describe('withActiveSpan()', () => {
     });
   });
 
-  it('should create child spans when calling startSpan within the callback', done => {
-    expect.assertions(2);
+  it('should create child spans when calling startSpan within the callback', () => {
     const inactiveSpan = startInactiveSpan({ name: 'inactive-span' });
 
-    withActiveSpan(inactiveSpan!, () => {
-      startSpan({ name: 'child-span' }, childSpan => {
-        // eslint-disable-next-line deprecation/deprecation
-        expect(childSpan?.parentSpanId).toBe(inactiveSpan?.spanContext().spanId);
-        expect(spanToJSON(childSpan!).parent_span_id).toBe(inactiveSpan?.spanContext().spanId);
-        done();
+    const parentSpanId = withActiveSpan(inactiveSpan!, () => {
+      return startSpan({ name: 'child-span' }, childSpan => {
+        return spanToJSON(childSpan!).parent_span_id;
       });
     });
+
+    expect(parentSpanId).toBe(inactiveSpan?.spanContext().spanId);
   });
 
   it('when `null` is passed, no span should be active within the callback', () => {

packages/core/test/lib/sdk.test.ts

@@ -1,5 +1,5 @@
-import { captureCheckIn, getCurrentScope, setCurrentClient } from '@sentry/core';
 import type { Client, Integration, IntegrationFnResult } from '@sentry/types';
+import { captureCheckIn, getCurrentScope, setCurrentClient } from '../../src';
 
 import { installedIntegrations } from '../../src/integration';
 import { initAndBind } from '../../src/sdk';

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

@@ -1,13 +1,14 @@
-import { addTracingExtensions, setCurrentClient, spanToJSON, startInactiveSpan, startSpan } from '@sentry/core';
 import type { HandlerDataError, HandlerDataUnhandledRejection } from '@sentry/types';
+import { addTracingExtensions, setCurrentClient, spanToJSON, startInactiveSpan, startSpan } from '../../../src';
 
-import { registerErrorInstrumentation } from '../../../src/tracing/errors';
+import { _resetErrorsInstrumented, registerErrorInstrumentation } from '../../../src/tracing/errors';
 import { TestClient, getDefaultTestClientOptions } from '../../mocks/client';
 
 const mockAddGlobalErrorInstrumentationHandler = jest.fn();
 const mockAddGlobalUnhandledRejectionInstrumentationHandler = jest.fn();
 let mockErrorCallback: (data: HandlerDataError) => void = () => {};
 let mockUnhandledRejectionCallback: (data: HandlerDataUnhandledRejection) => void = () => {};
+
 jest.mock('@sentry/utils', () => {
   const actual = jest.requireActual('@sentry/utils');
   return {
@@ -36,6 +37,7 @@ describe('registerErrorHandlers()', () => {
     const client = new TestClient(options);
     setCurrentClient(client);
     client.init();
+    _resetErrorsInstrumented();
   });
 
   it('registers error instrumentation', () => {

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

@@ -1,6 +1,12 @@
+/* eslint-disable deprecation/deprecation */
+import { TestClient, getDefaultTestClientOptions } from '../../mocks/client';
+
 import {
+  IdleTransaction,
+  SentrySpan,
   TRACING_DEFAULTS,
   Transaction,
+  getClient,
   getCurrentHub,
   getCurrentScope,
   getGlobalScope,
@@ -10,11 +16,7 @@ import {
   startInactiveSpan,
   startSpan,
   startSpanManual,
-} from '@sentry/core';
-/* eslint-disable deprecation/deprecation */
-import { TestClient, getDefaultTestClientOptions } from '../../mocks/client';
-
-import { IdleTransaction, SentrySpan, getClient } from '../../../src';
+} from '../../../src';
 import { IdleTransactionSpanRecorder } from '../../../src/tracing/idletransaction';
 
 const dsn = 'https://[email protected]/42';
@@ -47,6 +49,7 @@ describe('IdleTransaction', () => {
       transaction.initSpanRecorder(10);
 
       const scope = getCurrentScope();
+
       // eslint-disable-next-line deprecation/deprecation
       expect(scope.getTransaction()).toBe(transaction);
     });

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

@@ -287,10 +287,7 @@ describe('startSpan', () => {
       expect(getCurrentScope()).not.toBe(initialScope);
       expect(getCurrentScope()).toBe(manualScope);
       expect(getActiveSpan()).toBe(span);
-
       expect(spanToJSON(span!).parent_span_id).toBe('parent-span-id');
-      // eslint-disable-next-line deprecation/deprecation
-      expect(span?.parentSpanId).toBe('parent-span-id');
     });
 
     expect(getCurrentScope()).toBe(initialScope);
@@ -565,8 +562,6 @@ describe('startSpanManual', () => {
       expect(getCurrentScope()).toBe(manualScope);
       expect(getActiveSpan()).toBe(span);
       expect(spanToJSON(span!).parent_span_id).toBe('parent-span-id');
-      // eslint-disable-next-line deprecation/deprecation
-      expect(span?.parentSpanId).toBe('parent-span-id');
 
       finish();
 
@@ -789,8 +784,6 @@ describe('startInactiveSpan', () => {
 
     expect(span).toBeDefined();
     expect(spanToJSON(span!).parent_span_id).toBe('parent-span-id');
-    // eslint-disable-next-line deprecation/deprecation
-    expect(span?.parentSpanId).toBe('parent-span-id');
     expect(getActiveSpan()).toBeUndefined();
 
     span?.end();

packages/node/src/integrations/http.ts

@@ -1,7 +1,7 @@
 /* eslint-disable max-lines */
 import type * as http from 'http';
 import type * as https from 'https';
-import type { Hub } from '@sentry/core';
+import type { Hub, SentrySpan } from '@sentry/core';
 import { defineIntegration, getIsolationScope, hasTracingEnabled } from '@sentry/core';
 import {
   addBreadcrumb,
@@ -319,7 +319,7 @@ function _createWrappedRequestMethodFactory(
 
       const scope = getCurrentScope();
       const isolationScope = getIsolationScope();
-      const parentSpan = getActiveSpan();
+      const parentSpan = getActiveSpan() as SentrySpan;
 
       const data = getRequestSpanData(requestUrl, requestOptions);
 

packages/node/src/integrations/undici/index.ts

@@ -1,3 +1,4 @@
+import type { SentrySpan } from '@sentry/core';
 import {
   addBreadcrumb,
   defineIntegration,
@@ -183,7 +184,7 @@ export class Undici implements Integration {
     const clientOptions = client.getOptions();
     const scope = getCurrentScope();
     const isolationScope = getIsolationScope();
-    const parentSpan = getActiveSpan();
+    const parentSpan = getActiveSpan() as SentrySpan;
 
     const span = this._shouldCreateSpan(stringUrl) ? createRequestSpan(parentSpan, request, stringUrl) : undefined;
     if (span) {
@@ -320,7 +321,7 @@ function setHeadersOnRequest(
 }
 
 function createRequestSpan(
-  activeSpan: Span | undefined,
+  activeSpan: SentrySpan | undefined,
   request: RequestWithSentry,
   stringUrl: string,
 ): Span | undefined {