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';