From 7eee58bbc6d147510386c1e0ee2933af16c59fd8 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 2 Jan 2024 09:55:55 +0100 Subject: [PATCH 1/2] feat(node-experimental): Allow to pass base span options to trace methods --- packages/core/src/tracing/trace.ts | 2 +- packages/node-experimental/src/index.ts | 2 +- packages/opentelemetry/src/index.ts | 2 +- packages/opentelemetry/src/trace.ts | 44 ++++++- packages/opentelemetry/src/types.ts | 4 +- packages/opentelemetry/test/trace.test.ts | 154 +++++++++++++++++++++- 6 files changed, 199 insertions(+), 9 deletions(-) diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 1b01ee20490c..ceb59e1434dd 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -138,7 +138,7 @@ export const startActiveSpan = startSpan; /** * Similar to `Sentry.startSpan`. Wraps a function with a transaction/span, but does not finish the span - * after the function is done automatically. + * after the function is done automatically. You'll have to call `span.end()` manually. * * The created span is the active span and will be used as parent by other spans created inside the function * and can be accessed via `Sentry.getActiveSpan()`, as long as the function is executed while the scope is active. diff --git a/packages/node-experimental/src/index.ts b/packages/node-experimental/src/index.ts index e45616b2e65a..c6771601c16f 100644 --- a/packages/node-experimental/src/index.ts +++ b/packages/node-experimental/src/index.ts @@ -13,7 +13,7 @@ export { getAutoPerformanceIntegrations } from './integrations/getAutoPerformanc export * as Handlers from './sdk/handlers'; export type { Span } from './types'; -export { startSpan, startInactiveSpan, getActiveSpan } from '@sentry/opentelemetry'; +export { startSpan, startSpanManual, startInactiveSpan, getActiveSpan } from '@sentry/opentelemetry'; export { getClient, addBreadcrumb, diff --git a/packages/opentelemetry/src/index.ts b/packages/opentelemetry/src/index.ts index f379b4216da5..534c99c95fb4 100644 --- a/packages/opentelemetry/src/index.ts +++ b/packages/opentelemetry/src/index.ts @@ -29,7 +29,7 @@ export { export { isSentryRequestSpan } from './utils/isSentryRequest'; export { getActiveSpan, getRootSpan } from './utils/getActiveSpan'; -export { startSpan, startInactiveSpan } from './trace'; +export { startSpan, startSpanManual, startInactiveSpan } from './trace'; export { getCurrentHub, setupGlobalHub, getClient } from './custom/hub'; export { OpenTelemetryScope } from './custom/scope'; diff --git a/packages/opentelemetry/src/trace.ts b/packages/opentelemetry/src/trace.ts index 3bd635a953df..b5b1e48a8e4b 100644 --- a/packages/opentelemetry/src/trace.ts +++ b/packages/opentelemetry/src/trace.ts @@ -23,7 +23,7 @@ export function startSpan(spanContext: OpenTelemetrySpanContext, callback: (s const { name } = spanContext; - return tracer.startActiveSpan(name, span => { + return tracer.startActiveSpan(name, spanContext, span => { function finishSpan(): void { span.end(); } @@ -57,6 +57,46 @@ export function startSpan(spanContext: OpenTelemetrySpanContext, callback: (s }); } +/** + * Similar to `Sentry.startSpan`. Wraps a function with a span, but does not finish the span + * after the function is done automatically. You'll have to call `span.end()` manually. + * + * The created span is the active span and will be used as parent by other spans created inside the function + * and can be accessed via `Sentry.getActiveSpan()`, as long as the function is executed while the scope is active. + * + * Note that you'll always get a span passed to the callback, it may just be a NonRecordingSpan if the span is not sampled. + */ +export function startSpanManual(spanContext: OpenTelemetrySpanContext, callback: (span: Span) => T): T { + const tracer = getTracer(); + + const { name } = spanContext; + + // @ts-expect-error - isThenable returns the wrong type + return tracer.startActiveSpan(name, spanContext, span => { + _applySentryAttributesToSpan(span, spanContext); + + let maybePromiseResult: T; + try { + maybePromiseResult = callback(span); + } catch (e) { + span.setStatus({ code: SpanStatusCode.ERROR }); + throw e; + } + + if (isThenable(maybePromiseResult)) { + return maybePromiseResult.then( + res => res, + e => { + span.setStatus({ code: SpanStatusCode.ERROR }); + throw e; + }, + ); + } + + return maybePromiseResult; + }); +} + /** * @deprecated Use {@link startSpan} instead. */ @@ -77,7 +117,7 @@ export function startInactiveSpan(spanContext: OpenTelemetrySpanContext): Span { const { name } = spanContext; - const span = tracer.startSpan(name); + const span = tracer.startSpan(name, spanContext); _applySentryAttributesToSpan(span, spanContext); diff --git a/packages/opentelemetry/src/types.ts b/packages/opentelemetry/src/types.ts index 0cb5342a3ac8..4b2ae42190a4 100644 --- a/packages/opentelemetry/src/types.ts +++ b/packages/opentelemetry/src/types.ts @@ -1,4 +1,4 @@ -import type { Span as WriteableSpan, Tracer } from '@opentelemetry/api'; +import type { Span as WriteableSpan, SpanOptions, Tracer } from '@opentelemetry/api'; import type { BasicTracerProvider, ReadableSpan, Span } from '@opentelemetry/sdk-trace-base'; import type { SpanOrigin, TransactionMetadata, TransactionSource } from '@sentry/types'; @@ -7,7 +7,7 @@ export interface OpenTelemetryClient { traceProvider: BasicTracerProvider | undefined; } -export interface OpenTelemetrySpanContext { +export interface OpenTelemetrySpanContext extends SpanOptions { name: string; op?: string; metadata?: Partial; diff --git a/packages/opentelemetry/test/trace.test.ts b/packages/opentelemetry/test/trace.test.ts index 4075eef1e4de..2e9ee20000c6 100644 --- a/packages/opentelemetry/test/trace.test.ts +++ b/packages/opentelemetry/test/trace.test.ts @@ -1,14 +1,16 @@ -import type { Span } from '@opentelemetry/api'; +import type { Link, Span } from '@opentelemetry/api'; +import { SpanKind } from '@opentelemetry/api'; import { TraceFlags, context, trace } from '@opentelemetry/api'; import type { ReadableSpan } from '@opentelemetry/sdk-trace-base'; import type { PropagationContext } from '@sentry/types'; import { getCurrentHub } from '../src/custom/hub'; import { InternalSentrySemanticAttributes } from '../src/semanticAttributes'; -import { startInactiveSpan, startSpan } from '../src/trace'; +import { startInactiveSpan, startSpan, startSpanManual } from '../src/trace'; import type { AbstractSpan } from '../src/types'; import { setPropagationContextOnContext } from '../src/utils/contextData'; import { getActiveSpan, getRootSpan } from '../src/utils/getActiveSpan'; +import { getSpanKind } from '../src/utils/getSpanKind'; import { getSpanMetadata } from '../src/utils/spanData'; import { spanHasAttributes, spanHasName } from '../src/utils/spanTypes'; import { cleanupOtel, mockSdkInit } from './helpers/mockSdkInit'; @@ -231,6 +233,42 @@ describe('trace', () => { }, ); }); + + it('allows to pass base SpanOptions', () => { + const date = Date.now() - 1000; + const link: Link = { + context: { + spanId: 'test-span-id', + traceId: 'test-trace-id', + traceFlags: TraceFlags.SAMPLED, + }, + attributes: {}, + }; + + startSpan( + { + name: 'outer', + kind: SpanKind.CLIENT, + attributes: { + test1: 'test 1', + test2: 2, + }, + links: [link], + startTime: date, + }, + span => { + expect(span).toBeDefined(); + expect(getSpanName(span)).toEqual('outer'); + expect(getSpanAttributes(span)).toEqual({ + [InternalSentrySemanticAttributes.SAMPLE_RATE]: 1, + test1: 'test 1', + test2: 2, + }); + expect(getSpanKind(span)).toEqual(SpanKind.CLIENT); + expect((span as unknown as ReadableSpan).links).toEqual([link]); + }, + ); + }); }); describe('startInactiveSpan', () => { @@ -297,6 +335,118 @@ describe('trace', () => { expect(getSpanMetadata(span2)).toEqual({ requestPath: 'test-path' }); }); + + it('allows to pass base SpanOptions', () => { + const date = Date.now() - 1000; + const link: Link = { + context: { + spanId: 'test-span-id', + traceId: 'test-trace-id', + traceFlags: TraceFlags.SAMPLED, + }, + attributes: {}, + }; + + const span = startInactiveSpan({ + name: 'outer', + kind: SpanKind.CLIENT, + attributes: { + test1: 'test 1', + test2: 2, + }, + links: [link], + startTime: date, + }); + + expect(span).toBeDefined(); + expect(getSpanName(span)).toEqual('outer'); + expect(getSpanAttributes(span)).toEqual({ + [InternalSentrySemanticAttributes.SAMPLE_RATE]: 1, + test1: 'test 1', + test2: 2, + }); + expect(getSpanKind(span)).toEqual(SpanKind.CLIENT); + expect((span as unknown as ReadableSpan).links).toEqual([link]); + }); + }); + + describe('startSpanManual', () => { + it('does not automatically finish the span', () => { + expect(getActiveSpan()).toEqual(undefined); + + let _outerSpan: Span | undefined; + let _innerSpan: Span | undefined; + + const res = startSpanManual({ name: 'outer' }, outerSpan => { + expect(outerSpan).toBeDefined(); + _outerSpan = outerSpan; + + expect(getSpanName(outerSpan)).toEqual('outer'); + expect(getActiveSpan()).toEqual(outerSpan); + + startSpanManual({ name: 'inner' }, innerSpan => { + expect(innerSpan).toBeDefined(); + _innerSpan = innerSpan; + + expect(getSpanName(innerSpan)).toEqual('inner'); + expect(getActiveSpan()).toEqual(innerSpan); + }); + + expect(getSpanEndTime(_innerSpan!)).toEqual([0, 0]); + + _innerSpan!.end(); + + expect(getSpanEndTime(_innerSpan!)).not.toEqual([0, 0]); + + return 'test value'; + }); + + expect(getSpanEndTime(_outerSpan!)).toEqual([0, 0]); + + _outerSpan!.end(); + + expect(getSpanEndTime(_outerSpan!)).not.toEqual([0, 0]); + + expect(res).toEqual('test value'); + + expect(getActiveSpan()).toEqual(undefined); + }); + + it('allows to pass base SpanOptions', () => { + const date = Date.now() - 1000; + const link: Link = { + context: { + spanId: 'test-span-id', + traceId: 'test-trace-id', + traceFlags: TraceFlags.SAMPLED, + }, + attributes: {}, + }; + + startSpanManual( + { + name: 'outer', + kind: SpanKind.CLIENT, + attributes: { + test1: 'test 1', + test2: 2, + }, + links: [link], + startTime: date, + }, + span => { + expect(span).toBeDefined(); + expect(getSpanName(span)).toEqual('outer'); + expect(getSpanAttributes(span)).toEqual({ + [InternalSentrySemanticAttributes.SAMPLE_RATE]: 1, + test1: 'test 1', + test2: 2, + }); + expect(getSpanKind(span)).toEqual(SpanKind.CLIENT); + expect((span as unknown as ReadableSpan).links).toEqual([link]); + }, + ); + }); }); }); From cf39eb52de59e94344910c2f4c093e7a9601e9c6 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 2 Jan 2024 09:59:50 +0100 Subject: [PATCH 2/2] explicitly specifiy what we support --- packages/opentelemetry/src/types.ts | 9 +++++-- packages/opentelemetry/test/trace.test.ts | 33 ++--------------------- 2 files changed, 9 insertions(+), 33 deletions(-) diff --git a/packages/opentelemetry/src/types.ts b/packages/opentelemetry/src/types.ts index 4b2ae42190a4..fdab000a6e09 100644 --- a/packages/opentelemetry/src/types.ts +++ b/packages/opentelemetry/src/types.ts @@ -1,4 +1,4 @@ -import type { Span as WriteableSpan, SpanOptions, Tracer } from '@opentelemetry/api'; +import type { Attributes, Span as WriteableSpan, SpanKind, TimeInput, Tracer } from '@opentelemetry/api'; import type { BasicTracerProvider, ReadableSpan, Span } from '@opentelemetry/sdk-trace-base'; import type { SpanOrigin, TransactionMetadata, TransactionSource } from '@sentry/types'; @@ -7,12 +7,17 @@ export interface OpenTelemetryClient { traceProvider: BasicTracerProvider | undefined; } -export interface OpenTelemetrySpanContext extends SpanOptions { +export interface OpenTelemetrySpanContext { name: string; op?: string; metadata?: Partial; origin?: SpanOrigin; source?: TransactionSource; + + // Base SpanOptions we support + attributes?: Attributes; + kind?: SpanKind; + startTime?: TimeInput; } /** diff --git a/packages/opentelemetry/test/trace.test.ts b/packages/opentelemetry/test/trace.test.ts index 2e9ee20000c6..54a1082830ae 100644 --- a/packages/opentelemetry/test/trace.test.ts +++ b/packages/opentelemetry/test/trace.test.ts @@ -1,4 +1,4 @@ -import type { Link, Span } from '@opentelemetry/api'; +import type { Span } from '@opentelemetry/api'; import { SpanKind } from '@opentelemetry/api'; import { TraceFlags, context, trace } from '@opentelemetry/api'; import type { ReadableSpan } from '@opentelemetry/sdk-trace-base'; @@ -236,14 +236,6 @@ describe('trace', () => { it('allows to pass base SpanOptions', () => { const date = Date.now() - 1000; - const link: Link = { - context: { - spanId: 'test-span-id', - traceId: 'test-trace-id', - traceFlags: TraceFlags.SAMPLED, - }, - attributes: {}, - }; startSpan( { @@ -253,7 +245,7 @@ describe('trace', () => { test1: 'test 1', test2: 2, }, - links: [link], + startTime: date, }, span => { @@ -265,7 +257,6 @@ describe('trace', () => { test2: 2, }); expect(getSpanKind(span)).toEqual(SpanKind.CLIENT); - expect((span as unknown as ReadableSpan).links).toEqual([link]); }, ); }); @@ -338,14 +329,6 @@ describe('trace', () => { it('allows to pass base SpanOptions', () => { const date = Date.now() - 1000; - const link: Link = { - context: { - spanId: 'test-span-id', - traceId: 'test-trace-id', - traceFlags: TraceFlags.SAMPLED, - }, - attributes: {}, - }; const span = startInactiveSpan({ name: 'outer', @@ -354,7 +337,6 @@ describe('trace', () => { test1: 'test 1', test2: 2, }, - links: [link], startTime: date, }); @@ -366,7 +348,6 @@ describe('trace', () => { test2: 2, }); expect(getSpanKind(span)).toEqual(SpanKind.CLIENT); - expect((span as unknown as ReadableSpan).links).toEqual([link]); }); }); @@ -414,14 +395,6 @@ describe('trace', () => { it('allows to pass base SpanOptions', () => { const date = Date.now() - 1000; - const link: Link = { - context: { - spanId: 'test-span-id', - traceId: 'test-trace-id', - traceFlags: TraceFlags.SAMPLED, - }, - attributes: {}, - }; startSpanManual( { @@ -431,7 +404,6 @@ describe('trace', () => { test1: 'test 1', test2: 2, }, - links: [link], startTime: date, }, span => { @@ -443,7 +415,6 @@ describe('trace', () => { test2: 2, }); expect(getSpanKind(span)).toEqual(SpanKind.CLIENT); - expect((span as unknown as ReadableSpan).links).toEqual([link]); }, ); });