feat(core): Add spanToJSON() method to get span properties

mydea · mydea · commit a9970374e3e4 · 2024-01-05T14:20:21.000+01:00
This is supposed to be an internal API and not necessarily to be used by users.

add comment for circular dependency
diff --git a/packages/core/package.json b/packages/core/package.json
@@ -56,5 +56,12 @@
   "volta": {
     "extends": "../../package.json"
   },
+  "madge": {
+    "detectiveOptions": {
+      "ts": {
+        "skipTypeImports": true
+      }
+    }
+  },
   "sideEffects": false
 }
diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts
@@ -72,7 +72,10 @@ export { createCheckInEnvelope } from './checkin';
 export { hasTracingEnabled } from './utils/hasTracingEnabled';
 export { isSentryRequestUrl } from './utils/isSentryRequestUrl';
 export { handleCallbackErrors } from './utils/handleCallbackErrors';
-export { spanToTraceHeader } from './utils/spanUtils';
+export {
+  spanToTraceHeader,
+  spanToJSON,
+} from './utils/spanUtils';
 export { DEFAULT_ENVIRONMENT } from './constants';
 export { ModuleMetadata } from './integrations/metadata';
 export { RequestData } from './integrations/requestdata';
diff --git a/packages/core/src/tracing/span.ts b/packages/core/src/tracing/span.ts
@@ -6,6 +6,7 @@ import type {
   SpanAttributeValue,
   SpanAttributes,
   SpanContext,
+  SpanJSON,
   SpanOrigin,
   SpanTimeInput,
   TraceContext,
@@ -372,22 +373,9 @@ export class Span implements SpanInterface {
   }
 
   /**
-   * @inheritDoc
+   * Get JSON representation of this span.
    */
-  public toJSON(): {
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    data?: { [key: string]: any };
-    description?: string;
-    op?: string;
-    parent_span_id?: string;
-    span_id: string;
-    start_timestamp: number;
-    status?: string;
-    tags?: { [key: string]: Primitive };
-    timestamp?: number;
-    trace_id: string;
-    origin?: SpanOrigin;
-  } {
+  public getSpanJSON(): SpanJSON {
     return dropUndefinedKeys({
       data: this._getData(),
       description: this.description,
@@ -408,6 +396,14 @@ export class Span implements SpanInterface {
     return !this.endTimestamp && !!this.sampled;
   }
 
+  /**
+   * Convert the object to JSON.
+   * @deprecated Use `spanToJSON(span)` instead.
+   */
+  public toJSON(): SpanJSON {
+    return this.getSpanJSON();
+  }
+
   /**
    * Get the merged data for this span.
    * For now, this combines `data` and `attributes` together,
diff --git a/packages/core/src/tracing/transaction.ts b/packages/core/src/tracing/transaction.ts
@@ -285,6 +285,7 @@ export class Transaction extends SpanClass implements TransactionInterface {
         // We don't want to override trace context
         trace: spanToTraceContext(this),
       },
+      // TODO: Pass spans serialized via `spanToJSON()` here instead in v8.
       spans: finishedSpans,
       start_timestamp: this.startTimestamp,
       tags: this.tags,
diff --git a/packages/core/src/utils/spanUtils.ts b/packages/core/src/utils/spanUtils.ts
@@ -1,11 +1,13 @@
-import type { Span, SpanTimeInput, TraceContext } from '@sentry/types';
+import type { Span, SpanJSON, SpanTimeInput, TraceContext } from '@sentry/types';
 import { dropUndefinedKeys, generateSentryTraceHeader, timestampInSeconds } from '@sentry/utils';
+import type { Span as SpanClass } from '../tracing/span';
 
 /**
  * Convert a span to a trace context, which can be sent as the `trace` context in an event.
  */
 export function spanToTraceContext(span: Span): TraceContext {
-  const { data, description, op, parent_span_id, span_id, status, tags, trace_id, origin } = span.toJSON();
+  const { spanId: span_id, traceId: trace_id } = span;
+  const { data, description, op, parent_span_id, status, tags, origin } = spanToJSON(span);
 
   return dropUndefinedKeys({
     data,
@@ -54,3 +56,35 @@ function ensureTimestampInSeconds(timestamp: number): number {
   const isMs = timestamp > 9999999999;
   return isMs ? timestamp / 1000 : timestamp;
 }
+
+/**
+ * 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 (spanIsSpanClass(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();
+  }
+
+  return {};
+}
+
+/**
+ * Sadly, due to circular dependency checks we cannot actually import the Span class here and check for instanceof.
+ * :( So instead we approximate this by checking if it has the `getSpanJSON` method.
+ */
+function spanIsSpanClass(span: Span): span is SpanClass {
+  return typeof (span as SpanClass).getSpanJSON === 'function';
+}
diff --git a/packages/core/test/lib/utils/spanUtils.test.ts b/packages/core/test/lib/utils/spanUtils.test.ts
@@ -1,6 +1,6 @@
 import { TRACEPARENT_REGEXP, timestampInSeconds } from '@sentry/utils';
 import { Span, spanToTraceHeader } from '../../../src';
-import { spanTimeInputToSeconds } from '../../../src/utils/spanUtils';
+import { spanTimeInputToSeconds, spanToJSON } from '../../../src/utils/spanUtils';
 
 describe('spanToTraceHeader', () => {
   test('simple', () => {
@@ -46,3 +46,72 @@ describe('spanTimeInputToSeconds', () => {
     expect(spanTimeInputToSeconds(timestamp)).toEqual(seconds + 0.000009);
   });
 });
+
+describe('spanToJSON', () => {
+  it('works with a simple span', () => {
+    const span = new Span();
+    expect(spanToJSON(span)).toEqual({
+      span_id: span.spanId,
+      trace_id: span.traceId,
+      origin: 'manual',
+      start_timestamp: span.startTimestamp,
+    });
+  });
+
+  it('works with a full span', () => {
+    const span = new Span({
+      name: 'test name',
+      op: 'test op',
+      parentSpanId: '1234',
+      spanId: '5678',
+      status: 'ok',
+      tags: {
+        foo: 'bar',
+      },
+      traceId: 'abcd',
+      origin: 'auto',
+      startTimestamp: 123,
+    });
+
+    expect(spanToJSON(span)).toEqual({
+      description: 'test name',
+      op: 'test op',
+      parent_span_id: '1234',
+      span_id: '5678',
+      status: 'ok',
+      tags: {
+        foo: 'bar',
+      },
+      trace_id: 'abcd',
+      origin: 'auto',
+      start_timestamp: 123,
+    });
+  });
+
+  it('works with a custom class without spanToJSON', () => {
+    const span = {
+      toJSON: () => {
+        return {
+          span_id: 'span_id',
+          trace_id: 'trace_id',
+          origin: 'manual',
+          start_timestamp: 123,
+        };
+      },
+    } as unknown as Span;
+
+    expect(spanToJSON(span)).toEqual({
+      span_id: 'span_id',
+      trace_id: 'trace_id',
+      origin: 'manual',
+      start_timestamp: 123,
+    });
+  });
+
+  it('returns empty object if span does not have getter methods', () => {
+    // eslint-disable-next-line
+    const span = new Span().toJSON();
+
+    expect(spanToJSON(span as unknown as Span)).toEqual({});
+  });
+});
diff --git a/packages/node-experimental/test/integration/transactions.test.ts b/packages/node-experimental/test/integration/transactions.test.ts
@@ -1,6 +1,7 @@
 import { SpanKind, TraceFlags, context, trace } from '@opentelemetry/api';
 import type { SpanProcessor } from '@opentelemetry/sdk-trace-base';
 import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
+import { spanToJSON } from '@sentry/core';
 import { SentrySpanProcessor, getCurrentHub, setPropagationContextOnContext } from '@sentry/opentelemetry';
 import type { Integration, PropagationContext, TransactionEvent } from '@sentry/types';
 import { logger } from '@sentry/utils';
@@ -145,7 +146,7 @@ describe('Integration | Transactions', () => {
 
     // note: Currently, spans do not have any context/span added to them
     // This is the same behavior as for the "regular" SDKs
-    expect(spans.map(span => span.toJSON())).toEqual([
+    expect(spans.map(span => spanToJSON(span))).toEqual([
       {
         data: { 'otel.kind': 'INTERNAL' },
         description: 'inner span 1',
@@ -399,7 +400,7 @@ describe('Integration | Transactions', () => {
 
     // note: Currently, spans do not have any context/span added to them
     // This is the same behavior as for the "regular" SDKs
-    expect(spans.map(span => span.toJSON())).toEqual([
+    expect(spans.map(span => spanToJSON(span))).toEqual([
       {
         data: { 'otel.kind': 'INTERNAL' },
         description: 'inner span 1',
diff --git a/packages/opentelemetry/test/custom/transaction.test.ts b/packages/opentelemetry/test/custom/transaction.test.ts
@@ -1,3 +1,4 @@
+import { spanToJSON } from '@sentry/core';
 import { getCurrentHub } from '../../src/custom/hub';
 import { OpenTelemetryScope } from '../../src/custom/scope';
 import { OpenTelemetryTransaction, startTransaction } from '../../src/custom/transaction';
@@ -157,7 +158,7 @@ describe('startTranscation', () => {
       spanMetadata: {},
     });
 
-    expect(transaction.toJSON()).toEqual(
+    expect(spanToJSON(transaction)).toEqual(
       expect.objectContaining({
         origin: 'manual',
         span_id: expect.any(String),
@@ -186,7 +187,7 @@ describe('startTranscation', () => {
       spanMetadata: {},
     });
 
-    expect(transaction.toJSON()).toEqual(
+    expect(spanToJSON(transaction)).toEqual(
       expect.objectContaining({
         origin: 'manual',
         span_id: 'span1',
diff --git a/packages/opentelemetry/test/integration/transactions.test.ts b/packages/opentelemetry/test/integration/transactions.test.ts
@@ -4,6 +4,7 @@ import { addBreadcrumb, setTag } from '@sentry/core';
 import type { PropagationContext, TransactionEvent } from '@sentry/types';
 import { logger } from '@sentry/utils';
 
+import { spanToJSON } from '@sentry/core';
 import { getCurrentHub } from '../../src/custom/hub';
 import { SentrySpanProcessor } from '../../src/spanProcessor';
 import { startInactiveSpan, startSpan } from '../../src/trace';
@@ -142,7 +143,7 @@ describe('Integration | Transactions', () => {
 
     // note: Currently, spans do not have any context/span added to them
     // This is the same behavior as for the "regular" SDKs
-    expect(spans.map(span => span.toJSON())).toEqual([
+    expect(spans.map(span => spanToJSON(span))).toEqual([
       {
         data: { 'otel.kind': 'INTERNAL' },
         description: 'inner span 1',
@@ -393,7 +394,7 @@ describe('Integration | Transactions', () => {
 
     // note: Currently, spans do not have any context/span added to them
     // This is the same behavior as for the "regular" SDKs
-    expect(spans.map(span => span.toJSON())).toEqual([
+    expect(spans.map(span => spanToJSON(span))).toEqual([
       {
         data: { 'otel.kind': 'INTERNAL' },
         description: 'inner span 1',
diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts
@@ -89,7 +89,15 @@ export type {
 
 // eslint-disable-next-line deprecation/deprecation
 export type { Severity, SeverityLevel } from './severity';
-export type { Span, SpanContext, SpanOrigin, SpanAttributeValue, SpanAttributes, SpanTimeInput } from './span';
+export type {
+  Span,
+  SpanContext,
+  SpanOrigin,
+  SpanAttributeValue,
+  SpanAttributes,
+  SpanTimeInput,
+  SpanJSON,
+} from './span';
 export type { StackFrame } from './stackframe';
 export type { Stacktrace, StackParser, StackLineParser, StackLineParserFn } from './stacktrace';
 export type { TextEncoderInternal } from './textencoder';
diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts
@@ -28,6 +28,21 @@ export type SpanAttributes = Record<string, SpanAttributeValue | undefined>;
 /** This type is aligned with the OpenTelemetry TimeInput type. */
 export type SpanTimeInput = HrTime | number | Date;
 
+/** A JSON representation of a span. */
+export interface SpanJSON {
+  data?: { [key: string]: any };
+  description?: string;
+  op?: string;
+  parent_span_id?: string;
+  span_id: string;
+  start_timestamp: number;
+  status?: string;
+  tags?: { [key: string]: Primitive };
+  timestamp?: number;
+  trace_id: string;
+  origin?: SpanOrigin;
+}
+
 /** Interface holding all properties that can be set on a Span on creation. */
 export interface SpanContext {
   /**
@@ -256,20 +271,11 @@ export interface Span extends SpanContext {
    */
   getTraceContext(): TraceContext;
 
-  /** Convert the object to JSON */
-  toJSON(): {
-    data?: { [key: string]: any };
-    description?: string;
-    op?: string;
-    parent_span_id?: string;
-    span_id: string;
-    start_timestamp: number;
-    status?: string;
-    tags?: { [key: string]: Primitive };
-    timestamp?: number;
-    trace_id: string;
-    origin?: SpanOrigin;
-  };
+  /**
+   * Convert the object to JSON.
+   * @deprecated Use `spanToJSON(span)` instead.
+   */
+  toJSON(): SpanJSON;
 
   /**
    * If this is span is actually recording data.
