feat(core): Streamline SpanJSON type (#14693)

Previously `spanToJSON` would return `Partial<SpanJSON>`. This meant
that you always had to guard all the stuff you picked from it - even
though in reality, we know that certain stuff will always be there.

To alleviate this, this PR changes this so that `spanToJSON` actually
returns `SpanJSON`. This means that in the fallback case, we return
`data: {}`, as well as a random (current) timestamp. Since we know that
in reality we will only have the two scenarios that we properly handle,
this is fine IMHO and makes usage of this everywhere else a little bit
less painful.

In a follow up, we can get rid of a bunch of `const data =
spanToJSON(span).data || {}` type code.

While at it, I also updated the type of `data` to `SpanAttributes`,
which is correct (it was `Record<string, any>` before). Since we only
allow span attributes to be set on this anyhow now, we can type this
better. This also uncovered a few places with slightly "incorrect" type
checks, I updated those too.

This change is on the v9 branch - I think it should not really be
breaking to a user in any way, as we simply return more data from
`spanToJSON`, and type what `data` is on there more tightly, so no
existing code relying on this should break. But to be safe, I figured we
may as well do that on v9 only.

dev-packages/node-integration-tests/suites/public-api/startSpan/with-nested-spans/test.ts

@@ -30,10 +30,12 @@ test('should report finished spans as children of the root transaction.', done =
             {
               description: 'span_3',
               parent_span_id: rootSpanId,
+              data: {},
             },
             {
               description: 'span_5',
               parent_span_id: span3Id,
+              data: {},
             },
           ] as SpanJSON[],
         });

packages/core/src/types-hoist/span.ts

