diff --git a/MIGRATION.md b/MIGRATION.md index 49be1f12de03..363c8a37d46d 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -186,6 +186,7 @@ In v8, the Span class is heavily reworked. The following properties & methods ar - `span.instrumenter` This field was removed and will be replaced internally. - `span.transaction`: Use `getRootSpan` utility function instead. - `span.spanRecorder`: Span recording will be handled internally by the SDK. +- `span.status`: Use `.setStatus` to set or update and `spanToJSON()` to read the span status. - `transaction.setMetadata()`: Use attributes instead, or set data on the scope. - `transaction.metadata`: Use attributes instead, or set data on the scope. - `transaction.setContext()`: Set context on the surrounding scope instead. diff --git a/packages/core/src/tracing/span.ts b/packages/core/src/tracing/span.ts index 2d6037aff9f3..3253f1580148 100644 --- a/packages/core/src/tracing/span.ts +++ b/packages/core/src/tracing/span.ts @@ -67,11 +67,6 @@ export class Span implements SpanInterface { */ public parentSpanId?: string; - /** - * Internal keeper of the status - */ - public status?: SpanStatusType | string; - /** * @inheritDoc */ @@ -128,6 +123,8 @@ export class Span implements SpanInterface { protected _startTime: number; /** Epoch timestamp in seconds when the span ended. */ protected _endTime?: number; + /** Internal keeper of the status */ + protected _status?: SpanStatusType | string; private _logMessage?: string; @@ -164,7 +161,7 @@ export class Span implements SpanInterface { this.op = spanContext.op; } if (spanContext.status) { - this.status = spanContext.status; + this._status = spanContext.status; } if (spanContext.endTimestamp) { this._endTime = spanContext.endTimestamp; @@ -302,6 +299,24 @@ export class Span implements SpanInterface { this._endTime = endTime; } + /** + * The status of the span. + * + * @deprecated Use `spanToJSON().status` instead to get the status. + */ + public get status(): SpanStatusType | string | undefined { + return this._status; + } + + /** + * The status of the span. + * + * @deprecated Use `.setStatus()` instead to set or update the status. + */ + public set status(status: SpanStatusType | string | undefined) { + this._status = status; + } + /* eslint-enable @typescript-eslint/member-ordering */ /** @inheritdoc */ @@ -404,7 +419,7 @@ export class Span implements SpanInterface { * @inheritDoc */ public setStatus(value: SpanStatusType): this { - this.status = value; + this._status = value; return this; } @@ -444,7 +459,7 @@ export class Span implements SpanInterface { * @inheritDoc */ public isSuccess(): boolean { - return this.status === 'ok'; + return this._status === 'ok'; } /** @@ -502,7 +517,7 @@ export class Span implements SpanInterface { sampled: this._sampled, spanId: this._spanId, startTimestamp: this._startTime, - status: this.status, + status: this._status, // eslint-disable-next-line deprecation/deprecation tags: this.tags, traceId: this._traceId, @@ -525,7 +540,7 @@ export class Span implements SpanInterface { this._sampled = spanContext.sampled; this._spanId = spanContext.spanId || this._spanId; this._startTime = spanContext.startTimestamp || this._startTime; - this.status = spanContext.status; + this._status = spanContext.status; // eslint-disable-next-line deprecation/deprecation this.tags = spanContext.tags || {}; this._traceId = spanContext.traceId || this._traceId; @@ -558,7 +573,7 @@ export class Span implements SpanInterface { parent_span_id: this.parentSpanId, span_id: this._spanId, start_timestamp: this._startTime, - status: this.status, + status: this._status, // eslint-disable-next-line deprecation/deprecation tags: Object.keys(this.tags).length > 0 ? this.tags : undefined, timestamp: this._endTime, diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts index 103c52ea9c0b..eb070510a3b7 100644 --- a/packages/core/src/tracing/trace.ts +++ b/packages/core/src/tracing/trace.ts @@ -18,7 +18,7 @@ import type { Hub } from '../hub'; import { getCurrentHub } from '../hub'; import { handleCallbackErrors } from '../utils/handleCallbackErrors'; import { hasTracingEnabled } from '../utils/hasTracingEnabled'; -import { spanTimeInputToSeconds } from '../utils/spanUtils'; +import { spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils'; interface StartSpanOptions extends TransactionContext { /** A manually specified start time for the created `Span` object. */ @@ -196,8 +196,11 @@ export function startSpan(context: StartSpanOptions, callback: (span: Span | () => callback(activeSpan), () => { // Only update the span status if it hasn't been changed yet - if (activeSpan && (!activeSpan.status || activeSpan.status === 'ok')) { - activeSpan.setStatus('internal_error'); + if (activeSpan) { + const { status } = spanToJSON(activeSpan); + if (!status || status === 'ok') { + activeSpan.setStatus('internal_error'); + } } }, () => activeSpan && activeSpan.end(), @@ -245,8 +248,11 @@ export function startSpanManual( () => callback(activeSpan, finishAndSetSpan), () => { // Only update the span status if it hasn't been changed yet, and the span is not yet finished - if (activeSpan && activeSpan.isRecording() && (!activeSpan.status || activeSpan.status === 'ok')) { - activeSpan.setStatus('internal_error'); + if (activeSpan && activeSpan.isRecording()) { + const { status } = spanToJSON(activeSpan); + if (!status || status === 'ok') { + activeSpan.setStatus('internal_error'); + } } }, ); diff --git a/packages/core/test/lib/tracing/errors.test.ts b/packages/core/test/lib/tracing/errors.test.ts index b1711f860679..b83820865ede 100644 --- a/packages/core/test/lib/tracing/errors.test.ts +++ b/packages/core/test/lib/tracing/errors.test.ts @@ -1,5 +1,5 @@ import { BrowserClient } from '@sentry/browser'; -import { Hub, addTracingExtensions, makeMain, startInactiveSpan, startSpan } from '@sentry/core'; +import { Hub, addTracingExtensions, makeMain, spanToJSON, startInactiveSpan, startSpan } from '@sentry/core'; import type { HandlerDataError, HandlerDataUnhandledRejection } from '@sentry/types'; import { getDefaultBrowserClientOptions } from '../../../../tracing/test/testutils'; @@ -51,13 +51,20 @@ describe('registerErrorHandlers()', () => { registerErrorInstrumentation(); const transaction = startInactiveSpan({ name: 'test' })!; + // eslint-disable-next-line deprecation/deprecation expect(transaction.status).toBe(undefined); + expect(spanToJSON(transaction).status).toBe(undefined); mockErrorCallback({} as HandlerDataError); + // eslint-disable-next-line deprecation/deprecation expect(transaction.status).toBe(undefined); + expect(spanToJSON(transaction).status).toBe(undefined); mockUnhandledRejectionCallback({}); + // eslint-disable-next-line deprecation/deprecation expect(transaction.status).toBe(undefined); + expect(spanToJSON(transaction).status).toBe(undefined); + transaction.end(); }); @@ -66,7 +73,9 @@ describe('registerErrorHandlers()', () => { startSpan({ name: 'test' }, span => { mockErrorCallback({} as HandlerDataError); - expect(span?.status).toBe('internal_error'); + // eslint-disable-next-line deprecation/deprecation + expect(span!.status).toBe('internal_error'); + expect(spanToJSON(span!).status).toBe('internal_error'); }); }); @@ -75,7 +84,9 @@ describe('registerErrorHandlers()', () => { startSpan({ name: 'test' }, span => { mockUnhandledRejectionCallback({}); - expect(span?.status).toBe('internal_error'); + // eslint-disable-next-line deprecation/deprecation + expect(span!.status).toBe('internal_error'); + expect(spanToJSON(span!).status).toBe('internal_error'); }); }); }); diff --git a/packages/node/test/handlers.test.ts b/packages/node/test/handlers.test.ts index 87e2ff9768da..d9001f6ffdd9 100644 --- a/packages/node/test/handlers.test.ts +++ b/packages/node/test/handlers.test.ts @@ -382,7 +382,9 @@ describe('tracingHandler', () => { setImmediate(() => { expect(finishTransaction).toHaveBeenCalled(); + // eslint-disable-next-line deprecation/deprecation expect(transaction.status).toBe('ok'); + expect(spanToJSON(transaction).status).toBe('ok'); // eslint-disable-next-line deprecation/deprecation expect(transaction.tags).toEqual(expect.objectContaining({ 'http.status_code': '200' })); expect(spanToJSON(transaction).data).toEqual(expect.objectContaining({ 'http.response.status_code': 200 })); diff --git a/packages/opentelemetry-node/test/spanprocessor.test.ts b/packages/opentelemetry-node/test/spanprocessor.test.ts index 305035490ea7..08cc38db0011 100644 --- a/packages/opentelemetry-node/test/spanprocessor.test.ts +++ b/packages/opentelemetry-node/test/spanprocessor.test.ts @@ -344,11 +344,15 @@ describe('SentrySpanProcessor', () => { const transaction = getSpanForOtelSpan(otelSpan) as Transaction; // status is only set after end + // eslint-disable-next-line deprecation/deprecation expect(transaction?.status).toBe(undefined); + expect(spanToJSON(transaction!).status).toBe(undefined); otelSpan.end(); + // eslint-disable-next-line deprecation/deprecation expect(transaction?.status).toBe('ok'); + expect(spanToJSON(transaction!).status).toBe('ok'); }); it('sets status for span', async () => { @@ -358,11 +362,15 @@ describe('SentrySpanProcessor', () => { tracer.startActiveSpan('SELECT * FROM users;', child => { const sentrySpan = getSpanForOtelSpan(child); + // eslint-disable-next-line deprecation/deprecation expect(sentrySpan?.status).toBe(undefined); + expect(spanToJSON(sentrySpan!).status).toBe(undefined); child.end(); + // eslint-disable-next-line deprecation/deprecation expect(sentrySpan?.status).toBe('ok'); + expect(spanToJSON(sentrySpan!).status).toBe('ok'); parentOtelSpan.end(); }); @@ -444,7 +452,9 @@ describe('SentrySpanProcessor', () => { } otelSpan.end(); + // eslint-disable-next-line deprecation/deprecation expect(transaction?.status).toBe(expected); + expect(spanToJSON(transaction!).status).toBe(expected); }, ); }); diff --git a/packages/tracing-internal/src/browser/backgroundtab.ts b/packages/tracing-internal/src/browser/backgroundtab.ts index 849f20ad20de..5d9fb29cd08e 100644 --- a/packages/tracing-internal/src/browser/backgroundtab.ts +++ b/packages/tracing-internal/src/browser/backgroundtab.ts @@ -1,5 +1,5 @@ import type { IdleTransaction, SpanStatusType } from '@sentry/core'; -import { getActiveTransaction } from '@sentry/core'; +import { getActiveTransaction, spanToJSON } from '@sentry/core'; import { logger } from '@sentry/utils'; import { DEBUG_BUILD } from '../common/debug-build'; @@ -23,7 +23,7 @@ export function registerBackgroundTabDetection(): void { ); // We should not set status if it is already set, this prevent important statuses like // error or data loss from being overwritten on transaction. - if (!activeTransaction.status) { + if (!spanToJSON(activeTransaction).status) { activeTransaction.setStatus(statusType); } // TODO: Can we rewrite this to an attribute? diff --git a/packages/tracing-internal/test/browser/backgroundtab.test.ts b/packages/tracing-internal/test/browser/backgroundtab.test.ts index 6c7ea89d982f..27cba1d934fa 100644 --- a/packages/tracing-internal/test/browser/backgroundtab.test.ts +++ b/packages/tracing-internal/test/browser/backgroundtab.test.ts @@ -55,10 +55,14 @@ conditionalTest({ min: 10 })('registerBackgroundTabDetection', () => { global.document.hidden = true; events.visibilitychange(); + const { status, timestamp } = spanToJSON(span!); + + // eslint-disable-next-line deprecation/deprecation expect(span?.status).toBe('cancelled'); + expect(status).toBeDefined(); // eslint-disable-next-line deprecation/deprecation expect(span?.tags.visibilitychange).toBe('document.hidden'); - expect(spanToJSON(span!).timestamp).toBeDefined(); + expect(timestamp).toBeDefined(); }); }); }); diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index 5bbe6408ea30..cc11df4cab83 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -234,6 +234,15 @@ export interface Span extends SpanContext { */ instrumenter: Instrumenter; + /** + * Completion status of the Span. + * + * See: {@sentry/tracing SpanStatus} for possible values + * + * @deprecated Use `.setStatus` to set or update and `spanToJSON()` to read the status. + */ + status?: string; + /** * Get context data for this span. * This includes the spanId & the traceId.