Skip to content

Commit 25a783b

Browse files
mydeamydea
authored
ref(core): Remove status field from Span (#10856)
This removes the public field to get/set `status` on a span/transaction.
1 parent a838c17 commit 25a783b

File tree

17 files changed

+95
-149
lines changed

17 files changed

+95
-149
lines changed

‎packages/bun/test/integrations/bunserver.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ describe('Bun Serve Integration', () => {
2424

2525
test('generates a transaction around a request', async () => {
2626
client.on('finishTransaction', transaction => {
27-
expect(transaction.status).toBe('ok');
27+
expect(spanToJSON(transaction).status).toBe('ok');
2828
expect(spanToJSON(transaction).data?.['http.response.status_code']).toEqual(200);
2929
expect(spanToJSON(transaction).op).toEqual('http.server');
3030
expect(spanToJSON(transaction).description).toEqual('GET /');
@@ -44,7 +44,7 @@ describe('Bun Serve Integration', () => {
4444

4545
test('generates a post transaction', async () => {
4646
client.on('finishTransaction', transaction => {
47-
expect(transaction.status).toBe('ok');
47+
expect(spanToJSON(transaction).status).toBe('ok');
4848
expect(spanToJSON(transaction).data?.['http.response.status_code']).toEqual(200);
4949
expect(spanToJSON(transaction).op).toEqual('http.server');
5050
expect(spanToJSON(transaction).description).toEqual('POST /');

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

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,6 @@ export class SentrySpan implements SpanInterface {
135135
if ('sampled' in spanContext) {
136136
this._sampled = spanContext.sampled;
137137
}
138-
if (spanContext.status) {
139-
this._status = spanContext.status;
140-
}
141138
if (spanContext.endTimestamp) {
142139
this._endTime = spanContext.endTimestamp;
143140
}
@@ -260,24 +257,6 @@ export class SentrySpan implements SpanInterface {
260257
this._endTime = endTime;
261258
}
262259

263-
/**
264-
* The status of the span.
265-
*
266-
* @deprecated Use `spanToJSON().status` instead to get the status.
267-
*/
268-
public get status(): SpanStatusType | string | undefined {
269-
return this._status;
270-
}
271-
272-
/**
273-
* The status of the span.
274-
*
275-
* @deprecated Use `.setStatus()` instead to set or update the status.
276-
*/
277-
public set status(status: SpanStatusType | string | undefined) {
278-
this._status = status;
279-
}
280-
281260
/* eslint-enable @typescript-eslint/member-ordering */
282261

283262
/** @inheritdoc */

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,18 +50,12 @@ describe('registerErrorHandlers()', () => {
5050
registerErrorInstrumentation();
5151

5252
const transaction = startInactiveSpan({ name: 'test' })!;
53-
// eslint-disable-next-line deprecation/deprecation
54-
expect(transaction.status).toBe(undefined);
5553
expect(spanToJSON(transaction).status).toBe(undefined);
5654

5755
mockErrorCallback({} as HandlerDataError);
58-
// eslint-disable-next-line deprecation/deprecation
59-
expect(transaction.status).toBe(undefined);
6056
expect(spanToJSON(transaction).status).toBe(undefined);
6157

6258
mockUnhandledRejectionCallback({});
63-
// eslint-disable-next-line deprecation/deprecation
64-
expect(transaction.status).toBe(undefined);
6559
expect(spanToJSON(transaction).status).toBe(undefined);
6660

6761
transaction.end();
@@ -72,8 +66,6 @@ describe('registerErrorHandlers()', () => {
7266

7367
startSpan({ name: 'test' }, span => {
7468
mockErrorCallback({} as HandlerDataError);
75-
// eslint-disable-next-line deprecation/deprecation
76-
expect(span!.status).toBe('internal_error');
7769
expect(spanToJSON(span!).status).toBe('internal_error');
7870
});
7971
});
@@ -83,8 +75,6 @@ describe('registerErrorHandlers()', () => {
8375

8476
startSpan({ name: 'test' }, span => {
8577
mockUnhandledRejectionCallback({});
86-
// eslint-disable-next-line deprecation/deprecation
87-
expect(span!.status).toBe('internal_error');
8878
expect(spanToJSON(span!).status).toBe('internal_error');
8979
});
9080
});

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -232,12 +232,12 @@ describe('IdleTransaction', () => {
232232

233233
// Regular SentrySpan - should not modified
234234
expect(spans[1].spanContext().spanId).toBe(regularSpan.spanContext().spanId);
235-
expect(spans[1]['_endTime']).not.toBe(spanToJSON(transaction).timestamp);
235+
expect(spanToJSON(spans[1]).timestamp).not.toBe(spanToJSON(transaction).timestamp);
236236

237237
// Cancelled SentrySpan - has endtimestamp of transaction
238238
expect(spans[2].spanContext().spanId).toBe(cancelledSpan.spanContext().spanId);
239-
expect(spans[2].status).toBe('cancelled');
240-
expect(spans[2]['_endTime']).toBe(spanToJSON(transaction).timestamp);
239+
expect(spanToJSON(spans[2]).status).toBe('cancelled');
240+
expect(spanToJSON(spans[2]).timestamp).toBe(spanToJSON(transaction).timestamp);
241241
}
242242
});
243243

@@ -415,22 +415,22 @@ describe('IdleTransaction', () => {
415415
const transaction = new IdleTransaction({ name: 'foo' }, getCurrentHub(), 20000);
416416
const mockFinish = jest.spyOn(transaction, 'end');
417417

418-
expect(transaction.status).not.toEqual('deadline_exceeded');
418+
expect(spanToJSON(transaction).status).not.toEqual('deadline_exceeded');
419419
expect(mockFinish).toHaveBeenCalledTimes(0);
420420

421421
// Beat 1
422422
jest.advanceTimersByTime(TRACING_DEFAULTS.heartbeatInterval);
423-
expect(transaction.status).not.toEqual('deadline_exceeded');
423+
expect(spanToJSON(transaction).status).not.toEqual('deadline_exceeded');
424424
expect(mockFinish).toHaveBeenCalledTimes(0);
425425

426426
// Beat 2
427427
jest.advanceTimersByTime(TRACING_DEFAULTS.heartbeatInterval);
428-
expect(transaction.status).not.toEqual('deadline_exceeded');
428+
expect(spanToJSON(transaction).status).not.toEqual('deadline_exceeded');
429429
expect(mockFinish).toHaveBeenCalledTimes(0);
430430

431431
// Beat 3
432432
jest.advanceTimersByTime(TRACING_DEFAULTS.heartbeatInterval);
433-
expect(transaction.status).not.toEqual('deadline_exceeded');
433+
expect(spanToJSON(transaction).status).not.toEqual('deadline_exceeded');
434434
expect(mockFinish).toHaveBeenCalledTimes(0);
435435
});
436436

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

Lines changed: 38 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { Event, Span as SpanType } from '@sentry/types';
1+
import type { Event, Span } from '@sentry/types';
22
import {
33
SEMANTIC_ATTRIBUTE_SENTRY_OP,
44
addTracingExtensions,
@@ -7,6 +7,7 @@ import {
77
getGlobalScope,
88
getIsolationScope,
99
setCurrentClient,
10+
spanIsSampled,
1011
spanToJSON,
1112
withScope,
1213
} from '../../../src';
@@ -18,6 +19,7 @@ import {
1819
startSpan,
1920
startSpanManual,
2021
} from '../../../src/tracing';
22+
import { getSpanTree } from '../../../src/tracing/utils';
2123
import { TestClient, getDefaultTestClientOptions } from '../../mocks/client';
2224

2325
beforeAll(() => {
@@ -92,9 +94,9 @@ describe('startSpan', () => {
9294
});
9395

9496
it('creates a transaction', async () => {
95-
let ref: any = undefined;
97+
let _span: Span | undefined = undefined;
9698
client.on('finishTransaction', transaction => {
97-
ref = transaction;
99+
_span = transaction;
98100
});
99101
try {
100102
await startSpan({ name: 'GET users/[id]' }, () => {
@@ -103,16 +105,16 @@ describe('startSpan', () => {
103105
} catch (e) {
104106
//
105107
}
106-
expect(ref).toBeDefined();
108+
expect(_span).toBeDefined();
107109

108-
expect(spanToJSON(ref).description).toEqual('GET users/[id]');
109-
expect(ref.status).toEqual(isError ? 'internal_error' : undefined);
110+
expect(spanToJSON(_span!).description).toEqual('GET users/[id]');
111+
expect(spanToJSON(_span!).status).toEqual(isError ? 'internal_error' : undefined);
110112
});
111113

112114
it('allows traceparent information to be overriden', async () => {
113-
let ref: any = undefined;
115+
let _span: Span | undefined = undefined;
114116
client.on('finishTransaction', transaction => {
115-
ref = transaction;
117+
_span = transaction;
116118
});
117119
try {
118120
await startSpan(
@@ -129,17 +131,17 @@ describe('startSpan', () => {
129131
} catch (e) {
130132
//
131133
}
132-
expect(ref).toBeDefined();
134+
expect(_span).toBeDefined();
133135

134-
expect(ref.sampled).toEqual(true);
135-
expect(ref.traceId).toEqual('12345678901234567890123456789012');
136-
expect(ref.parentSpanId).toEqual('1234567890123456');
136+
expect(spanIsSampled(_span!)).toEqual(true);
137+
expect(spanToJSON(_span!).trace_id).toEqual('12345678901234567890123456789012');
138+
expect(spanToJSON(_span!).parent_span_id).toEqual('1234567890123456');
137139
});
138140

139141
it('allows for transaction to be mutated', async () => {
140-
let ref: any = undefined;
142+
let _span: Span | undefined = undefined;
141143
client.on('finishTransaction', transaction => {
142-
ref = transaction;
144+
_span = transaction;
143145
});
144146
try {
145147
await startSpan({ name: 'GET users/[id]' }, span => {
@@ -152,13 +154,13 @@ describe('startSpan', () => {
152154
//
153155
}
154156

155-
expect(spanToJSON(ref).op).toEqual('http.server');
157+
expect(spanToJSON(_span!).op).toEqual('http.server');
156158
});
157159

158160
it('creates a span with correct description', async () => {
159-
let ref: any = undefined;
161+
let _span: Span | undefined = undefined;
160162
client.on('finishTransaction', transaction => {
161-
ref = transaction;
163+
_span = transaction;
162164
});
163165
try {
164166
await startSpan({ name: 'GET users/[id]', parentSampled: true }, () => {
@@ -170,16 +172,19 @@ describe('startSpan', () => {
170172
//
171173
}
172174

173-
expect(ref.spanRecorder.spans).toHaveLength(2);
174-
expect(spanToJSON(ref.spanRecorder.spans[1]).description).toEqual('SELECT * from users');
175-
expect(ref.spanRecorder.spans[1].parentSpanId).toEqual(ref.spanId);
176-
expect(ref.spanRecorder.spans[1].status).toEqual(isError ? 'internal_error' : undefined);
175+
expect(_span).toBeDefined();
176+
const spans = getSpanTree(_span!);
177+
178+
expect(spans).toHaveLength(2);
179+
expect(spanToJSON(spans[1]).description).toEqual('SELECT * from users');
180+
expect(spanToJSON(spans[1]).parent_span_id).toEqual(_span!.spanContext().spanId);
181+
expect(spanToJSON(spans[1]).status).toEqual(isError ? 'internal_error' : undefined);
177182
});
178183

179184
it('allows for span to be mutated', async () => {
180-
let ref: any = undefined;
185+
let _span: Span | undefined = undefined;
181186
client.on('finishTransaction', transaction => {
182-
ref = transaction;
187+
_span = transaction;
183188
});
184189
try {
185190
await startSpan({ name: 'GET users/[id]', parentSampled: true }, () => {
@@ -194,8 +199,11 @@ describe('startSpan', () => {
194199
//
195200
}
196201

197-
expect(ref.spanRecorder.spans).toHaveLength(2);
198-
expect(spanToJSON(ref.spanRecorder.spans[1]).op).toEqual('db.query');
202+
expect(_span).toBeDefined();
203+
const spans = getSpanTree(_span!);
204+
205+
expect(spans).toHaveLength(2);
206+
expect(spanToJSON(spans[1]).op).toEqual('db.query');
199207
});
200208

201209
it.each([
@@ -204,9 +212,9 @@ describe('startSpan', () => {
204212
// attribute should take precedence over top level origin
205213
{ origin: 'manual', attributes: { 'sentry.origin': 'auto.http.browser' } },
206214
])('correctly sets the span origin', async () => {
207-
let ref: any = undefined;
215+
let _span: Span | undefined = undefined;
208216
client.on('finishTransaction', transaction => {
209-
ref = transaction;
217+
_span = transaction;
210218
});
211219
try {
212220
await startSpan({ name: 'GET users/[id]', origin: 'auto.http.browser' }, () => {
@@ -216,7 +224,8 @@ describe('startSpan', () => {
216224
//
217225
}
218226

219-
const jsonSpan = spanToJSON(ref);
227+
expect(_span).toBeDefined();
228+
const jsonSpan = spanToJSON(_span!);
220229
expect(jsonSpan).toEqual({
221230
data: {
222231
'sentry.origin': 'auto.http.browser',
@@ -944,7 +953,7 @@ describe('startInactiveSpan', () => {
944953
setCurrentClient(client);
945954
client.init();
946955

947-
let span: SpanType | undefined;
956+
let span: Span | undefined;
948957

949958
withScope(scope => {
950959
scope.setTag('scope', 1);

‎packages/core/test/lib/utils/spanUtils.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,12 +67,12 @@ describe('spanToJSON', () => {
6767
op: 'test op',
6868
parentSpanId: '1234',
6969
spanId: '5678',
70-
status: 'ok',
7170
traceId: 'abcd',
7271
origin: 'auto',
7372
startTimestamp: 123,
7473
endTimestamp: 456,
7574
});
75+
span.setStatus('ok');
7676

7777
expect(spanToJSON(span)).toEqual({
7878
description: 'test name',

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -407,8 +407,6 @@ describe('tracingHandler', () => {
407407

408408
setImmediate(() => {
409409
expect(finishTransaction).toHaveBeenCalled();
410-
// eslint-disable-next-line deprecation/deprecation
411-
expect(transaction.status).toBe('ok');
412410
expect(spanToJSON(transaction).status).toBe('ok');
413411
expect(spanToJSON(transaction).data).toEqual(expect.objectContaining({ 'http.response.status_code': 200 }));
414412
done();

‎packages/node/test/integrations/undici.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ conditionalTest({ min: 16 })('Undici integration', () => {
150150
expect(spans.length).toBe(2);
151151

152152
const span = spans[1];
153-
expect(span).toEqual(expect.objectContaining({ status: 'internal_error' }));
153+
expect(spanToJSON(span).status).toEqual('internal_error');
154154
});
155155
});
156156

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

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -361,14 +361,10 @@ describe('SentrySpanProcessor', () => {
361361
const transaction = getSpanForOtelSpan(otelSpan) as Transaction;
362362

363363
// status is only set after end
364-
// eslint-disable-next-line deprecation/deprecation
365-
expect(transaction?.status).toBe(undefined);
366364
expect(spanToJSON(transaction!).status).toBe(undefined);
367365

368366
otelSpan.end();
369367

370-
// eslint-disable-next-line deprecation/deprecation
371-
expect(transaction?.status).toBe('ok');
372368
expect(spanToJSON(transaction!).status).toBe('ok');
373369
});
374370

@@ -379,14 +375,10 @@ describe('SentrySpanProcessor', () => {
379375
tracer.startActiveSpan('SELECT * FROM users;', child => {
380376
const sentrySpan = getSpanForOtelSpan(child);
381377

382-
// eslint-disable-next-line deprecation/deprecation
383-
expect(sentrySpan?.status).toBe(undefined);
384378
expect(spanToJSON(sentrySpan!).status).toBe(undefined);
385379

386380
child.end();
387381

388-
// eslint-disable-next-line deprecation/deprecation
389-
expect(sentrySpan?.status).toBe('ok');
390382
expect(spanToJSON(sentrySpan!).status).toBe('ok');
391383

392384
parentOtelSpan.end();
@@ -469,8 +461,6 @@ describe('SentrySpanProcessor', () => {
469461
}
470462

471463
otelSpan.end();
472-
// eslint-disable-next-line deprecation/deprecation
473-
expect(transaction?.status).toBe(expected);
474464
expect(spanToJSON(transaction!).status).toBe(expected);
475465
},
476466
);

0 commit comments

Comments
 (0)