feat(core): Update spanToJSON to handle OTEL spans (#10922)

This is an important step towards aligning OTEL & Sentry Spans 🎉 You can
see that the `Span` interface is now aligned enough that it accepts an
OTEL span.

Author: mydea

packages/angular/test/tracing.test.ts

@@ -1,6 +1,6 @@
 import { Component } from '@angular/core';
 import type { ActivatedRouteSnapshot, CanActivate, RouterStateSnapshot } from '@angular/router';
-import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
+import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, spanToJSON } from '@sentry/core';
 
 import { TraceClassDecorator, TraceDirective, TraceMethodDecorator, instrumentAngularRouting } from '../src';
 import { getParameterizedRouteFromSnapshot } from '../src/tracing';
@@ -13,7 +13,7 @@ const defaultStartTransaction = (ctx: any) => {
     ...ctx,
     updateName: jest.fn(name => (transaction.name = name)),
     setAttribute: jest.fn(),
-    toJSON: () => ({
+    getSpanJSON: () => ({
       data: {
         [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom',
         ...ctx.data,
@@ -117,7 +117,7 @@ describe('Angular Tracing', () => {
       const customStartTransaction = jest.fn((ctx: any) => {
         transaction = {
           ...ctx,
-          toJSON: () => ({
+          getSpanJSON: () => ({
             data: {
               ...ctx.data,
               [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom',
@@ -154,7 +154,7 @@ describe('Angular Tracing', () => {
 
       expect(transaction.updateName).toHaveBeenCalledTimes(0);
       expect(transaction.name).toEqual(url);
-      expect(transaction.toJSON().data).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom' });
+      expect(spanToJSON(transaction).data).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom' });
 
       env.destroy();
     });

packages/core/src/tracing/sentrySpan.ts

@@ -21,11 +21,11 @@ import { getRootSpan } from '../utils/getRootSpan';
 import {
   TRACE_FLAG_NONE,
   TRACE_FLAG_SAMPLED,
+  getStatusMessage,
   spanTimeInputToSeconds,
   spanToJSON,
   spanToTraceContext,
 } from '../utils/spanUtils';
-import { SPAN_STATUS_OK, SPAN_STATUS_UNSET } from './spanstatus';
 import { addChildSpanToSpan } from './utils';
 
 /**
@@ -500,15 +500,3 @@ export class SentrySpan implements Span {
     return hasData ? data : attributes;
   }
 }
-
-function getStatusMessage(status: SpanStatus | undefined): string | undefined {
-  if (!status || status.code === SPAN_STATUS_UNSET) {
-    return undefined;
-  }
-
-  if (status.code === SPAN_STATUS_OK) {
-    return 'ok';
-  }
-
-  return status.message || 'unknown_error';
-}

packages/core/src/tracing/transaction.ts

@@ -5,6 +5,7 @@ import type {
   Hub,
   MeasurementUnit,
   Measurements,
+  SpanJSON,
   SpanTimeInput,
   Transaction as TransactionInterface,
   TransactionContext,
@@ -253,8 +254,8 @@ export class Transaction extends SentrySpan implements TransactionInterface {
       return undefined;
     }
 
-    // We only want to include finished spans in the event
-    const finishedSpans = getSpanTree(this).filter(span => span !== this && spanToJSON(span).timestamp);
+    // The transaction span itself should be filtered out
+    const finishedSpans = getSpanTree(this).filter(span => span !== this);
 
     if (this._trimEnd && finishedSpans.length > 0) {
       const endTimes = finishedSpans.map(span => spanToJSON(span).timestamp).filter(Boolean) as number[];
@@ -263,6 +264,13 @@ export class Transaction extends SentrySpan implements TransactionInterface {
       });
     }
 
+    // We want to filter out any incomplete SpanJSON objects
+    function isFullFinishedSpan(input: Partial<SpanJSON>): input is SpanJSON {
+      return !!input.start_timestamp && !!input.timestamp && !!input.span_id && !!input.trace_id;
+    }
+
+    const spans = finishedSpans.map(span => spanToJSON(span)).filter(isFullFinishedSpan);
+
     const { scope: capturedSpanScope, isolationScope: capturedSpanIsolationScope } = getCapturedScopesOnSpan(this);
 
     // eslint-disable-next-line deprecation/deprecation
@@ -276,7 +284,7 @@ export class Transaction extends SentrySpan implements TransactionInterface {
         // We don't want to override trace context
         trace: spanToTraceContext(this),
       },
-      spans: finishedSpans.map(span => spanToJSON(span)),
+      spans,
       start_timestamp: this._startTime,
       // eslint-disable-next-line deprecation/deprecation
       tags: this.tags,

packages/core/src/utils/spanUtils.ts

@@ -1,5 +1,16 @@
-import type { Span, SpanJSON, SpanTimeInput, TraceContext } from '@sentry/types';
+import type {
+  Span,
+  SpanAttributes,
+  SpanJSON,
+  SpanOrigin,
+  SpanStatus,
+  SpanTimeInput,
+  TraceContext,
+} from '@sentry/types';
 import { dropUndefinedKeys, generateSentryTraceHeader, timestampInSeconds } from '@sentry/utils';
+import { getMetricSummaryJsonForSpan } from '../metrics/metric-summary';
+import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../semanticAttributes';
+import { SPAN_STATUS_OK, SPAN_STATUS_UNSET } from '../tracing';
 import type { SentrySpan } from '../tracing/sentrySpan';
 
 // These are aligned with OpenTelemetry trace flags
@@ -62,8 +73,6 @@ 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.
@@ -77,14 +86,52 @@ export function spanToJSON(span: Span): Partial<SpanJSON> {
     return span.getSpanJSON();
   }
 
-  // Fallback: We also check for `.toJSON()` here...
-  if (typeof (span as SpanWithToJSON).toJSON === 'function') {
-    return (span as SpanWithToJSON).toJSON();
+  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),
+      });
+    }
+
+    // Finally, at least we have `spanContext()`....
+    return {
+      span_id,
+      trace_id,
+    };
+  } catch {
+    return {};
   }
+}
 
-  // TODO: Also handle OTEL spans here!
+function spanIsOpenTelemetrySdkTraceBaseSpan(span: Span): span is OpenTelemetrySdkTraceBaseSpan {
+  const castSpan = span as OpenTelemetrySdkTraceBaseSpan;
+  return !!castSpan.attributes && !!castSpan.startTime && !!castSpan.name && !!castSpan.endTime && !!castSpan.status;
+}
 
-  return {};
+/** Exported only for tests. */
+export interface OpenTelemetrySdkTraceBaseSpan extends Span {
+  attributes: SpanAttributes;
+  startTime: SpanTimeInput;
+  name: string;
+  status: SpanStatus;
+  endTime: SpanTimeInput;
+  parentSpanId?: string;
 }
 
 /**
@@ -108,3 +155,16 @@ export function spanIsSampled(span: Span): boolean {
   // eslint-disable-next-line no-bitwise
   return Boolean(traceFlags & TRACE_FLAG_SAMPLED);
 }
+
+/** Get the status message to use for a JSON representation of a span. */
+export function getStatusMessage(status: SpanStatus | undefined): string | undefined {
+  if (!status || status.code === SPAN_STATUS_UNSET) {
+    return undefined;
+  }
+
+  if (status.code === SPAN_STATUS_OK) {
+    return 'ok';
+  }
+
+  return status.message || 'unknown_error';
+}