8000 feat(core): Deprecate `Span.status` (#10208) · getsentry/sentry-javascript@64ba9ec · GitHub
[go: up one dir, main page]
Skip to content

Commit 64ba9ec

Browse files
Lms24Lms24
authored
feat(core): Deprecate Span.status (#10208)
This PR deprecates the `status` field on the span interface as well as on the class. The replacements for this field are * `span.setStatus` to set or update a span status (this API exists on the Otel Span interface but the types don't align yet) * `spanToJson` to read the status ref #10184
1 parent 03a3d93 commit 64ba9ec

File tree

9 files changed

+80
-22
lines changed

9 files changed

+80
-22
lines changed

‎MIGRATION.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ In v8, the Span class is heavily reworked. The following properties & methods ar
204204
- `span.instrumenter` This field was removed and will be replaced internally.
205205
- `span.transaction`: Use `getRootSpan` utility function instead.
206206
- `span.spanRecorder`: Span recording will be handled internally by the SDK.
207+
- `span.status`: Use `.setStatus` to set or update and `spanToJSON()` to read the span status.
207208
- `transaction.setMetadata()`: Use attributes instead, or set data on the scope.
208209
- `transaction.metadata`: Use attributes instead, or set data on the scope.
209210
- `transaction.setContext()`: Set context on the surrounding scope instead.

‎packages/core/src/tracing/span.ts

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,6 @@ export class Span implements SpanInterface {
6767
*/
6868
public parentSpanId?: string;
6969

70-
/**
71-
* Internal keeper of the status
72-
*/
73-
public status?: SpanStatusType | string;
74-
7570
/**
7671
* @inheritDoc
7772
*/
@@ -128,6 +123,8 @@ export class Span implements SpanInterface {
128123
protected _startTime: number;
129124
/** Epoch timestamp in seconds when the span ended. */
130125
protected _endTime?: number;
126+
/** Internal keeper of the status */
127+
protected _status?: SpanStatusType | string;
131128

132129
private _logMessage?: string;
133130

@@ -164,7 +161,7 @@ export class Span implements SpanInterface {
164161
this.op = spanContext.op;
165162
}
166163
if (spanContext.status) {
167-
this.status = spanContext.status;
164+
this._status = spanContext.status;
168165
}
169166
if (spanContext.endTimestamp) {
170167
this._endTime = spanContext.endTimestamp;
@@ -302,6 +299,24 @@ export class Span implements SpanInterface {
302299
this._endTime = endTime;
303300
}
304301

302+
/**
303+
* The status of the span.
304+
*
305+
* @deprecated Use `spanToJSON().status` instead to get the status.
306+
*/
307+
public get status(): SpanStatusType | string | undefined {
308+
return this._status;
309+
}
310+
311+
/**
312+
* The status of the span.
313+
*
314+
* @deprecated Use `.setStatus()` instead to set or update the status.
315+
*/
316+
public set status(status: SpanStatusType | string | undefined) {
317+
this._status = status;
318+
}
319+
305320
/* eslint-enable @typescript-eslint/member-ordering */
306321

307322
/** @inheritdoc */
@@ -404,7 +419,7 @@ export class Span implements SpanInterface {
404419
* @inheritDoc
405420
*/
406421
public setStatus(value: SpanStatusType): this {
407-
this.status = value;
422+
this._status = value;
408423
return this;
409424
}
410425

@@ -444,7 +459,7 @@ export class Span implements SpanInterface {
444459
* @inheritDoc
445460
*/
446461
public isSuccess(): boolean {
447-
return this.status === 'ok';
462+
return this._status === 'ok';
448463
}
449464

450465
/**
@@ -502,7 +517,7 @@ export class Span implements SpanInterface {
502517
sampled: this._sampled,
503518
spanId: this._spanId,
504519
startTimestamp: this._startTime,
505-
status: this.status,
520+
status: this._status,
506521
// eslint-disable-next-line deprecation/deprecation
507522
tags: this.tags,
508523
traceId: this._traceId,
@@ -525,7 +540,7 @@ export class Span implements SpanInterface {
525540
this._sampled = spanContext.sampled;
526541
this._spanId = spanContext.spanId || this._spanId;
527542
this._startTime = spanContext.startTimestamp || this._startTime;
528-
this.status = spanContext.status;
543+
this._status = spanContext.status;
529544
// eslint-disable-next-line deprecation/deprecation
530545
this.tags = spanContext.tags || {};
531546
this._traceId = spanContext.traceId || this._traceId;
@@ -558,7 +573,7 @@ export class Span implements SpanInterface {
558573
parent_span_id: this.parentSpanId,
559574
span_id: this._spanId,
560575
start_timestamp: this._startTime,
561-
status: this.status,
576+
status: this._status,
562577
// eslint-disable-next-line deprecation/deprecation
563578
tags: Object.keys(this.tags).length > 0 ? this.tags : undefined,
564579
timestamp: this._endTime,

‎packages/core/src/tracing/trace.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import type { Hub } from '../hub';
1818
import { getCurrentHub } from '../hub';
1919
import { handleCallbackErrors } from '../utils/handleCallbackErrors';
2020
import { hasTracingEnabled } from '../utils/hasTracingEnabled';
21-
import { spanTimeInputToSeconds } from '../utils/spanUtils';
21+
import { spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils';
2222

2323
interface StartSpanOptions extends TransactionContext {
2424
/** A manually specified start time for the created `Span` object. */
@@ -196,8 +196,11 @@ export function startSpan<T>(context: StartSpanOptions, callback: (span: Span |
196196
() => callback(activeSpan),
197197
() => {
198198
// Only update the span status if it hasn't been changed yet
199-
if (activeSpan && (!activeSpan.status || activeSpan.status === 'ok')) {
200-
activeSpan.setStatus('internal_error');
199+
if (activeSpan) {
200+
const { status } = spanToJSON(activeSpan);
201+
if (!status || status === 'ok') {
202+
activeSpan.setStatus('internal_error');
203+
}
201204
}
202205
},
203206
() => activeSpan && activeSpan.end(),
@@ -245,8 +248,11 @@ export function startSpanManual<T>(
245248
() => callback(activeSpan, finishAndSetSpan),
246249
() => {
247250
// Only update the span status if it hasn't been changed yet, and the span is not yet finished
248-
if (activeSpan && activeSpan.isRecording() && (!activeSpan.status || activeSpan.status === 'ok')) {
249-
activeSpan.setStatus('internal_error');
251+
if (activeSpan && activeSpan.isRecording()) {
252+
const { status } = spanToJSON(activeSpan);
253+
if (!status || status === 'ok') {
254+
activeSpan.setStatus('internal_error');
255+
}
250256
}
251257
},
252258
);

‎packages/core/test/lib/tracing/errors.test.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { BrowserClient } from '@sentry/browser';
2-
import { Hub, addTracingExtensions, makeMain, startInactiveSpan, startSpan } from '@sentry/core';
2+
import { Hub, addTracingExtensions, makeMain, spanToJSON, startInactiveSpan, startSpan } from '@sentry/core';
33
import type { HandlerDataError, HandlerDataUnhandledRejection } from '@sentry/types';
44

55
import { getDefaultBrowserClientOptions } from '../../../../tracing/test/testutils';
@@ -51,13 +51,20 @@ describe('registerErrorHandlers()', () => {
5151
registerErrorInstrumentation();
5252

5353
const transaction = startInactiveSpan({ name: 'test' })!;
54+
// eslint-disable-next-line deprecation/deprecation
5455
expect(transaction.status).toBe(undefined);
56+
expect(spanToJSON(transaction).status).toBe(undefined);
5557

5658
mockErrorCallback({} as HandlerDataError);
59+
// eslint-disable-next-line deprecation/deprecation
5760
expect(transaction.status).toBe(undefined);
61+
expect(spanToJSON(transaction).status).toBe(undefined);
5862

5963
mockUnhandledRejectionCallback({});
64+
// eslint-disable-next-line deprecation/deprecation
6065
expect(transaction.status).toBe(undefined);
66+
expect(spanToJSON(transaction).status).toBe(undefined);
67+
6168
transaction.end();
6269
});
6370

@@ -66,7 +73,9 @@ describe('registerErrorHandlers()', () => {
6673

6774
startSpan({ name: 'test' }, span => {
6875
mockErrorCallback({} as HandlerDataError);
69-
expect(span?.status).toBe('internal_error');
76+
// eslint-disable-next-line deprecation/deprecation
77+
expect(span!.status).toBe('internal_error');
78+
expect(spanToJSON(span!).status).toBe('internal_error');
7079
});
7180
});
7281

@@ -75,7 +84,9 @@ describe('registerErrorHandlers()', () => {
7584

7685
startSpan({ name: 'test' }, span => {
7786
mockUnhandledRejectionCallback({});
78-
expect(span?.status).toBe('internal_error');
87+
// eslint-disable-next-line deprecation/deprecation
88+
expect(span!.status).toBe('internal_error');
89+
expect(spanToJSON(span!).status).toBe('internal_error');
7990
});
8091
});
8192
});

‎packages/node/test/handlers.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,9 @@ describe('tracingHandler', () => {
382382

383383
setImmediate(() => {
384384
expect(finishTransaction).toHaveBeenCalled();
385+
// eslint-disable-next-line deprecation/deprecation
385386
expect(transaction.status).toBe('ok');
387+
expect(spanToJSON(transaction).status).toBe('ok');
386388
// eslint-disable-next-line deprecation/deprecation
387389
expect(transaction.tags).toEqual(expect.objectContaining({ 'http.status_code': '200' }));
388390
expect(spanToJSON(transaction).data).toEqual(expect.objectContaining({ 'http.response.status_code': 200 }));

‎packages/opentelemetry-node/test/spanprocessor.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,11 +344,15 @@ describe('SentrySpanProcessor', () => {
344344
const transaction = getSpanForOtelSpan(otelSpan) as Transaction;
345345

346346
// status is only set after end
347+
// eslint-disable-next-line deprecation/deprecation
347348
expect(transaction?.status).toBe(undefined);
349+
expect(spanToJSON(transaction!).status).toBe(undefined);
348350

349351
otelSpan.end();
350352

353+
// eslint-disable-next-line deprecation/deprecation
351354
expect(transaction?.status).toBe('ok');
355+
expect(spanToJSON(transaction!).status).toBe('ok');
352356
});
353357

354358
it('sets status for span', async () => {
@@ -358,11 +362,15 @@ describe('SentrySpanProcessor', () => {
358362
tracer.startActiveSpan('SELECT * FROM users;', child => {
359363
const sentrySpan = getSpanForOtelSpan(child);
360364

365+
// eslint-disable-next-line deprecation/deprecation
361366
expect(sentrySpan?.status).toBe(undefined);
367+
expect(spanToJSON(sentrySpan!).status).toBe(undefined);
362368

363369
child.end();
364370

371+
// eslint-disable-next-line deprecation/deprecation
365372
expect(sentrySpan?.status).toBe('ok');
373+
expect(spanToJSON(sentrySpan!).status).toBe('ok');
366374

367375
parentOtelSpan.end();
368376
});
@@ -444,7 +452,9 @@ describe('SentrySpanProcessor', () => {
444452
}
445453

446454
otelSpan.end();
455+
// eslint-disable-next-line deprecation/deprecation
447456
expect(transaction?.status).toBe(expected);
457+
expect(spanToJSON(transaction!).status).toBe(expected);
448458
},
449459
);
450460
});

‎packages/tracing-internal/src/browser/backgroundtab.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { IdleTransaction, SpanStatusType } from '@sentry/core';
2-
import { getActiveTransaction } from '@sentry/core';
2+
import { getActiveTransaction, spanToJSON } from '@sentry/core';
33
import { logger } from '@sentry/utils';
44

55
import { DEBUG_BUILD } from '../common/debug-build';
@@ -23,7 +23,7 @@ export function registerBackgroundTabDetection(): void {
2323
);
2424
// We should not set status if it is already set, this prevent important statuses like
2525
// error or data loss from being overwritten on transaction.
26-
if (!activeTransaction.status) {
26+
if (!spanToJSON(activeTransaction).status) {
2727
activeTransaction.setStatus(statusType);
2828
}
2929
// TODO: Can we rewrite this to an attribute?

‎packages/tracing-internal/test/browser/backgroundtab.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,14 @@ conditionalTest({ min: 10 })('registerBackgroundTabDetection', () => {
5555
global.document.hidden = true;
5656
events.visibilitychange();
5757

58+
const { status, timestamp } = spanToJSON(span!);
59+
60+
// eslint-disable-next-line deprecation/deprecation
5861
expect(span?.status).toBe('cancelled');
62+
expect(status).toBeDefined();
5963
// eslint-disable-next-line deprecation/deprecation
6064
expect(span?.tags.visibilitychange).toBe('document.hidden');
61-
expect(spanToJSON(span!).timestamp).toBeDefined();
65+
expect(timestamp).toBeDefined();
6266
});
6367
});
6468
});

‎packages/types/src/span.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,15 @@ export interface Span extends SpanContext {
234234
*/
235235
instrumenter: Instrumenter;
236236

237+
/**
238+
* Completion status of the Span.
239+
*
240+
* See: {@sentry/tracing SpanStatus} for possible values
241+
*
242+
* @deprecated Use `.setStatus` to set or update and `spanToJSON()` to read the status.
243+
*/
244+
status?: string;
245+
237246
/**
238247
* Get context data for this span.
239248
* This includes the spanId & the traceId.

0 commit comments

Comments
 (0)
0