From 37fd60626822e52117e908d8425d4f7fa5b0f8ed Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 5 Mar 2024 10:15:51 +0000 Subject: [PATCH] feat(core): Update `spanToJSON` to handle OTEL spans --- packages/angular/test/tracing.test.ts | 8 +- packages/core/src/tracing/sentrySpan.ts | 14 +- packages/core/src/tracing/transaction.ts | 14 +- packages/core/src/utils/spanUtils.ts | 76 ++++++- .../core/test/lib/utils/spanUtils.test.ts | 202 ++++++++++++------ .../opentelemetry/test/helpers/createSpan.ts | 18 +- .../test/utils/spanToJSON.test.ts | 57 +++++ packages/types/src/event.ts | 2 +- packages/vue/test/router.test.ts | 2 +- 9 files changed, 299 insertions(+), 94 deletions(-) create mode 100644 packages/opentelemetry/test/utils/spanToJSON.test.ts diff --git a/packages/angular/test/tracing.test.ts b/packages/angular/test/tracing.test.ts index 5a935f178c71..f48de0043fc6 100644 --- a/packages/angular/test/tracing.test.ts +++ b/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(); }); diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts index b0e82321e33e..5a31f511cc3e 100644 --- a/packages/core/src/tracing/sentrySpan.ts +++ b/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'; -} diff --git a/packages/core/src/tracing/transaction.ts b/packages/core/src/tracing/transaction.ts index 9f4194681338..bb187f95f502 100644 --- a/packages/core/src/tracing/transaction.ts +++ b/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): 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, diff --git a/packages/core/src/utils/spanUtils.ts b/packages/core/src/utils/spanUtils.ts index 436d383b8a7c..8560aece5ca4 100644 --- a/packages/core/src/utils/spanUtils.ts +++ b/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 { 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'; +} diff --git a/packages/core/test/lib/utils/spanUtils.test.ts b/packages/core/test/lib/utils/spanUtils.test.ts index 56b07405b02c..4faf18686e49 100644 --- a/packages/core/test/lib/utils/spanUtils.test.ts +++ b/packages/core/test/lib/utils/spanUtils.test.ts @@ -1,5 +1,15 @@ +import type { Span, SpanAttributes, SpanStatus, SpanTimeInput } from '@sentry/types'; import { TRACEPARENT_REGEXP, timestampInSeconds } from '@sentry/utils'; -import { SPAN_STATUS_OK, SentrySpan, spanToTraceHeader } from '../../../src'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SPAN_STATUS_ERROR, + SPAN_STATUS_OK, + SPAN_STATUS_UNSET, + SentrySpan, + spanToTraceHeader, +} from '../../../src'; +import type { OpenTelemetrySdkTraceBaseSpan } from '../../../src/utils/spanUtils'; import { spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../../../src/utils/spanUtils'; describe('spanToTraceHeader', () => { @@ -48,74 +58,146 @@ describe('spanTimeInputToSeconds', () => { }); describe('spanToJSON', () => { - it('works with a simple span', () => { - const span = new SentrySpan(); - expect(spanToJSON(span)).toEqual({ - span_id: span.spanContext().spanId, - trace_id: span.spanContext().traceId, - origin: 'manual', - start_timestamp: span['_startTime'], - data: { - 'sentry.origin': 'manual', - }, + describe('SentrySpan', () => { + it('works with a simple span', () => { + const span = new SentrySpan(); + expect(spanToJSON(span)).toEqual({ + span_id: span.spanContext().spanId, + trace_id: span.spanContext().traceId, + origin: 'manual', + start_timestamp: span['_startTime'], + data: { + 'sentry.origin': 'manual', + }, + }); }); - }); - it('works with a full span', () => { - const span = new SentrySpan({ - name: 'test name', - op: 'test op', - parentSpanId: '1234', - spanId: '5678', - traceId: 'abcd', - origin: 'auto', - startTimestamp: 123, - endTimestamp: 456, - }); - span.setStatus({ code: SPAN_STATUS_OK }); - - expect(spanToJSON(span)).toEqual({ - description: 'test name', - op: 'test op', - parent_span_id: '1234', - span_id: '5678', - status: 'ok', - trace_id: 'abcd', - origin: 'auto', - start_timestamp: 123, - timestamp: 456, - data: { - 'sentry.op': 'test op', - 'sentry.origin': 'auto', - }, + it('works with a full span', () => { + const span = new SentrySpan({ + name: 'test name', + op: 'test op', + parentSpanId: '1234', + spanId: '5678', + traceId: 'abcd', + origin: 'auto', + startTimestamp: 123, + endTimestamp: 456, + }); + span.setStatus({ code: SPAN_STATUS_OK }); + + expect(spanToJSON(span)).toEqual({ + description: 'test name', + op: 'test op', + parent_span_id: '1234', + span_id: '5678', + status: 'ok', + trace_id: 'abcd', + origin: 'auto', + start_timestamp: 123, + timestamp: 456, + data: { + 'sentry.op': 'test op', + 'sentry.origin': 'auto', + }, + }); }); }); - 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 SentrySpan; - - expect(spanToJSON(span)).toEqual({ - span_id: 'span_id', - trace_id: 'trace_id', - origin: 'manual', - start_timestamp: 123, + describe('OpenTelemetry Span', () => { + function createMockedOtelSpan({ + spanId, + traceId, + attributes, + startTime, + name, + status, + endTime, + parentSpanId, + }: { + spanId: string; + traceId: string; + attributes: SpanAttributes; + startTime: SpanTimeInput; + name: string; + status: SpanStatus; + endTime: SpanTimeInput; + parentSpanId?: string; + }): Span { + return { + spanContext: () => { + return { + spanId, + traceId, + }; + }, + attributes, + startTime, + name, + status, + endTime, + parentSpanId, + } as OpenTelemetrySdkTraceBaseSpan; + } + + it('works with a simple span', () => { + const span = createMockedOtelSpan({ + spanId: 'SPAN-1', + traceId: 'TRACE-1', + name: 'test span', + startTime: 123, + endTime: [0, 0], + attributes: {}, + status: { code: SPAN_STATUS_UNSET }, + }); + + expect(spanToJSON(span)).toEqual({ + span_id: 'SPAN-1', + trace_id: 'TRACE-1', + start_timestamp: 123, + description: 'test span', + data: {}, + }); + }); + + it('works with a full span', () => { + const span = createMockedOtelSpan({ + spanId: 'SPAN-1', + traceId: 'TRACE-1', + name: 'test span', + startTime: 123, + endTime: 456, + attributes: { + attr1: 'value1', + attr2: 2, + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'test op', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto', + }, + status: { code: SPAN_STATUS_ERROR, message: 'unknown_error' }, + }); + + expect(spanToJSON(span)).toEqual({ + span_id: 'SPAN-1', + trace_id: 'TRACE-1', + start_timestamp: 123, + timestamp: 456, + description: 'test span', + op: 'test op', + origin: 'auto', + data: { + attr1: 'value1', + attr2: 2, + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'test op', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto', + }, + status: 'unknown_error', + }); }); }); - it('returns empty object if span does not have getter methods', () => { - // eslint-disable-next-line - const span = new SentrySpan().toJSON(); + it('returns empty object for unknown span implementation', () => { + const span = { other: 'other' }; - expect(spanToJSON(span as unknown as SentrySpan)).toEqual({}); + expect(spanToJSON(span as unknown as Span)).toEqual({}); }); }); diff --git a/packages/opentelemetry/test/helpers/createSpan.ts b/packages/opentelemetry/test/helpers/createSpan.ts index 38c4ed96f3a8..e68a82986dea 100644 --- a/packages/opentelemetry/test/helpers/createSpan.ts +++ b/packages/opentelemetry/test/helpers/createSpan.ts @@ -1,4 +1,4 @@ -import type { Context, SpanContext } from '@opentelemetry/api'; +import type { Context, SpanContext, TimeInput } from '@opentelemetry/api'; import { SpanKind } from '@opentelemetry/api'; import type { Tracer } from '@opentelemetry/sdk-trace-base'; import { Span } from '@opentelemetry/sdk-trace-base'; @@ -6,7 +6,17 @@ import { uuid4 } from '@sentry/utils'; export function createSpan( name?: string, - { spanId, parentSpanId }: { spanId?: string; parentSpanId?: string } = {}, + { + spanId, + parentSpanId, + traceId, + startTime, + }: { + spanId?: string; + parentSpanId?: string; + traceId?: string; + startTime?: TimeInput; + } = {}, ): Span { const spanProcessor = { onStart: () => {}, @@ -21,10 +31,10 @@ export function createSpan( const spanContext: SpanContext = { spanId: spanId || uuid4(), - traceId: uuid4(), + traceId: traceId || uuid4(), traceFlags: 0, }; // eslint-disable-next-line deprecation/deprecation - return new Span(tracer, {} as Context, name || 'test', spanContext, SpanKind.INTERNAL, parentSpanId); + return new Span(tracer, {} as Context, name || 'test', spanContext, SpanKind.INTERNAL, parentSpanId, [], startTime); } diff --git a/packages/opentelemetry/test/utils/spanToJSON.test.ts b/packages/opentelemetry/test/utils/spanToJSON.test.ts new file mode 100644 index 000000000000..103ede3eaafd --- /dev/null +++ b/packages/opentelemetry/test/utils/spanToJSON.test.ts @@ -0,0 +1,57 @@ +import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, spanToJSON } from '@sentry/core'; +import { createSpan } from '../helpers/createSpan'; + +describe('spanToJSON', () => { + describe('OpenTelemetry Span', () => { + it('works with a simple span', () => { + const span = createSpan('test span', { + spanId: 'SPAN-1', + traceId: 'TRACE-1', + startTime: [123, 0], + }); + + expect(spanToJSON(span)).toEqual({ + span_id: 'SPAN-1', + trace_id: 'TRACE-1', + start_timestamp: 123, + description: 'test span', + data: {}, + }); + }); + + it('works with a full span', () => { + const span = createSpan('test span', { + spanId: 'SPAN-1', + traceId: 'TRACE-1', + startTime: [123, 0], + }); + + span.setAttributes({ + attr1: 'value1', + attr2: 2, + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'test op', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto', + }); + + span.setStatus({ code: 2, message: 'unknown_error' }); + span.end([456, 0]); + + expect(spanToJSON(span)).toEqual({ + span_id: 'SPAN-1', + trace_id: 'TRACE-1', + start_timestamp: 123, + timestamp: 456, + description: 'test span', + op: 'test op', + origin: 'auto', + data: { + attr1: 'value1', + attr2: 2, + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'test op', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto', + }, + status: 'unknown_error', + }); + }); + }); +}); diff --git a/packages/types/src/event.ts b/packages/types/src/event.ts index 37146c674f52..da207156b8c8 100644 --- a/packages/types/src/event.ts +++ b/packages/types/src/event.ts @@ -47,7 +47,7 @@ export interface Event { extra?: Extras; user?: User; type?: EventType; - spans?: Partial[]; + spans?: SpanJSON[]; measurements?: Measurements; debug_meta?: DebugMeta; // A place to stash data which is needed at some point in the SDK's event processing pipeline but which shouldn't get sent to Sentry diff --git a/packages/vue/test/router.test.ts b/packages/vue/test/router.test.ts index c3a786a8f887..1a27e84961b1 100644 --- a/packages/vue/test/router.test.ts +++ b/packages/vue/test/router.test.ts @@ -288,7 +288,7 @@ describe('instrumentVueRouter()', () => { setAttribute: jest.fn(), setAttributes: jest.fn(), name: '', - toJSON: () => ({ + getSpanJSON: () => ({ op: 'pageload', data: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',