diff --git a/packages/bun/src/integrations/bunserver.ts b/packages/bun/src/integrations/bunserver.ts index f5b6a52d3dd0..8ab9a73194da 100644 --- a/packages/bun/src/integrations/bunserver.ts +++ b/packages/bun/src/integrations/bunserver.ts @@ -96,9 +96,7 @@ function instrumentBunServeOptions(serveOptions: Parameters[0] typeof serveOptions.fetch >); if (response && response.status) { - if (span) { - setHttpStatus(span, response.status); - } + setHttpStatus(span, response.status); if (span instanceof Transaction) { const scope = getCurrentScope(); scope.setContext('response', { diff --git a/packages/core/src/fetch.ts b/packages/core/src/fetch.ts index 05e9324c8b9f..5255e7fa206d 100644 --- a/packages/core/src/fetch.ts +++ b/packages/core/src/fetch.ts @@ -14,6 +14,7 @@ import { setHttpStatus, startInactiveSpan, } from './tracing'; +import { SentryNonRecordingSpan } from './tracing/sentryNonRecordingSpan'; import { hasTracingEnabled } from './utils/hasTracingEnabled'; import { spanToTraceHeader } from './utils/spanUtils'; @@ -92,12 +93,10 @@ export function instrumentFetchRequest( }, op: 'http.client', }) - : undefined; + : new SentryNonRecordingSpan(); - if (span) { - handlerData.fetchData.__span = span.spanContext().spanId; - spans[span.spanContext().spanId] = span; - } + handlerData.fetchData.__span = span.spanContext().spanId; + spans[span.spanContext().spanId] = span; if (shouldAttachHeaders(handlerData.fetchData.url) && client) { const request: string | Request = handlerData.args[0]; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index c2671aa07ca0..eddc533f68b5 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -90,6 +90,7 @@ export { spanToJSON, spanIsSampled, spanToTraceContext, + getSpanDescendants, } from './utils/spanUtils'; export { getRootSpan } from './utils/getRootSpan'; export { applySdkMetadata } from './utils/sdkMetadata'; diff --git a/packages/core/src/tracing/idleSpan.ts b/packages/core/src/tracing/idleSpan.ts index 425d5b1aa39e..a72a7f8c596f 100644 --- a/packages/core/src/tracing/idleSpan.ts +++ b/packages/core/src/tracing/idleSpan.ts @@ -3,10 +3,12 @@ import { logger, timestampInSeconds } from '@sentry/utils'; import { getClient, getCurrentScope } from '../currentScopes'; import { DEBUG_BUILD } from '../debug-build'; -import { spanToJSON } from '../utils/spanUtils'; +import { hasTracingEnabled } from '../utils/hasTracingEnabled'; +import { getSpanDescendants, removeChildSpanFromSpan, spanToJSON } from '../utils/spanUtils'; +import { SentryNonRecordingSpan } from './sentryNonRecordingSpan'; import { SPAN_STATUS_ERROR } from './spanstatus'; import { startInactiveSpan } from './trace'; -import { getActiveSpan, getSpanDescendants, removeChildSpanFromSpan } from './utils'; +import { getActiveSpan } from './utils'; export const TRACING_DEFAULTS = { idleTimeout: 1_000, @@ -71,10 +73,7 @@ interface IdleSpanOptions { * An idle span is a span that automatically finishes. It does this by tracking child spans as activities. * An idle span is always the active span. */ -export function startIdleSpan( - startSpanOptions: StartSpanOptions, - options: Partial = {}, -): Span | undefined { +export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Partial = {}): Span { // Activities store a list of active spans const activities = new Map(); @@ -101,21 +100,13 @@ export function startIdleSpan( const client = getClient(); - if (!client) { - return; + if (!client || !hasTracingEnabled()) { + return new SentryNonRecordingSpan(); } const scope = getCurrentScope(); const previousActiveSpan = getActiveSpan(); - const _span = _startIdleSpan(startSpanOptions); - - // Span _should_ always be defined here, but TS does not know that... - if (!_span) { - return; - } - - // For TS, so that we know everything below here has a span - const span = _span; + const span = _startIdleSpan(startSpanOptions); /** * Cancels the existing idle timeout, if there is one. @@ -319,15 +310,13 @@ export function startIdleSpan( return span; } -function _startIdleSpan(options: StartSpanOptions): Span | undefined { +function _startIdleSpan(options: StartSpanOptions): Span { const span = startInactiveSpan(options); // eslint-disable-next-line deprecation/deprecation getCurrentScope().setSpan(span); - if (span) { - DEBUG_BUILD && logger.log(`Setting idle span on scope. Span ID: ${span.spanContext().spanId}`); - } + DEBUG_BUILD && logger.log(`Setting idle span on scope. Span ID: ${span.spanContext().spanId}`); return span; } diff --git a/packages/core/src/tracing/index.ts b/packages/core/src/tracing/index.ts index 8054e0d06aaa..59ad557cba90 100644 --- a/packages/core/src/tracing/index.ts +++ b/packages/core/src/tracing/index.ts @@ -2,9 +2,10 @@ export { startIdleTransaction, addTracingExtensions } from './hubextensions'; export { IdleTransaction, TRACING_DEFAULTS } from './idletransaction'; export type { BeforeFinishCallback } from './idletransaction'; export { SentrySpan } from './sentrySpan'; +export { SentryNonRecordingSpan } from './sentryNonRecordingSpan'; export { Transaction } from './transaction'; // eslint-disable-next-line deprecation/deprecation -export { getActiveTransaction, getActiveSpan, getSpanDescendants } from './utils'; +export { getActiveTransaction, getActiveSpan } from './utils'; export { setHttpStatus, getSpanStatusFromHttpCode, diff --git a/packages/core/src/tracing/sentryNonRecordingSpan.ts b/packages/core/src/tracing/sentryNonRecordingSpan.ts new file mode 100644 index 000000000000..6b86fe4a0fec --- /dev/null +++ b/packages/core/src/tracing/sentryNonRecordingSpan.ts @@ -0,0 +1,62 @@ +import type { + Span, + SpanAttributeValue, + SpanAttributes, + SpanContext, + SpanContextData, + SpanStatus, + SpanTimeInput, +} from '@sentry/types'; +import { uuid4 } from '@sentry/utils'; +import { TRACE_FLAG_NONE } from '../utils/spanUtils'; + +/** + * A Sentry Span that is non-recording, meaning it will not be sent to Sentry. + */ +export class SentryNonRecordingSpan implements Span { + private _traceId: string; + private _spanId: string; + + public constructor(spanContext: SpanContext = {}) { + this._traceId = spanContext.traceId || uuid4(); + this._spanId = spanContext.spanId || uuid4().substring(16); + } + + /** @inheritdoc */ + public spanContext(): SpanContextData { + return { + spanId: this._spanId, + traceId: this._traceId, + traceFlags: TRACE_FLAG_NONE, + }; + } + + /** @inheritdoc */ + // eslint-disable-next-line @typescript-eslint/no-empty-function + public end(_timestamp?: SpanTimeInput): void {} + + /** @inheritdoc */ + public setAttribute(_key: string, _value: SpanAttributeValue | undefined): this { + return this; + } + + /** @inheritdoc */ + public setAttributes(_values: SpanAttributes): this { + return this; + } + + /** @inheritdoc */ + public setStatus(_status: SpanStatus): this { + return this; + } + + /** @inheritdoc */ + public updateName(_name: string): this { + return this; + } + + /** @inheritdoc */ + public isRecording(): boolean { + return false; + } +} diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts index 36a7acc403b0..305f24b921b1 100644 --- a/packages/core/src/tracing/sentrySpan.ts +++ b/packages/core/src/tracing/sentrySpan.ts @@ -22,12 +22,12 @@ import { getRootSpan } from '../utils/getRootSpan'; import { TRACE_FLAG_NONE, TRACE_FLAG_SAMPLED, + addChildSpanToSpan, getStatusMessage, spanTimeInputToSeconds, spanToJSON, spanToTraceContext, } from '../utils/spanUtils'; -import { addChildSpanToSpan } from './utils'; /** * Keeps track of finished spans for a given transaction diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 98cd511493c6..9e0481eb807f 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -7,11 +7,12 @@ import { DEBUG_BUILD } from '../debug-build'; import { getCurrentHub } from '../hub'; import { handleCallbackErrors } from '../utils/handleCallbackErrors'; import { hasTracingEnabled } from '../utils/hasTracingEnabled'; -import { spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils'; +import { addChildSpanToSpan, spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils'; import { getDynamicSamplingContextFromSpan } from './dynamicSamplingContext'; +import { SentryNonRecordingSpan } from './sentryNonRecordingSpan'; import type { SentrySpan } from './sentrySpan'; import { SPAN_STATUS_ERROR } from './spanstatus'; -import { addChildSpanToSpan, getActiveSpan, setCapturedScopesOnSpan } from './utils'; +import { getActiveSpan, setCapturedScopesOnSpan } from './utils'; /** * Wraps a function with a transaction/span and finishes the span after the function is done. @@ -24,7 +25,7 @@ import { addChildSpanToSpan, getActiveSpan, setCapturedScopesOnSpan } from './ut * or you didn't set `tracesSampleRate`, this function will not generate spans * and the `span` returned from the callback will be undefined. */ -export function startSpan(context: StartSpanOptions, callback: (span: Span | undefined) => T): T { +export function startSpan(context: StartSpanOptions, callback: (span: Span) => T): T { const spanContext = normalizeContext(context); return withScope(context.scope, scope => { @@ -35,7 +36,7 @@ export function startSpan(context: StartSpanOptions, callback: (span: Span | const shouldSkipSpan = context.onlyIfParent && !parentSpan; const activeSpan = shouldSkipSpan - ? undefined + ? new SentryNonRecordingSpan() : createChildSpanOrTransaction(hub, { parentSpan, spanContext, @@ -43,10 +44,8 @@ export function startSpan(context: StartSpanOptions, callback: (span: Span | scope, }); - if (activeSpan) { - // eslint-disable-next-line deprecation/deprecation - scope.setSpan(activeSpan); - } + // eslint-disable-next-line deprecation/deprecation + scope.setSpan(activeSpan); return handleCallbackErrors( () => callback(activeSpan), @@ -75,10 +74,7 @@ export function startSpan(context: StartSpanOptions, callback: (span: Span | * or you didn't set `tracesSampleRate`, this function will not generate spans * and the `span` returned from the callback will be undefined. */ -export function startSpanManual( - context: StartSpanOptions, - callback: (span: Span | undefined, finish: () => void) => T, -): T { +export function startSpanManual(context: StartSpanOptions, callback: (span: Span, finish: () => void) => T): T { const spanContext = normalizeContext(context); return withScope(context.scope, scope => { @@ -89,7 +85,7 @@ export function startSpanManual( const shouldSkipSpan = context.onlyIfParent && !parentSpan; const activeSpan = shouldSkipSpan - ? undefined + ? new SentryNonRecordingSpan() : createChildSpanOrTransaction(hub, { parentSpan, spanContext, @@ -97,10 +93,8 @@ export function startSpanManual( scope, }); - if (activeSpan) { - // eslint-disable-next-line deprecation/deprecation - scope.setSpan(activeSpan); - } + // eslint-disable-next-line deprecation/deprecation + scope.setSpan(activeSpan); function finishAndSetSpan(): void { activeSpan && activeSpan.end(); @@ -131,11 +125,7 @@ export function startSpanManual( * or you didn't set `tracesSampleRate` or `tracesSampler`, this function will not generate spans * and the `span` returned from the callback will be undefined. */ -export function startInactiveSpan(context: StartSpanOptions): Span | undefined { - if (!hasTracingEnabled()) { - return undefined; - } - +export function startInactiveSpan(context: StartSpanOptions): Span { const spanContext = normalizeContext(context); // eslint-disable-next-line deprecation/deprecation const hub = getCurrentHub(); @@ -147,7 +137,7 @@ export function startInactiveSpan(context: StartSpanOptions): Span | undefined { const shouldSkipSpan = context.onlyIfParent && !parentSpan; if (shouldSkipSpan) { - return undefined; + return new SentryNonRecordingSpan(); } const scope = context.scope || getCurrentScope(); @@ -275,14 +265,14 @@ function createChildSpanOrTransaction( forceTransaction?: boolean; scope: Scope; }, -): Span | undefined { +): Span { if (!hasTracingEnabled()) { - return undefined; + return new SentryNonRecordingSpan(); } const isolationScope = getIsolationScope(); - let span: Span | undefined; + let span: Span; if (parentSpan && !forceTransaction) { // eslint-disable-next-line deprecation/deprecation span = parentSpan.startChild(spanContext); diff --git a/packages/core/src/tracing/transaction.ts b/packages/core/src/tracing/transaction.ts index 574b8888616d..ad7a120a52a6 100644 --- a/packages/core/src/tracing/transaction.ts +++ b/packages/core/src/tracing/transaction.ts @@ -19,10 +19,10 @@ import { DEBUG_BUILD } from '../debug-build'; import { getCurrentHub } from '../hub'; import { getMetricSummaryJsonForSpan } from '../metrics/metric-summary'; import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes'; -import { spanTimeInputToSeconds, spanToJSON, spanToTraceContext } from '../utils/spanUtils'; +import { getSpanDescendants, spanTimeInputToSeconds, spanToJSON, spanToTraceContext } from '../utils/spanUtils'; import { getDynamicSamplingContextFromSpan } from './dynamicSamplingContext'; import { SentrySpan, SpanRecorder } from './sentrySpan'; -import { getCapturedScopesOnSpan, getSpanDescendants } from './utils'; +import { getCapturedScopesOnSpan } from './utils'; /** JSDoc */ export class Transaction extends SentrySpan implements TransactionInterface { diff --git a/packages/core/src/tracing/utils.ts b/packages/core/src/tracing/utils.ts index c299281a6f95..03a4c45c28cf 100644 --- a/packages/core/src/tracing/utils.ts +++ b/packages/core/src/tracing/utils.ts @@ -31,54 +31,6 @@ export function getActiveSpan(): Span | undefined { return getCurrentScope().getSpan(); } -const CHILD_SPANS_FIELD = '_sentryChildSpans'; - -type SpanWithPotentialChildren = Span & { - [CHILD_SPANS_FIELD]?: Set; -}; - -/** - * Adds an opaque child span reference to a span. - */ -export function addChildSpanToSpan(span: SpanWithPotentialChildren, childSpan: Span): void { - if (span[CHILD_SPANS_FIELD] && span[CHILD_SPANS_FIELD].size < 1000) { - span[CHILD_SPANS_FIELD].add(childSpan); - } else { - span[CHILD_SPANS_FIELD] = new Set([childSpan]); - } -} - -/** This is only used internally by Idle Spans. */ -export function removeChildSpanFromSpan(span: SpanWithPotentialChildren, childSpan: Span): void { - if (span[CHILD_SPANS_FIELD]) { - span[CHILD_SPANS_FIELD].delete(childSpan); - } -} - -/** - * Returns an array of the given span and all of its descendants. - */ -export function getSpanDescendants(span: SpanWithPotentialChildren): Span[] { - const resultSet = new Set(); - - function addSpanChildren(span: SpanWithPotentialChildren): void { - // This exit condition is required to not infinitely loop in case of a circular dependency. - if (resultSet.has(span)) { - return; - } else { - resultSet.add(span); - const childSpans = span[CHILD_SPANS_FIELD] ? Array.from(span[CHILD_SPANS_FIELD]) : []; - for (const childSpan of childSpans) { - addSpanChildren(childSpan); - } - } - } - - addSpanChildren(span); - - return Array.from(resultSet); -} - const SCOPE_ON_START_SPAN_FIELD = '_sentryScope'; const ISOLATION_SCOPE_ON_START_SPAN_FIELD = '_sentryIsolationScope'; diff --git a/packages/core/src/utils/spanUtils.ts b/packages/core/src/utils/spanUtils.ts index 8560aece5ca4..e6f50edcfd8a 100644 --- a/packages/core/src/utils/spanUtils.ts +++ b/packages/core/src/utils/spanUtils.ts @@ -10,8 +10,8 @@ import type { 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'; +import { SPAN_STATUS_OK, SPAN_STATUS_UNSET } from '../tracing/spanstatus'; // These are aligned with OpenTelemetry trace flags export const TRACE_FLAG_NONE = 0x0; @@ -168,3 +168,52 @@ export function getStatusMessage(status: SpanStatus | undefined): string | undef return status.message || 'unknown_error'; } + +const CHILD_SPANS_FIELD = '_sentryChildSpans'; + +type SpanWithPotentialChildren = Span & { + [CHILD_SPANS_FIELD]?: Set; +}; + +/** + * Adds an opaque child span reference to a span. + */ +export function addChildSpanToSpan(span: SpanWithPotentialChildren, childSpan: Span): void { + if (span[CHILD_SPANS_FIELD] && span[CHILD_SPANS_FIELD].size < 1000) { + span[CHILD_SPANS_FIELD].add(childSpan); + } else { + span[CHILD_SPANS_FIELD] = new Set([childSpan]); + } +} + +/** This is only used internally by Idle Spans. */ +export function removeChildSpanFromSpan(span: SpanWithPotentialChildren, childSpan: Span): void { + if (span[CHILD_SPANS_FIELD]) { + span[CHILD_SPANS_FIELD].delete(childSpan); + } +} + +/** + * Returns an array of the given span and all of its descendants. + */ +export function getSpanDescendants(span: SpanWithPotentialChildren): Span[] { + const resultSet = new Set(); + + function addSpanChildren(span: SpanWithPotentialChildren): void { + // This exit condition is required to not infinitely loop in case of a circular dependency. + if (resultSet.has(span)) { + return; + // We want to ignore unsampled spans (e.g. non recording spans) + } else if (spanIsSampled(span)) { + resultSet.add(span); + const childSpans = span[CHILD_SPANS_FIELD] ? Array.from(span[CHILD_SPANS_FIELD]) : []; + for (const childSpan of childSpans) { + addSpanChildren(childSpan); + } + } + } + + addSpanChildren(span); + + return Array.from(resultSet); +} diff --git a/packages/core/test/lib/scope.test.ts b/packages/core/test/lib/scope.test.ts index 48a39d7f09b0..6ddb81086744 100644 --- a/packages/core/test/lib/scope.test.ts +++ b/packages/core/test/lib/scope.test.ts @@ -956,7 +956,7 @@ describe('withActiveSpan()', () => { expect(getActiveSpan()).not.toBe(inactiveSpan); - withActiveSpan(inactiveSpan!, () => { + withActiveSpan(inactiveSpan, () => { expect(getActiveSpan()).toBe(inactiveSpan); }); }); @@ -964,13 +964,13 @@ describe('withActiveSpan()', () => { it('should create child spans when calling startSpan within the callback', () => { const inactiveSpan = startInactiveSpan({ name: 'inactive-span' }); - const parentSpanId = withActiveSpan(inactiveSpan!, () => { + const parentSpanId = withActiveSpan(inactiveSpan, () => { return startSpan({ name: 'child-span' }, childSpan => { - return spanToJSON(childSpan!).parent_span_id; + return spanToJSON(childSpan).parent_span_id; }); }); - expect(parentSpanId).toBe(inactiveSpan?.spanContext().spanId); + expect(parentSpanId).toBe(inactiveSpan.spanContext().spanId); }); it('when `null` is passed, no span should be active within the callback', () => { diff --git a/packages/core/test/lib/tracing/errors.test.ts b/packages/core/test/lib/tracing/errors.test.ts index 24c19a24121d..76c4aa9609f2 100644 --- a/packages/core/test/lib/tracing/errors.test.ts +++ b/packages/core/test/lib/tracing/errors.test.ts @@ -68,7 +68,7 @@ describe('registerErrorHandlers()', () => { startSpan({ name: 'test' }, span => { mockErrorCallback({} as HandlerDataError); - expect(spanToJSON(span!).status).toBe('internal_error'); + expect(spanToJSON(span).status).toBe('internal_error'); }); }); @@ -77,7 +77,7 @@ describe('registerErrorHandlers()', () => { startSpan({ name: 'test' }, span => { mockUnhandledRejectionCallback({}); - expect(spanToJSON(span!).status).toBe('internal_error'); + expect(spanToJSON(span).status).toBe('internal_error'); }); }); }); diff --git a/packages/core/test/lib/tracing/idleSpan.test.ts b/packages/core/test/lib/tracing/idleSpan.test.ts index ade484bbce4b..fd23b9b8d041 100644 --- a/packages/core/test/lib/tracing/idleSpan.test.ts +++ b/packages/core/test/lib/tracing/idleSpan.test.ts @@ -2,6 +2,8 @@ import { TestClient, getDefaultTestClientOptions } from '../../mocks/client'; import type { Event, Span } from '@sentry/types'; import { + SentryNonRecordingSpan, + SentrySpan, addTracingExtensions, getActiveSpan, getClient, @@ -40,6 +42,7 @@ describe('startIdleSpan', () => { it('sets & unsets the idle span on the scope', () => { const idleSpan = startIdleSpan({ name: 'foo' }); expect(idleSpan).toBeDefined(); + expect(idleSpan).toBeInstanceOf(SentrySpan); expect(getActiveSpan()).toBe(idleSpan); @@ -49,12 +52,26 @@ describe('startIdleSpan', () => { expect(getActiveSpan()).toBe(undefined); }); + it('returns non recording span if tracing is disabled', () => { + const options = getDefaultTestClientOptions({ dsn }); + const client = new TestClient(options); + setCurrentClient(client); + client.init(); + + const idleSpan = startIdleSpan({ name: 'foo' }); + expect(idleSpan).toBeDefined(); + expect(idleSpan).toBeInstanceOf(SentryNonRecordingSpan); + + // not set as active span, though + expect(getActiveSpan()).toBe(undefined); + }); + it('does not finish idle span if there are still active activities', () => { - const idleSpan = startIdleSpan({ name: 'foo' })!; + const idleSpan = startIdleSpan({ name: 'foo' }); expect(idleSpan).toBeDefined(); startSpanManual({ name: 'inner1' }, span => { - const childSpan = startInactiveSpan({ name: 'inner2' })!; + const childSpan = startInactiveSpan({ name: 'inner2' }); span?.end(); jest.advanceTimersByTime(TRACING_DEFAULTS.idleTimeout + 1); @@ -72,7 +89,7 @@ describe('startIdleSpan', () => { it('calls beforeSpanEnd callback before finishing', () => { const beforeSpanEnd = jest.fn(); - const idleSpan = startIdleSpan({ name: 'foo' }, { beforeSpanEnd })!; + const idleSpan = startIdleSpan({ name: 'foo' }, { beforeSpanEnd }); expect(idleSpan).toBeDefined(); expect(beforeSpanEnd).not.toHaveBeenCalled(); @@ -108,7 +125,7 @@ describe('startIdleSpan', () => { const inner = startInactiveSpan({ name: 'from beforeSpanEnd', startTime: baseTimeInSeconds }); inner?.end(baseTimeInSeconds); }); - const idleSpan = startIdleSpan({ name: 'idle span 2', startTime: baseTimeInSeconds }, { beforeSpanEnd })!; + const idleSpan = startIdleSpan({ name: 'idle span 2', startTime: baseTimeInSeconds }, { beforeSpanEnd }); expect(idleSpan).toBeDefined(); expect(beforeSpanEnd).not.toHaveBeenCalled(); @@ -152,26 +169,26 @@ describe('startIdleSpan', () => { // We want to accomodate a bit of drift there, so we ensure this starts earlier... const baseTimeInSeconds = Math.floor(Date.now() / 1000) - 9999; - const idleSpan = startIdleSpan({ name: 'idle span', startTime: baseTimeInSeconds })!; + const idleSpan = startIdleSpan({ name: 'idle span', startTime: baseTimeInSeconds }); expect(idleSpan).toBeDefined(); // regular child - should be kept const regularSpan = startInactiveSpan({ name: 'regular span', startTime: baseTimeInSeconds + 2, - })!; + }); // discardedSpan - startTimestamp is too large - const discardedSpan = startInactiveSpan({ name: 'discarded span', startTime: baseTimeInSeconds + 99 })!; + const discardedSpan = startInactiveSpan({ name: 'discarded span', startTime: baseTimeInSeconds + 99 }); // discardedSpan2 - endTime is too large - const discardedSpan2 = startInactiveSpan({ name: 'discarded span', startTime: baseTimeInSeconds + 3 })!; + const discardedSpan2 = startInactiveSpan({ name: 'discarded span', startTime: baseTimeInSeconds + 3 }); discardedSpan2.end(baseTimeInSeconds + 99)!; // Should be cancelled - will not finish const cancelledSpan = startInactiveSpan({ name: 'cancelled span', startTime: baseTimeInSeconds + 4, - })!; + }); regularSpan.end(baseTimeInSeconds + 4); idleSpan.end(baseTimeInSeconds + 10); @@ -225,7 +242,7 @@ describe('startIdleSpan', () => { hookSpans.push({ span, hook: 'spanEnd' }); }); - const idleSpan = startIdleSpan({ name: 'idle span' })!; + const idleSpan = startIdleSpan({ name: 'idle span' }); expect(idleSpan).toBeDefined(); expect(hookSpans).toEqual([{ span: idleSpan, hook: 'spanStart' }]); @@ -250,7 +267,7 @@ describe('startIdleSpan', () => { const recordDroppedEventSpy = jest.spyOn(client, 'recordDroppedEvent'); - const idleSpan = startIdleSpan({ name: 'idle span' })!; + const idleSpan = startIdleSpan({ name: 'idle span' }); expect(idleSpan).toBeDefined(); idleSpan?.end(); @@ -260,7 +277,7 @@ describe('startIdleSpan', () => { describe('idleTimeout', () => { it('finishes if no activities are added to the idle span', () => { - const idleSpan = startIdleSpan({ name: 'idle span' })!; + const idleSpan = startIdleSpan({ name: 'idle span' }); expect(idleSpan).toBeDefined(); jest.advanceTimersByTime(TRACING_DEFAULTS.idleTimeout); @@ -268,7 +285,7 @@ describe('startIdleSpan', () => { }); it('does not finish if a activity is started', () => { - const idleSpan = startIdleSpan({ name: 'idle span' })!; + const idleSpan = startIdleSpan({ name: 'idle span' }); expect(idleSpan).toBeDefined(); startInactiveSpan({ name: 'span' }); @@ -279,7 +296,7 @@ describe('startIdleSpan', () => { it('does not finish when idleTimeout is not exceed after last activity finished', () => { const idleTimeout = 10; - const idleSpan = startIdleSpan({ name: 'idle span' }, { idleTimeout })!; + const idleSpan = startIdleSpan({ name: 'idle span' }, { idleTimeout }); expect(idleSpan).toBeDefined(); startSpan({ name: 'span1' }, () => {}); @@ -295,7 +312,7 @@ describe('startIdleSpan', () => { it('finish when idleTimeout is exceeded after last activity finished', () => { const idleTimeout = 10; - const idleSpan = startIdleSpan({ name: 'idle span', startTime: 1234 }, { idleTimeout })!; + const idleSpan = startIdleSpan({ name: 'idle span', startTime: 1234 }, { idleTimeout }); expect(idleSpan).toBeDefined(); startSpan({ name: 'span1' }, () => {}); @@ -312,7 +329,7 @@ describe('startIdleSpan', () => { describe('child span timeout', () => { it('finishes when a child span exceed timeout', () => { - const idleSpan = startIdleSpan({ name: 'idle span' })!; + const idleSpan = startIdleSpan({ name: 'idle span' }); expect(idleSpan).toBeDefined(); // Start any span to cancel idle timeout @@ -333,7 +350,7 @@ describe('startIdleSpan', () => { }); it('resets after new activities are added', () => { - const idleSpan = startIdleSpan({ name: 'idle span' }, { finalTimeout: 99_999 })!; + const idleSpan = startIdleSpan({ name: 'idle span' }, { finalTimeout: 99_999 }); expect(idleSpan).toBeDefined(); // Start any span to cancel idle timeout @@ -370,7 +387,7 @@ describe('startIdleSpan', () => { describe('disableAutoFinish', () => { it('skips idle timeout if disableAutoFinish=true', () => { - const idleSpan = startIdleSpan({ name: 'idle span' }, { disableAutoFinish: true })!; + const idleSpan = startIdleSpan({ name: 'idle span' }, { disableAutoFinish: true }); expect(idleSpan).toBeDefined(); jest.advanceTimersByTime(TRACING_DEFAULTS.idleTimeout); @@ -387,7 +404,7 @@ describe('startIdleSpan', () => { }); it('skips span timeout if disableAutoFinish=true', () => { - const idleSpan = startIdleSpan({ name: 'idle span' }, { disableAutoFinish: true, finalTimeout: 99_999 })!; + const idleSpan = startIdleSpan({ name: 'idle span' }, { disableAutoFinish: true, finalTimeout: 99_999 }); expect(idleSpan).toBeDefined(); startInactiveSpan({ name: 'inner' }); @@ -406,7 +423,7 @@ describe('startIdleSpan', () => { }); it('times out at final timeout if disableAutoFinish=true', () => { - const idleSpan = startIdleSpan({ name: 'idle span' }, { disableAutoFinish: true })!; + const idleSpan = startIdleSpan({ name: 'idle span' }, { disableAutoFinish: true }); expect(idleSpan).toBeDefined(); jest.advanceTimersByTime(TRACING_DEFAULTS.finalTimeout); @@ -414,8 +431,8 @@ describe('startIdleSpan', () => { }); it('ignores it if hook is emitted with other span', () => { - const span = startInactiveSpan({ name: 'other span' })!; - const idleSpan = startIdleSpan({ name: 'idle span' }, { disableAutoFinish: true })!; + const span = startInactiveSpan({ name: 'other span' }); + const idleSpan = startIdleSpan({ name: 'idle span' }, { disableAutoFinish: true }); expect(idleSpan).toBeDefined(); jest.advanceTimersByTime(TRACING_DEFAULTS.idleTimeout); diff --git a/packages/core/test/lib/tracing/idletransaction.test.ts b/packages/core/test/lib/tracing/idletransaction.test.ts index 56ed93abe5d9..ac55ed7c1c58 100644 --- a/packages/core/test/lib/tracing/idletransaction.test.ts +++ b/packages/core/test/lib/tracing/idletransaction.test.ts @@ -170,10 +170,10 @@ describe('IdleTransaction', () => { startSpanManual({ name: 'inner1' }, span => { const childSpan = startInactiveSpan({ name: 'inner2' })!; expect(transaction.activities).toMatchObject({ - [span!.spanContext().spanId]: true, + [span.spanContext().spanId]: true, [childSpan.spanContext().spanId]: true, }); - span?.end(); + span.end(); jest.advanceTimersByTime(TRACING_DEFAULTS.idleTimeout + 1); expect(mockFinish).toHaveBeenCalledTimes(0); diff --git a/packages/core/test/lib/tracing/sentryNonRecordingSpan.test.ts b/packages/core/test/lib/tracing/sentryNonRecordingSpan.test.ts new file mode 100644 index 000000000000..d5643c3d3651 --- /dev/null +++ b/packages/core/test/lib/tracing/sentryNonRecordingSpan.test.ts @@ -0,0 +1,37 @@ +import type { Span } from '@sentry/types'; +import { SPAN_STATUS_ERROR } from '../../../src/tracing'; +import { SentryNonRecordingSpan } from '../../../src/tracing/sentryNonRecordingSpan'; +import { TRACE_FLAG_NONE, spanIsSampled, spanToJSON } from '../../../src/utils/spanUtils'; + +describe('SentryNonRecordingSpan', () => { + it('satisfies the Span interface', () => { + const span: Span = new SentryNonRecordingSpan(); + + expect(span.spanContext()).toEqual({ + spanId: expect.any(String), + traceId: expect.any(String), + traceFlags: TRACE_FLAG_NONE, + }); + + expect(spanIsSampled(span)).toBe(false); + expect(span.isRecording()).toBe(false); + expect(spanToJSON(span)).toEqual({ + span_id: expect.any(String), + trace_id: expect.any(String), + }); + + // Ensure all methods work + span.end(); + span.end(123); + span.updateName('name'); + span.setAttribute('key', 'value'); + span.setAttributes({ key: 'value' }); + span.setStatus({ code: SPAN_STATUS_ERROR }); + + // but nothing is actually set/readable + expect(spanToJSON(span)).toEqual({ + span_id: expect.any(String), + trace_id: expect.any(String), + }); + }); +}); diff --git a/packages/core/test/lib/tracing/span.test.ts b/packages/core/test/lib/tracing/sentrySpan.test.ts similarity index 99% rename from packages/core/test/lib/tracing/span.test.ts rename to packages/core/test/lib/tracing/sentrySpan.test.ts index 9edceb86c33d..e065aeff33aa 100644 --- a/packages/core/test/lib/tracing/span.test.ts +++ b/packages/core/test/lib/tracing/sentrySpan.test.ts @@ -1,9 +1,9 @@ import { timestampInSeconds } from '@sentry/utils'; -import { SentrySpan } from '../../../src'; +import { SentrySpan } from '../../../src/tracing/sentrySpan'; import { SPAN_STATUS_ERROR } from '../../../src/tracing/spanstatus'; import { TRACE_FLAG_NONE, TRACE_FLAG_SAMPLED, spanToJSON, spanToTraceContext } from '../../../src/utils/spanUtils'; -describe('span', () => { +describe('SentrySpan', () => { describe('name', () => { it('works with name', () => { const span = new SentrySpan({ name: 'span name' }); diff --git a/packages/core/test/lib/tracing/spanstatus.test.ts b/packages/core/test/lib/tracing/spanstatus.test.ts index 3c72406209ca..2ed356b1d73e 100644 --- a/packages/core/test/lib/tracing/spanstatus.test.ts +++ b/packages/core/test/lib/tracing/spanstatus.test.ts @@ -18,9 +18,9 @@ describe('setHttpStatus', () => { ])('applies the correct span status and http status code to the span (%s - $%s)', (code, status) => { const span = new SentrySpan({ name: 'test' }); - setHttpStatus(span!, code); + setHttpStatus(span, code); - const { status: spanStatus, data } = spanToJSON(span!); + const { status: spanStatus, data } = spanToJSON(span); expect(spanStatus).toBe(status); expect(data).toMatchObject({ 'http.response.status_code': code }); @@ -29,9 +29,9 @@ describe('setHttpStatus', () => { it("doesn't set the status for an unknown http status code", () => { const span = new SentrySpan({ name: 'test' }); - setHttpStatus(span!, 600); + setHttpStatus(span, 600); - const { status: spanStatus, data } = spanToJSON(span!); + const { status: spanStatus, data } = spanToJSON(span); expect(spanStatus).toBeUndefined(); expect(data).toMatchObject({ 'http.response.status_code': 600 }); diff --git a/packages/core/test/lib/tracing/trace.test.ts b/packages/core/test/lib/tracing/trace.test.ts index d92032599211..a4ae80fcbc7c 100644 --- a/packages/core/test/lib/tracing/trace.test.ts +++ b/packages/core/test/lib/tracing/trace.test.ts @@ -19,7 +19,8 @@ import { startSpan, startSpanManual, } from '../../../src/tracing'; -import { getSpanDescendants } from '../../../src/tracing/utils'; +import { SentryNonRecordingSpan } from '../../../src/tracing/sentryNonRecordingSpan'; +import { getSpanDescendants } from '../../../src/utils/spanUtils'; import { TestClient, getDefaultTestClientOptions } from '../../mocks/client'; beforeAll(() => { @@ -145,9 +146,7 @@ describe('startSpan', () => { }); try { await startSpan({ name: 'GET users/[id]' }, span => { - if (span) { - span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'http.server'); - } + span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'http.server'); return callback(); }); } catch (e) { @@ -243,21 +242,35 @@ describe('startSpan', () => { }); }); + it('returns a non recording span if tracing is disabled', () => { + const options = getDefaultTestClientOptions({}); + client = new TestClient(options); + setCurrentClient(client); + client.init(); + + const span = startSpan({ name: 'GET users/[id]' }, span => { + return span; + }); + + expect(span).toBeDefined(); + expect(span).toBeInstanceOf(SentryNonRecordingSpan); + }); + it('creates & finishes span', async () => { - let _span: SentrySpan | undefined; - startSpan({ name: 'GET users/[id]' }, span => { + const span = startSpan({ name: 'GET users/[id]' }, span => { expect(span).toBeDefined(); - expect(spanToJSON(span!).timestamp).toBeUndefined(); - _span = span as SentrySpan; + expect(span).toBeInstanceOf(SentrySpan); + expect(spanToJSON(span).timestamp).toBeUndefined(); + return span; }); - expect(_span).toBeDefined(); - expect(spanToJSON(_span!).timestamp).toBeDefined(); + expect(span).toBeDefined(); + expect(spanToJSON(span).timestamp).toBeDefined(); }); it('allows to pass a `startTime`', () => { const start = startSpan({ name: 'outer', startTime: [1234, 0] }, span => { - return spanToJSON(span!).start_timestamp; + return spanToJSON(span).start_timestamp; }); expect(start).toEqual(1234); @@ -287,7 +300,7 @@ describe('startSpan', () => { expect(getCurrentScope()).not.toBe(initialScope); expect(getCurrentScope()).toBe(manualScope); expect(getActiveSpan()).toBe(span); - expect(spanToJSON(span!).parent_span_id).toBe('parent-span-id'); + expect(spanToJSON(span).parent_span_id).toBe('parent-span-id'); }); expect(getCurrentScope()).toBe(initialScope); @@ -407,18 +420,19 @@ describe('startSpan', () => { }); startSpan({ name: 'span' }, span => { - expect(span?.spanContext().traceId).toBe('99999999999999999999999999999999'); + expect(span.spanContext().traceId).toBe('99999999999999999999999999999999'); }); }); }); describe('onlyIfParent', () => { - it('does not create a span if there is no parent', () => { + it('starts a non recording span if there is no parent', () => { const span = startSpan({ name: 'test span', onlyIfParent: true }, span => { return span; }); - expect(span).toBeUndefined(); + expect(span).toBeDefined(); + expect(span).toBeInstanceOf(SentryNonRecordingSpan); }); it('creates a span if there is a parent', () => { @@ -431,6 +445,7 @@ describe('startSpan', () => { }); expect(span).toBeDefined(); + expect(span).toBeInstanceOf(SentrySpan); }); }); @@ -482,7 +497,7 @@ describe('startSpan', () => { startSpanManual({ name: 'my-span' }, span => { withScope(scope2 => { scope2.setTag('scope', 2); - span?.end(); + span.end(); }); }); }); @@ -519,12 +534,27 @@ describe('startSpanManual', () => { client.init(); }); + it('returns a non recording span if tracing is disabled', () => { + const options = getDefaultTestClientOptions({}); + client = new TestClient(options); + setCurrentClient(client); + client.init(); + + const span = startSpanManual({ name: 'GET users/[id]' }, span => { + return span; + }); + + expect(span).toBeDefined(); + expect(span).toBeInstanceOf(SentryNonRecordingSpan); + }); + it('creates & finishes span', async () => { startSpanManual({ name: 'GET users/[id]' }, (span, finish) => { expect(span).toBeDefined(); - expect(spanToJSON(span!).timestamp).toBeUndefined(); + expect(span).toBeInstanceOf(SentrySpan); + expect(spanToJSON(span).timestamp).toBeUndefined(); finish(); - expect(spanToJSON(span!).timestamp).toBeDefined(); + expect(spanToJSON(span).timestamp).toBeDefined(); }); }); @@ -557,7 +587,7 @@ describe('startSpanManual', () => { expect(getCurrentScope()).not.toBe(initialScope); expect(getCurrentScope()).toBe(manualScope); expect(getActiveSpan()).toBe(span); - expect(spanToJSON(span!).parent_span_id).toBe('parent-span-id'); + expect(spanToJSON(span).parent_span_id).toBe('parent-span-id'); finish(); @@ -589,13 +619,13 @@ describe('startSpanManual', () => { startSpanManual({ name: 'inner transaction', forceTransaction: true }, span => { startSpanManual({ name: 'inner span 2' }, span => { // all good - span?.end(); + span.end(); }); - span?.end(); + span.end(); }); - span?.end(); + span.end(); }); - span?.end(); + span.end(); }); await client.flush(); @@ -677,8 +707,8 @@ describe('startSpanManual', () => { it('allows to pass a `startTime`', () => { const start = startSpanManual({ name: 'outer', startTime: [1234, 0] }, span => { - span?.end(); - return spanToJSON(span!).start_timestamp; + span.end(); + return spanToJSON(span).start_timestamp; }); expect(start).toEqual(1234); @@ -695,8 +725,8 @@ describe('startSpanManual', () => { }); startSpanManual({ name: 'span' }, span => { - expect(span?.spanContext().traceId).toBe('99999999999999999999999999999991'); - span?.end(); + expect(span.spanContext().traceId).toBe('99999999999999999999999999999991'); + span.end(); }); }); }); @@ -706,8 +736,8 @@ describe('startSpanManual', () => { const span = startSpanManual({ name: 'test span', onlyIfParent: true }, span => { return span; }); - - expect(span).toBeUndefined(); + expect(span).toBeDefined(); + expect(span).toBeInstanceOf(SentryNonRecordingSpan); }); it('creates a span if there is a parent', () => { @@ -720,6 +750,7 @@ describe('startSpanManual', () => { }); expect(span).toBeDefined(); + expect(span).toBeInstanceOf(SentrySpan); }); }); @@ -742,15 +773,28 @@ describe('startInactiveSpan', () => { client.init(); }); + it('returns a non recording span if tracing is disabled', () => { + const options = getDefaultTestClientOptions({}); + client = new TestClient(options); + setCurrentClient(client); + client.init(); + + const span = startInactiveSpan({ name: 'GET users/[id]' }); + + expect(span).toBeDefined(); + expect(span).toBeInstanceOf(SentryNonRecordingSpan); + }); + it('creates & finishes span', async () => { const span = startInactiveSpan({ name: 'GET users/[id]' }); expect(span).toBeDefined(); - expect(spanToJSON(span!).timestamp).toBeUndefined(); + expect(span).toBeInstanceOf(SentrySpan); + expect(spanToJSON(span).timestamp).toBeUndefined(); - span?.end(); + span.end(); - expect(spanToJSON(span!).timestamp).toBeDefined(); + expect(spanToJSON(span).timestamp).toBeDefined(); }); it('does not set span on scope', () => { @@ -759,7 +803,7 @@ describe('startInactiveSpan', () => { expect(span).toBeDefined(); expect(getActiveSpan()).toBeUndefined(); - span?.end(); + span.end(); expect(getActiveSpan()).toBeUndefined(); }); @@ -775,10 +819,10 @@ describe('startInactiveSpan', () => { const span = startInactiveSpan({ name: 'GET users/[id]', scope: manualScope }); expect(span).toBeDefined(); - expect(spanToJSON(span!).parent_span_id).toBe('parent-span-id'); + expect(spanToJSON(span).parent_span_id).toBe('parent-span-id'); expect(getActiveSpan()).toBeUndefined(); - span?.end(); + span.end(); expect(getActiveSpan()).toBeUndefined(); }); @@ -884,7 +928,7 @@ describe('startInactiveSpan', () => { it('allows to pass a `startTime`', () => { const span = startInactiveSpan({ name: 'outer', startTime: [1234, 0] }); - expect(spanToJSON(span!).start_timestamp).toEqual(1234); + expect(spanToJSON(span).start_timestamp).toEqual(1234); }); it("picks up the trace id off the parent scope's propagation context", () => { @@ -898,8 +942,8 @@ describe('startInactiveSpan', () => { }); const span = startInactiveSpan({ name: 'span' }); - expect(span?.spanContext().traceId).toBe('99999999999999999999999999999991'); - span?.end(); + expect(span.spanContext().traceId).toBe('99999999999999999999999999999991'); + span.end(); }); }); @@ -907,17 +951,18 @@ describe('startInactiveSpan', () => { it('does not create a span if there is no parent', () => { const span = startInactiveSpan({ name: 'test span', onlyIfParent: true }); - expect(span).toBeUndefined(); + expect(span).toBeDefined(); + expect(span).toBeInstanceOf(SentryNonRecordingSpan); }); it('creates a span if there is a parent', () => { const span = startSpan({ name: 'parent span' }, () => { const span = startInactiveSpan({ name: 'test span', onlyIfParent: true }); - return span; }); expect(span).toBeDefined(); + expect(span).toBeInstanceOf(SentrySpan); }); }); @@ -934,7 +979,7 @@ describe('startInactiveSpan', () => { setCurrentClient(client); client.init(); - let span: Span | undefined; + let span: Span; const scope = getCurrentScope(); scope.setTag('outer', 'foo'); @@ -947,7 +992,7 @@ describe('startInactiveSpan', () => { withScope(scope => { scope.setTag('scope', 2); - span?.end(); + span.end(); }); await client.flush(); diff --git a/packages/nextjs/src/common/utils/edgeWrapperUtils.ts b/packages/nextjs/src/common/utils/edgeWrapperUtils.ts index be0c56236233..d260fb481b54 100644 --- a/packages/nextjs/src/common/utils/edgeWrapperUtils.ts +++ b/packages/nextjs/src/common/utils/edgeWrapperUtils.ts @@ -67,12 +67,10 @@ export function withEdgeWrapping( }, ); - if (span) { - if (handlerResult instanceof Response) { - setHttpStatus(span, handlerResult.status); - } else { - span.setStatus({ code: SPAN_STATUS_OK }); - } + if (handlerResult instanceof Response) { + setHttpStatus(span, handlerResult.status); + } else { + span.setStatus({ code: SPAN_STATUS_OK }); } return handlerResult; diff --git a/packages/nextjs/src/common/utils/responseEnd.ts b/packages/nextjs/src/common/utils/responseEnd.ts index 501208f81e84..c287933f6c39 100644 --- a/packages/nextjs/src/common/utils/responseEnd.ts +++ b/packages/nextjs/src/common/utils/responseEnd.ts @@ -39,11 +39,9 @@ export function autoEndSpanOnResponseEnd(span: Span, res: ServerResponse): void } /** Finish the given response's span and set HTTP status data */ -export function finishSpan(span: Span | undefined, res: ServerResponse): void { - if (span) { - setHttpStatus(span, res.statusCode); - span.end(); - } +export function finishSpan(span: Span, res: ServerResponse): void { + setHttpStatus(span, res.statusCode); + span.end(); } /** Flush the event queue to ensure that events get sent to Sentry before the response is finished and the lambda ends */ diff --git a/packages/nextjs/src/common/utils/wrapperUtils.ts b/packages/nextjs/src/common/utils/wrapperUtils.ts index 5e489fb90c09..f958089f6cad 100644 --- a/packages/nextjs/src/common/utils/wrapperUtils.ts +++ b/packages/nextjs/src/common/utils/wrapperUtils.ts @@ -128,15 +128,15 @@ export function withTracedServerSideDataFetcher Pr }, }, async dataFetcherSpan => { - dataFetcherSpan?.setStatus({ code: SPAN_STATUS_OK }); + dataFetcherSpan.setStatus({ code: SPAN_STATUS_OK }); try { return await origDataFetcher.apply(this, args); } catch (e) { - dataFetcherSpan?.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + dataFetcherSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); requestSpan?.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); throw e; } finally { - dataFetcherSpan?.end(); + dataFetcherSpan.end(); if (!platformSupportsStreaming()) { await flushQueue(); } @@ -182,16 +182,16 @@ export async function callDataFetcherTraced Promis }, }, async dataFetcherSpan => { - dataFetcherSpan?.setStatus({ code: SPAN_STATUS_OK }); + dataFetcherSpan.setStatus({ code: SPAN_STATUS_OK }); try { return await origFunction(...origFunctionArgs); } catch (e) { - dataFetcherSpan?.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + dataFetcherSpan.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); captureException(e, { mechanism: { handled: false } }); throw e; } finally { - dataFetcherSpan?.end(); + dataFetcherSpan.end(); if (!platformSupportsStreaming()) { await flushQueue(); } diff --git a/packages/nextjs/src/common/withServerActionInstrumentation.ts b/packages/nextjs/src/common/withServerActionInstrumentation.ts index 39c26e4dfca6..44d22606b974 100644 --- a/packages/nextjs/src/common/withServerActionInstrumentation.ts +++ b/packages/nextjs/src/common/withServerActionInstrumentation.ts @@ -102,11 +102,11 @@ async function withServerActionInstrumentationImplementation { if (isNotFoundNavigationError(error)) { // We don't want to report "not-found"s - span?.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); } else if (isRedirectNavigationError(error)) { // Don't do anything for redirects } else { - span?.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); captureException(error, { mechanism: { handled: false, diff --git a/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts b/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts index 1c217ac9cbf6..41537a4ad49f 100644 --- a/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts +++ b/packages/nextjs/src/common/wrapApiHandlerWithSentry.ts @@ -99,10 +99,8 @@ export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameteriz // eslint-disable-next-line @typescript-eslint/unbound-method res.end = new Proxy(res.end, { apply(target, thisArg, argArray) { - if (span) { - setHttpStatus(span, res.statusCode); - span.end(); - } + setHttpStatus(span, res.statusCode); + span.end(); if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) { target.apply(thisArg, argArray); } else { @@ -155,10 +153,8 @@ export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameteriz res.statusCode = 500; res.statusMessage = 'Internal Server Error'; - if (span) { - setHttpStatus(span, res.statusCode); - span.end(); - } + setHttpStatus(span, res.statusCode); + span.end(); // Make sure we have a chance to finish the transaction and flush events to Sentry before the handler errors // out. (Apps which are deployed on Vercel run their API routes in lambdas, and those lambdas will shut down the diff --git a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts index 0041d84c9838..339b64b300cf 100644 --- a/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts +++ b/packages/nextjs/src/common/wrapGenerationFunctionWithSentry.ts @@ -80,12 +80,12 @@ export function wrapGenerationFunctionWithSentry a err => { if (isNotFoundNavigationError(err)) { // We don't want to report "not-found"s - span?.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); } else if (isRedirectNavigationError(err)) { // We don't want to report redirects - span?.setStatus({ code: SPAN_STATUS_OK }); + span.setStatus({ code: SPAN_STATUS_OK }); } else { - span?.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); captureException(err, { mechanism: { handled: false, @@ -94,7 +94,7 @@ export function wrapGenerationFunctionWithSentry a } }, () => { - span?.end(); + span.end(); }, ); }, diff --git a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts index 06a72a61ac18..4740f366940a 100644 --- a/packages/nextjs/src/common/wrapServerComponentWithSentry.ts +++ b/packages/nextjs/src/common/wrapServerComponentWithSentry.ts @@ -69,12 +69,12 @@ export function wrapServerComponentWithSentry any> error => { if (isNotFoundNavigationError(error)) { // We don't want to report "not-found"s - span?.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'not_found' }); } else if (isRedirectNavigationError(error)) { // We don't want to report redirects - span?.setStatus({ code: SPAN_STATUS_OK }); + span.setStatus({ code: SPAN_STATUS_OK }); } else { - span?.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); captureException(error, { mechanism: { handled: false, @@ -83,7 +83,7 @@ export function wrapServerComponentWithSentry any> } }, () => { - span?.end(); + span.end(); // flushQueue should not throw // eslint-disable-next-line @typescript-eslint/no-floating-promises diff --git a/packages/node-experimental/test/integration/transactions.test.ts b/packages/node-experimental/test/integration/transactions.test.ts index aedde492cd63..200b84ed6091 100644 --- a/packages/node-experimental/test/integration/transactions.test.ts +++ b/packages/node-experimental/test/integration/transactions.test.ts @@ -623,7 +623,7 @@ describe('Integration | Transactions', () => { void Sentry.startSpan({ name: 'test name' }, async () => { const subSpan = Sentry.startInactiveSpan({ name: 'inner span 1' }); - innerSpan1Id = subSpan?.spanContext().spanId; + innerSpan1Id = subSpan.spanContext().spanId; subSpan.end(); Sentry.startSpan({ name: 'inner span 2' }, innerSpan => { diff --git a/packages/node/test/integrations/undici.test.ts b/packages/node/test/integrations/undici.test.ts index 381251d83edd..86bfb98a6802 100644 --- a/packages/node/test/integrations/undici.test.ts +++ b/packages/node/test/integrations/undici.test.ts @@ -240,7 +240,7 @@ conditionalTest({ min: 16 })('Undici integration', () => { expect(spans.length).toBe(2); const span = spans[1]; - expect(requestHeaders['sentry-trace']).toEqual(spanToTraceHeader(span!)); + expect(requestHeaders['sentry-trace']).toEqual(spanToTraceHeader(span)); expect(requestHeaders['baggage']).toEqual( `sentry-environment=production,sentry-public_key=0,sentry-trace_id=${ span.spanContext().traceId diff --git a/packages/node/test/performance.test.ts b/packages/node/test/performance.test.ts index 0d528e37f7e2..86907e408746 100644 --- a/packages/node/test/performance.test.ts +++ b/packages/node/test/performance.test.ts @@ -106,7 +106,7 @@ describe('startSpanManual()', () => { startSpanManual({ name: 'first' }, span => { return new Promise(resolve => { setTimeout(() => { - span?.end(); + span.end(); resolve(); }, 500); }); @@ -115,7 +115,7 @@ describe('startSpanManual()', () => { startSpanManual({ name: 'second' }, span => { return new Promise(resolve => { setTimeout(() => { - span?.end(); + span.end(); resolve(); }, 500); }); @@ -182,7 +182,7 @@ describe('startSpanManual()', () => { isolationScope2.setTag('isolationScope', 2); withScope(scope2 => { scope2.setTag('scope', 2); - span?.end(); + span.end(); }); }); }); diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index 348304c82a92..d818bd0d9763 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -11,7 +11,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, getCurrentHub, } from '@sentry/core'; -import type { Scope, SpanOrigin, TransactionSource } from '@sentry/types'; +import type { Scope, Span, SpanOrigin, TransactionSource } from '@sentry/types'; import { addNonEnumerableProperty, dropUndefinedKeys, logger } from '@sentry/utils'; import { startTransaction } from './custom/transaction'; @@ -314,9 +314,7 @@ const SCOPE_ON_START_SPAN_FIELD = '_sentryScope'; const ISOLATION_SCOPE_ON_START_SPAN_FIELD = '_sentryIsolationScope'; /** Sets the scope and isolation scope to be used for when the transaction is finished. */ -function setCapturedScopesOnTransaction(span: Transaction, scope: Scope, isolationScope: Scope): void { - if (span) { - addNonEnumerableProperty(span, ISOLATION_SCOPE_ON_START_SPAN_FIELD, isolationScope); - addNonEnumerableProperty(span, SCOPE_ON_START_SPAN_FIELD, scope); - } +function setCapturedScopesOnTransaction(span: Span, scope: Scope, isolationScope: Scope): void { + addNonEnumerableProperty(span, ISOLATION_SCOPE_ON_START_SPAN_FIELD, isolationScope); + addNonEnumerableProperty(span, SCOPE_ON_START_SPAN_FIELD, scope); } diff --git a/packages/opentelemetry/test/integration/transactions.test.ts b/packages/opentelemetry/test/integration/transactions.test.ts index 99d46abad4f9..38316b15f264 100644 --- a/packages/opentelemetry/test/integration/transactions.test.ts +++ b/packages/opentelemetry/test/integration/transactions.test.ts @@ -473,8 +473,8 @@ describe('Integration | Transactions', () => { } const subSpan = startInactiveSpan({ name: 'inner span 1' }); - innerSpan1Id = subSpan?.spanContext().spanId; - subSpan?.end(); + innerSpan1Id = subSpan.spanContext().spanId; + subSpan.end(); startSpan({ name: 'inner span 2' }, innerSpan => { if (!innerSpan) { diff --git a/packages/opentelemetry/test/trace.test.ts b/packages/opentelemetry/test/trace.test.ts index 2a6b9f6c3426..ed2900541587 100644 --- a/packages/opentelemetry/test/trace.test.ts +++ b/packages/opentelemetry/test/trace.test.ts @@ -535,18 +535,17 @@ describe('trace', () => { const initialScope = getCurrentScope(); let manualScope: Scope; - let parentSpan: Span; - startSpanManual({ name: 'detached' }, span => { - parentSpan = span; + const parentSpan = startSpanManual({ name: 'detached' }, span => { manualScope = getCurrentScope(); manualScope.setTag('manual', 'tag'); + return span; }); getCurrentScope().setTag('outer', 'tag'); const span = startInactiveSpan({ name: 'GET users/[id]', scope: manualScope! }); - expect(getSpanParentSpanId(span)).toBe(parentSpan!.spanContext().spanId); + expect(getSpanParentSpanId(span)).toBe(parentSpan.spanContext().spanId); expect(getCurrentScope()).toBe(initialScope); expect(getActiveSpan()).toBe(undefined); @@ -675,7 +674,7 @@ describe('trace', () => { client.getOptions().beforeSendTransaction = beforeSendTransaction; - let span: Span | undefined; + let span: Span; const scope = getCurrentScope(); scope.setTag('outer', 'foo'); @@ -688,7 +687,7 @@ describe('trace', () => { withScope(scope => { scope.setTag('scope', 2); - span?.end(); + span.end(); }); await client.flush(); @@ -835,13 +834,13 @@ describe('trace', () => { startSpanManual({ name: 'inner transaction', forceTransaction: true }, span => { startSpanManual({ name: 'inner span 2' }, span => { // all good - span?.end(); + span.end(); }); - span?.end(); + span.end(); }); - span?.end(); + span.end(); }); - span?.end(); + span.end(); }); await client.flush(); diff --git a/packages/opentelemetry/test/utils/setupEventContextTrace.test.ts b/packages/opentelemetry/test/utils/setupEventContextTrace.test.ts index 5d074a1c4ea2..029c25ad968d 100644 --- a/packages/opentelemetry/test/utils/setupEventContextTrace.test.ts +++ b/packages/opentelemetry/test/utils/setupEventContextTrace.test.ts @@ -72,11 +72,11 @@ describe('setupEventContextTrace', () => { let traceId: string | undefined; client.tracer.startActiveSpan('outer', outerSpan => { - outerId = outerSpan?.spanContext().spanId; - traceId = outerSpan?.spanContext().traceId; + outerId = outerSpan.spanContext().spanId; + traceId = outerSpan.spanContext().traceId; client.tracer.startActiveSpan('inner', innerSpan => { - innerId = innerSpan?.spanContext().spanId; + innerId = innerSpan.spanContext().spanId; captureException(error); }); }); diff --git a/packages/serverless/src/awsservices.ts b/packages/serverless/src/awsservices.ts index 33e4816ac836..fb241e0e9638 100644 --- a/packages/serverless/src/awsservices.ts +++ b/packages/serverless/src/awsservices.ts @@ -64,10 +64,10 @@ function wrapMakeRequest( orig: MakeRequestFunction, ): MakeRequestFunction { return function (this: TService, operation: string, params?: GenericParams, callback?: MakeRequestCallback) { - let span: Span | undefined; const req = orig.call(this, operation, params); if (SETUP_CLIENTS.has(getClient() as Client)) { + let span: Span | undefined; req.on('afterBuild', () => { span = startInactiveSpan({ name: describe(this, operation, params), @@ -79,9 +79,7 @@ function wrapMakeRequest( }); }); req.on('complete', () => { - if (span) { - span.end(); - } + span?.end(); }); } diff --git a/packages/serverless/src/gcpfunction/cloud_events.ts b/packages/serverless/src/gcpfunction/cloud_events.ts index 3d5ac9d3d1dc..e24332c19f35 100644 --- a/packages/serverless/src/gcpfunction/cloud_events.ts +++ b/packages/serverless/src/gcpfunction/cloud_events.ts @@ -48,7 +48,7 @@ function _wrapCloudEventFunction( if (args[0] !== null && args[0] !== undefined) { captureException(args[0], scope => markEventUnhandled(scope)); } - span?.end(); + span.end(); // eslint-disable-next-line @typescript-eslint/no-floating-promises flush(options.flushTimeout) diff --git a/packages/serverless/src/gcpfunction/events.ts b/packages/serverless/src/gcpfunction/events.ts index 3503a427ee1f..9f6fc6ad2a43 100644 --- a/packages/serverless/src/gcpfunction/events.ts +++ b/packages/serverless/src/gcpfunction/events.ts @@ -51,7 +51,7 @@ function _wrapEventFunction if (args[0] !== null && args[0] !== undefined) { captureException(args[0], scope => markEventUnhandled(scope)); } - span?.end(); + span.end(); // eslint-disable-next-line @typescript-eslint/no-floating-promises flush(options.flushTimeout) diff --git a/packages/serverless/src/gcpfunction/http.ts b/packages/serverless/src/gcpfunction/http.ts index 17ee38d18e0e..724fa2f4faf3 100644 --- a/packages/serverless/src/gcpfunction/http.ts +++ b/packages/serverless/src/gcpfunction/http.ts @@ -73,10 +73,8 @@ function _wrapHttpFunction(fn: HttpFunction, options: Partial): const _end = res.end; // eslint-disable-next-line @typescript-eslint/no-explicit-any res.end = function (chunk?: any | (() => void), encoding?: string | (() => void), cb?: () => void): any { - if (span) { - setHttpStatus(span, res.statusCode); - span.end(); - } + setHttpStatus(span, res.statusCode); + span.end(); // eslint-disable-next-line @typescript-eslint/no-floating-promises flush(flushTimeout) diff --git a/packages/serverless/src/google-cloud-http.ts b/packages/serverless/src/google-cloud-http.ts index 78abb2f97ff1..9c50cccab5a1 100644 --- a/packages/serverless/src/google-cloud-http.ts +++ b/packages/serverless/src/google-cloud-http.ts @@ -1,6 +1,7 @@ import type * as common from '@google-cloud/common'; import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SentryNonRecordingSpan, convertIntegrationFnToClass, defineIntegration, getClient, @@ -70,11 +71,9 @@ function wrapRequestFunction(orig: RequestFunction): RequestFunction { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.serverless', }, }) - : undefined; + : new SentryNonRecordingSpan(); orig.call(this, reqOpts, (...args: Parameters) => { - if (span) { - span.end(); - } + span.end(); callback(...args); }); }; diff --git a/packages/sveltekit/src/server/handle.ts b/packages/sveltekit/src/server/handle.ts index f37742b2f09b..90909ea2c0a2 100644 --- a/packages/sveltekit/src/server/handle.ts +++ b/packages/sveltekit/src/server/handle.ts @@ -11,7 +11,6 @@ import { import { getActiveTransaction, startSpan } from '@sentry/core'; import { captureException } from '@sentry/node-experimental'; /* eslint-disable @sentry-internal/sdk/no-optional-chaining */ -import type { Span } from '@sentry/types'; import { dynamicSamplingContextToSentryBaggageHeader, objectify } from '@sentry/utils'; import type { Handle, ResolveOptions } from '@sveltejs/kit'; @@ -188,13 +187,11 @@ async function instrumentHandle( dynamicSamplingContext: traceparentData && !dynamicSamplingContext ? {} : dynamicSamplingContext, }, }, - async (span?: Span) => { + async span => { const res = await resolve(event, { transformPageChunk: addSentryCodeToPage(options), }); - if (span) { - setHttpStatus(span, res.status); - } + setHttpStatus(span, res.status); return res; }, ); diff --git a/packages/sveltekit/test/server/handle.test.ts b/packages/sveltekit/test/server/handle.test.ts index fbaf0f1e0581..5653623c31ad 100644 --- a/packages/sveltekit/test/server/handle.test.ts +++ b/packages/sveltekit/test/server/handle.test.ts @@ -215,7 +215,7 @@ describe('handleSentry', () => { // } - expect(_span!).toBeDefined(); + expect(_span).toBeDefined(); expect(_span!.spanContext().traceId).toEqual('1234567890abcdef1234567890abcdef'); expect(spanToJSON(_span!).parent_span_id).toEqual('1234567890abcdef'); expect(spanIsSampled(_span!)).toEqual(true); diff --git a/packages/tracing-internal/src/browser/request.ts b/packages/tracing-internal/src/browser/request.ts index bf63558665fe..b808136b0325 100644 --- a/packages/tracing-internal/src/browser/request.ts +++ b/packages/tracing-internal/src/browser/request.ts @@ -1,5 +1,6 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SentryNonRecordingSpan, getClient, getCurrentScope, getDynamicSamplingContextFromClient, @@ -320,12 +321,10 @@ export function xhrCallback( }, op: 'http.client', }) - : undefined; + : new SentryNonRecordingSpan(); - if (span) { - xhr.__sentry_xhr_span_id__ = span.spanContext().spanId; - spans[xhr.__sentry_xhr_span_id__] = span; - } + xhr.__sentry_xhr_span_id__ = span.spanContext().spanId; + spans[xhr.__sentry_xhr_span_id__] = span; const client = getClient(); diff --git a/packages/tracing-internal/test/browser/backgroundtab.test.ts b/packages/tracing-internal/test/browser/backgroundtab.test.ts index bb90927ec099..d44f08621ddc 100644 --- a/packages/tracing-internal/test/browser/backgroundtab.test.ts +++ b/packages/tracing-internal/test/browser/backgroundtab.test.ts @@ -50,7 +50,7 @@ describe('registerBackgroundTabDetection', () => { global.document.hidden = true; events.visibilitychange(); - const { status, timestamp, data } = spanToJSON(span!); + const { status, timestamp, data } = spanToJSON(span); expect(status).toBe('cancelled'); expect(status).toBeDefined(); diff --git a/packages/tracing-internal/test/browser/browserTracingIntegration.test.ts b/packages/tracing-internal/test/browser/browserTracingIntegration.test.ts index 1e29fa8528b6..cc0d96a9c98f 100644 --- a/packages/tracing-internal/test/browser/browserTracingIntegration.test.ts +++ b/packages/tracing-internal/test/browser/browserTracingIntegration.test.ts @@ -264,7 +264,7 @@ describe('browserTracingIntegration', () => { const childSpan = startInactiveSpan({ name: 'pageload-child' }); const timestamp = timestampInSeconds(); - childSpan?.end(timestamp); + childSpan.end(timestamp); pageloadSpan?.end(timestamp + 12345); expect(spanToJSON(pageloadSpan!).timestamp).toBe(timestamp);