@@ -44,7 +44,7 @@ export type SpanTimeInput = HrTime | number | Date;
 
 /** A JSON representation of a span. */
 export interface SpanJSON {
-  data?: { [key: string]: any };
+  data: SpanAttributes;
   description?: string;
   op?: string;
   parent_span_id?: string;

packages/core/src/utils/spanUtils.ts

@@ -112,42 +112,41 @@ function ensureTimestampInSeconds(timestamp: number): number {
 // 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.
-export function spanToJSON(span: Span): Partial<SpanJSON> {
+export function spanToJSON(span: Span): SpanJSON {
   if (spanIsSentrySpan(span)) {
     return span.getSpanJSON();
   }
 
-  try {
-    const { spanId: span_id, traceId: trace_id } = span.spanContext();
-
-    // Handle a span from @opentelemetry/sdk-base-trace's `Span` class
-    if (spanIsOpenTelemetrySdkTraceBaseSpan(span)) {
-      const { attributes, startTime, name, endTime, parentSpanId, status } = span;
-
-      return dropUndefinedKeys({
-        span_id,
-        trace_id,
-        data: attributes,
-        description: name,
-        parent_span_id: parentSpanId,
-        start_timestamp: spanTimeInputToSeconds(startTime),
-        // This is [0,0] by default in OTEL, in which case we want to interpret this as no end time
-        timestamp: spanTimeInputToSeconds(endTime) || undefined,
-        status: getStatusMessage(status),
-        op: attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP],
-        origin: attributes[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] as SpanOrigin | undefined,
-        _metrics_summary: getMetricSummaryJsonForSpan(span),
-      });
-    }
+  const { spanId: span_id, traceId: trace_id } = span.spanContext();
 
-    // Finally, at least we have `spanContext()`....
-    return {
+  // Handle a span from @opentelemetry/sdk-base-trace's `Span` class
+  if (spanIsOpenTelemetrySdkTraceBaseSpan(span)) {
+    const { attributes, startTime, name, endTime, parentSpanId, status } = span;
+
+    return dropUndefinedKeys({
       span_id,
       trace_id,
-    };
-  } catch {
-    return {};
+      data: attributes,
+      description: name,
+      parent_span_id: parentSpanId,
+      start_timestamp: spanTimeInputToSeconds(startTime),
+      // This is [0,0] by default in OTEL, in which case we want to interpret this as no end time
+      timestamp: spanTimeInputToSeconds(endTime) || undefined,
+      status: getStatusMessage(status),
+      op: attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP],
+      origin: attributes[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] as SpanOrigin | undefined,
+      _metrics_summary: getMetricSummaryJsonForSpan(span),
+    });
   }
+
+  // Finally, at least we have `spanContext()`....
+  // This should not actually happen in reality, but we need to handle it for type safety.
+  return {
+    span_id,
+    trace_id,
+    start_timestamp: 0,
+    data: {},
+  };
 }
 
 function spanIsOpenTelemetrySdkTraceBaseSpan(span: Span): span is OpenTelemetrySdkTraceBaseSpan {

packages/core/test/lib/baseclient.test.ts

@@ -938,14 +938,14 @@ describe('BaseClient', () => {
         event_id: '972f45b826a248bba98e990878a177e1',
         spans: [
           {
-            data: { _sentry_extra_metrics: { M1: { value: 1 }, M2: { value: 2 } } },
             description: 'first-paint',
             timestamp: 1591603196.637835,
             op: 'paint',
             parent_span_id: 'a3df84a60c2e4e76',
             span_id: '9e15bf99fbe4bc80',
             start_timestamp: 1591603196.637835,
             trace_id: '86f39e84263a4de99c326acab3bfe3bd',
+            data: {},
           },
           {
             description: 'first-contentful-paint',
@@ -955,6 +955,7 @@ describe('BaseClient', () => {
             span_id: 'aa554c1f506b0783',
             start_timestamp: 1591603196.637835,
             trace_id: '86f39e84263a4de99c326acab3bfe3bd',
+            data: {},
           },
         ],
         start_timestamp: 1591603196.614865,
@@ -1016,12 +1017,14 @@ describe('BaseClient', () => {
             span_id: '9e15bf99fbe4bc80',
             start_timestamp: 1591603196.637835,
             trace_id: '86f39e84263a4de99c326acab3bfe3bd',
+            data: {},
           },
           {
             description: 'second span',
             span_id: 'aa554c1f506b0783',
             start_timestamp: 1591603196.637835,
             trace_id: '86f39e84263a4de99c326acab3bfe3bd',
+            data: {},
           },
         ],
       };
@@ -1076,9 +1079,9 @@ describe('BaseClient', () => {
         transaction: '/dogs/are/great',
         type: 'transaction',
         spans: [
-          { span_id: 'span1', trace_id: 'trace1', start_timestamp: 1234 },
-          { span_id: 'span2', trace_id: 'trace1', start_timestamp: 1234 },
-          { span_id: 'span3', trace_id: 'trace1', start_timestamp: 1234 },
+          { span_id: 'span1', trace_id: 'trace1', start_timestamp: 1234, data: {} },
+          { span_id: 'span2', trace_id: 'trace1', start_timestamp: 1234, data: {} },
+          { span_id: 'span3', trace_id: 'trace1', start_timestamp: 1234, data: {} },
         ],
       });
 
@@ -1107,12 +1110,14 @@ describe('BaseClient', () => {
             span_id: '9e15bf99fbe4bc80',
             start_timestamp: 1591603196.637835,
             trace_id: '86f39e84263a4de99c326acab3bfe3bd',
+            data: {},
           },
           {
             description: 'second span',
             span_id: 'aa554c1f506b0783',
             start_timestamp: 1591603196.637835,
             trace_id: '86f39e84263a4de99c326acab3bfe3bd',
+            data: {},
           },
         ],
       };
@@ -1181,12 +1186,14 @@ describe('BaseClient', () => {
             span_id: '9e15bf99fbe4bc80',
             start_timestamp: 1591603196.637835,
             trace_id: '86f39e84263a4de99c326acab3bfe3bd',
+            data: {},
           },
           {
             description: 'second span',
             span_id: 'aa554c1f506b0783',
             start_timestamp: 1591603196.637835,
             trace_id: '86f39e84263a4de99c326acab3bfe3bd',
+            data: {},
           },
         ],
       };

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

@@ -18,6 +18,8 @@ describe('SentryNonRecordingSpan', () => {
     expect(spanToJSON(span)).toEqual({
       span_id: expect.stringMatching(/[a-f0-9]{16}/),
       trace_id: expect.stringMatching(/[a-f0-9]{32}/),
+      data: {},
+      start_timestamp: 0,
     });
 
     // Ensure all methods work
@@ -32,6 +34,8 @@ describe('SentryNonRecordingSpan', () => {
     expect(spanToJSON(span)).toEqual({
       span_id: expect.stringMatching(/[a-f0-9]{16}/),
       trace_id: expect.stringMatching(/[a-f0-9]{32}/),
+      data: {},
+      start_timestamp: 0,
     });
   });
 });

packages/core/test/lib/utils/spanUtils.test.ts

@@ -287,10 +287,21 @@ describe('spanToJSON', () => {
     });
   });
 
-  it('returns empty object for unknown span implementation', () => {
-    const span = { other: 'other' };
-
-    expect(spanToJSON(span as unknown as Span)).toEqual({});
+  it('returns minimal object for unknown span implementation', () => {
+    const span = {
+      // This is the minimal interface we require from a span
+      spanContext: () => ({
+        spanId: 'SPAN-1',
+        traceId: 'TRACE-1',
+      }),
+    };
+
+    expect(spanToJSON(span as unknown as Span)).toEqual({
+      span_id: 'SPAN-1',
+      trace_id: 'TRACE-1',
+      start_timestamp: 0,
+      data: {},
+    });
   });
 });
 

packages/node/src/integrations/tracing/graphql.ts

@@ -67,9 +67,9 @@ export const instrumentGraphql = generateInstrumentOnce<GraphqlOptions>(
           // We keep track of each operation on the root span
           // This can either be a string, or an array of strings (if there are multiple operations)
           if (Array.isArray(existingOperations)) {
-            existingOperations.push(newOperation);
+            (existingOperations as string[]).push(newOperation);
             rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION, existingOperations);
-          } else if (existingOperations) {
+          } else if (typeof existingOperations === 'string') {
             rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION, [existingOperations, newOperation]);
           } else {
             rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION, newOperation);

packages/node/src/integrations/tracing/vercelai/index.ts

@@ -32,8 +32,8 @@ const _vercelAIIntegration = (() => {
             span.data['ai.prompt_tokens.used'] = attributes['ai.usage.promptTokens'];
           }
           if (
-            attributes['ai.usage.completionTokens'] != undefined &&
-            attributes['ai.usage.promptTokens'] != undefined
+            typeof attributes['ai.usage.completionTokens'] == 'number' &&
+            typeof attributes['ai.usage.promptTokens'] == 'number'
           ) {
             span.data['ai.total_tokens.used'] =
               attributes['ai.usage.completionTokens'] + attributes['ai.usage.promptTokens'];
@@ -56,13 +56,13 @@ const _vercelAIIntegration = (() => {
         }
 
         // The id of the model
-        const aiModelId: string | undefined = attributes['ai.model.id'];
+        const aiModelId = attributes['ai.model.id'];
 
         // the provider of the model
-        const aiModelProvider: string | undefined = attributes['ai.model.provider'];
+        const aiModelProvider = attributes['ai.model.provider'];
 
         // both of these must be defined for the integration to work
-        if (!aiModelId || !aiModelProvider) {
+        if (typeof aiModelId !== 'string' || typeof aiModelProvider !== 'string' || !aiModelId || !aiModelProvider) {
           return;
         }
 
@@ -137,9 +137,10 @@ const _vercelAIIntegration = (() => {
         span.updateName(nameWthoutAi);
 
         // If a Telemetry name is set and it is a pipeline span, use that as the operation name
-        if (attributes['ai.telemetry.functionId'] && isPipelineSpan) {
-          span.updateName(attributes['ai.telemetry.functionId']);
-          span.setAttribute('ai.pipeline.name', attributes['ai.telemetry.functionId']);
+        const functionId = attributes['ai.telemetry.functionId'];
+        if (functionId && typeof functionId === 'string' && isPipelineSpan) {
+          span.updateName(functionId);
+          span.setAttribute('ai.pipeline.name', functionId);
         }
 
         if (attributes['ai.prompt']) {

packages/opentelemetry/src/propagator.ts

@@ -316,7 +316,7 @@ function getCurrentURL(span: Span): string | undefined {
   // `ATTR_URL_FULL` is the new attribute, but we still support the old one, `SEMATTRS_HTTP_URL`, for now.
   // eslint-disable-next-line deprecation/deprecation
   const urlAttribute = spanData?.[SEMATTRS_HTTP_URL] || spanData?.[ATTR_URL_FULL];
-  if (urlAttribute) {
+  if (typeof urlAttribute === 'string') {
     return urlAttribute;
   }
 

packages/opentelemetry/test/trace.test.ts

@@ -1565,6 +1565,8 @@ describe('continueTrace', () => {
         expect(spanToJSON(span)).toEqual({
           span_id: '1121201211212012',
           trace_id: '12312012123120121231201212312012',
+          data: {},
+          start_timestamp: 0,
         });
         expect(getSamplingDecision(span.spanContext())).toBe(false);
         expect(spanIsSampled(span)).toBe(false);
@@ -1596,6 +1598,8 @@ describe('continueTrace', () => {
         expect(spanToJSON(span)).toEqual({
           span_id: '1121201211212012',
           trace_id: '12312012123120121231201212312012',
+          data: {},
+          start_timestamp: 0,
         });
         expect(getSamplingDecision(span.spanContext())).toBe(true);
         expect(spanIsSampled(span)).toBe(true);
@@ -1630,6 +1634,8 @@ describe('continueTrace', () => {
         expect(spanToJSON(span)).toEqual({
           span_id: '1121201211212012',
           trace_id: '12312012123120121231201212312012',
+          data: {},
+          start_timestamp: 0,
         });
         expect(getSamplingDecision(span.spanContext())).toBe(true);
         expect(spanIsSampled(span)).toBe(true);

packages/vue/test/router.test.ts

@@ -13,7 +13,9 @@ vi.mock('@sentry/core', async () => {
   const actual = await vi.importActual('@sentry/core');
   return {
     ...actual,
-    getActiveSpan: vi.fn().mockReturnValue({}),
+    getActiveSpan: vi.fn().mockReturnValue({
+      spanContext: () => ({ traceId: '1234', spanId: '5678' }),
+    }),
   };
 });