Skip to content

Commit d3fecc1

Browse files
Lms24Lms24
authored
feat(core): Deprecate Span.parentSpanId (#10244)
Deprecate the `Span.parentSpanId` field on the interface and class. This required only a couple of code replacements and a bunch of test adjustments. Also went ahead and changed the integration test event type in the tests I was modifying.
1 parent f3b2e7d commit d3fecc1

File tree

12 files changed

+94
-48
lines changed

12 files changed

+94
-48
lines changed

‎MIGRATION.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ In v8, the Span class is heavily reworked. The following properties & methods ar
193193
- `span.getTraceContext()`: Use `spanToTraceContext(span)` utility function instead.
194194
- `span.sampled`: Use `span.isRecording()` instead.
195195
- `span.spanId`: Use `span.spanContext().spanId` instead.
196+
- `span.parentSpanId`: Use `spanToJSON(span).parent_span_id` instead.
196197
- `span.traceId`: Use `span.spanContext().traceId` instead.
197198
- `span.name`: Use `spanToJSON(span).description` instead.
198199
- `span.description`: Use `spanToJSON(span).description` instead.

‎dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect } from '@playwright/test';
2-
import type { Event } from '@sentry/types';
2+
import type { SerializedEvent } from '@sentry/types';
33

44
import { sentryTest } from '../../../../utils/fixtures';
55
import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers';
@@ -10,7 +10,7 @@ sentryTest('should send a transaction in an envelope', async ({ getLocalTestPath
1010
}
1111

1212
const url = await getLocalTestPath({ testDir: __dirname });
13-
const transaction = await getFirstSentryEnvelopeRequest<Event>(page, url);
13+
const transaction = await getFirstSentryEnvelopeRequest<SerializedEvent>(page, url);
1414

1515
expect(transaction.transaction).toBe('parent_span');
1616
expect(transaction.spans).toBeDefined();
@@ -22,14 +22,13 @@ sentryTest('should report finished spans as children of the root transaction', a
2222
}
2323

2424
const url = await getLocalTestPath({ testDir: __dirname });
25-
const transaction = await getFirstSentryEnvelopeRequest<Event>(page, url);
25+
const transaction = await getFirstSentryEnvelopeRequest<SerializedEvent>(page, url);
2626

27-
const rootSpanId = transaction?.contexts?.trace?.spanId;
27+
const rootSpanId = transaction?.contexts?.trace?.span_id;
2828

2929
expect(transaction.spans).toHaveLength(1);
3030

3131
const span_1 = transaction.spans?.[0];
32-
// eslint-disable-next-line deprecation/deprecation
3332
expect(span_1?.description).toBe('child_span');
34-
expect(span_1?.parentSpanId).toEqual(rootSpanId);
33+
expect(span_1?.parent_span_id).toEqual(rootSpanId);
3534
});

‎dev-packages/browser-integration-tests/suites/public-api/startTransaction/basic_usage/test.ts

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect } from '@playwright/test';
2-
import type { Event } from '@sentry/types';
2+
import type { SerializedEvent } from '@sentry/types';
33

44
import { sentryTest } from '../../../../utils/fixtures';
55
import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers';
@@ -10,7 +10,7 @@ sentryTest('should report a transaction in an envelope', async ({ getLocalTestPa
1010
}
1111

1212
const url = await getLocalTestPath({ testDir: __dirname });
13-
const transaction = await getFirstSentryEnvelopeRequest<Event>(page, url);
13+
const transaction = await getFirstSentryEnvelopeRequest<SerializedEvent>(page, url);
1414

1515
expect(transaction.transaction).toBe('test_transaction_1');
1616
expect(transaction.spans).toBeDefined();
@@ -22,28 +22,26 @@ sentryTest('should report finished spans as children of the root transaction', a
2222
}
2323

2424
const url = await getLocalTestPath({ testDir: __dirname });
25-
const transaction = await getFirstSentryEnvelopeRequest<Event>(page, url);
25+
const transaction = await getFirstSentryEnvelopeRequest<SerializedEvent>(page, url);
2626

27-
const rootSpanId = transaction?.contexts?.trace?.spanId;
27+
const rootSpanId = transaction?.contexts?.trace?.span_id;
2828

2929
expect(transaction.spans).toHaveLength(3);
3030

3131
const span_1 = transaction.spans?.[0];
3232

33-
// eslint-disable-next-line deprecation/deprecation
34-
expect(span_1?.op).toBe('span_1');
35-
expect(span_1?.parentSpanId).toEqual(rootSpanId);
36-
// eslint-disable-next-line deprecation/deprecation
37-
expect(span_1?.data).toMatchObject({ foo: 'bar', baz: [1, 2, 3] });
33+
expect(span_1).toBeDefined();
34+
expect(span_1!.op).toBe('span_1');
35+
expect(span_1!.parent_span_id).toEqual(rootSpanId);
36+
expect(span_1!.data).toMatchObject({ foo: 'bar', baz: [1, 2, 3] });
3837

3938
const span_3 = transaction.spans?.[1];
40-
// eslint-disable-next-line deprecation/deprecation
41-
expect(span_3?.op).toBe('span_3');
42-
expect(span_3?.parentSpanId).toEqual(rootSpanId);
39+
expect(span_3).toBeDefined();
40+
expect(span_3!.op).toBe('span_3');
41+
expect(span_3!.parent_span_id).toEqual(rootSpanId);
4342

4443
const span_5 = transaction.spans?.[2];
45-
// eslint-disable-next-line deprecation/deprecation
46-
expect(span_5?.op).toBe('span_5');
47-
// eslint-disable-next-line deprecation/deprecation
48-
expect(span_5?.parentSpanId).toEqual(span_3?.spanId);
44+
expect(span_5).toBeDefined();
45+
expect(span_5!.op).toBe('span_5');
46+
expect(span_5!.parent_span_id).toEqual(span_3!.span_id);
4947
});

‎dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-fid/test.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect } from '@playwright/test';
2-
import type { Event } from '@sentry/types';
2+
import type { SerializedEvent } from '@sentry/types';
33

44
import { sentryTest } from '../../../../utils/fixtures';
55
import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers';
@@ -16,16 +16,14 @@ sentryTest('should capture a FID vital.', async ({ browserName, getLocalTestPath
1616
// To trigger FID
1717
await page.click('#fid-btn');
1818

19-
const eventData = await getFirstSentryEnvelopeRequest<Event>(page);
19+
const eventData = await getFirstSentryEnvelopeRequest<SerializedEvent>(page);
2020

2121
expect(eventData.measurements).toBeDefined();
2222
expect(eventData.measurements?.fid?.value).toBeDefined();
2323

24-
// eslint-disable-next-line deprecation/deprecation
2524
const fidSpan = eventData.spans?.filter(({ description }) => description === 'first input delay')[0];
2625

2726
expect(fidSpan).toBeDefined();
28-
// eslint-disable-next-line deprecation/deprecation
2927
expect(fidSpan?.op).toBe('ui.action');
30-
expect(fidSpan?.parentSpanId).toBe(eventData.contexts?.trace_span_id);
28+
expect(fidSpan?.parent_span_id).toBe(eventData.contexts?.trace?.span_id);
3129
});

‎dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-fp-fcp/test.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect } from '@playwright/test';
2-
import type { Event } from '@sentry/types';
2+
import type { SerializedEvent } from '@sentry/types';
33

44
import { sentryTest } from '../../../../utils/fixtures';
55
import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers';
@@ -11,18 +11,16 @@ sentryTest('should capture FP vital.', async ({ browserName, getLocalTestPath, p
1111
}
1212

1313
const url = await getLocalTestPath({ testDir: __dirname });
14-
const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
14+
const eventData = await getFirstSentryEnvelopeRequest<SerializedEvent>(page, url);
1515

1616
expect(eventData.measurements).toBeDefined();
1717
expect(eventData.measurements?.fp?.value).toBeDefined();
1818

19-
// eslint-disable-next-line deprecation/deprecation
2019
const fpSpan = eventData.spans?.filter(({ description }) => description === 'first-paint')[0];
2120

2221
expect(fpSpan).toBeDefined();
23-
// eslint-disable-next-line deprecation/deprecation
2422
expect(fpSpan?.op).toBe('paint');
25-
expect(fpSpan?.parentSpanId).toBe(eventData.contexts?.trace_span_id);
23+
expect(fpSpan?.parent_span_id).toBe(eventData.contexts?.trace?.span_id);
2624
});
2725

2826
sentryTest('should capture FCP vital.', async ({ getLocalTestPath, page }) => {
@@ -31,16 +29,14 @@ sentryTest('should capture FCP vital.', async ({ getLocalTestPath, page }) => {
3129
}
3230

3331
const url = await getLocalTestPath({ testDir: __dirname });
34-
const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
32+
const eventData = await getFirstSentryEnvelopeRequest<SerializedEvent>(page, url);
3533

3634
expect(eventData.measurements).toBeDefined();
3735
expect(eventData.measurements?.fcp?.value).toBeDefined();
3836

39-
// eslint-disable-next-line deprecation/deprecation
4037
const fcpSpan = eventData.spans?.filter(({ description }) => description === 'first-contentful-paint')[0];
4138

4239
expect(fcpSpan).toBeDefined();
43-
// eslint-disable-next-line deprecation/deprecation
4440
expect(fcpSpan?.op).toBe('paint');
45-
expect(fcpSpan?.parentSpanId).toBe(eventData.contexts?.trace_span_id);
41+
expect(fcpSpan?.parent_span_id).toBe(eventData.contexts?.trace?.span_id);
4642
});

‎docs/v8-new-performance-apis.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ below to see which things used to exist, and how they can/should be mapped going
4646
| --------------------- | ---------------------------------------------------- |
4747
| `traceId` | `spanContext().traceId` |
4848
| `spanId` | `spanContext().spanId` |
49-
| `parentSpanId` | Unchanged |
49+
| `parentSpanId` | `spanToJSON(span).parent_span_id` |
5050
| `status` | use utility method TODO |
5151
| `sampled` | `spanIsSampled(span)` |
5252
| `startTimestamp` | `startTime` - note that this has a different format! |

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

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,6 @@ export class SpanRecorder {
6363
* Span contains all data about a span
6464
*/
6565
export class Span implements SpanInterface {
66-
/**
67-
* @inheritDoc
68-
*/
69-
public parentSpanId?: string;
70-
7166
/**
7267
* Tags for the span.
7368
* @deprecated Use `getSpanAttributes(span)` instead.
@@ -112,6 +107,7 @@ export class Span implements SpanInterface {
112107

113108
protected _traceId: string;
114109
protected _spanId: string;
110+
protected _parentSpanId?: string;
115111
protected _sampled: boolean | undefined;
116112
protected _name?: string;
117113
protected _attributes: SpanAttributes;
@@ -147,7 +143,7 @@ export class Span implements SpanInterface {
147143
this._name = spanContext.name || spanContext.description;
148144

149145
if (spanContext.parentSpanId) {
150-
this.parentSpanId = spanContext.parentSpanId;
146+
this._parentSpanId = spanContext.parentSpanId;
151147
}
152148
// We want to include booleans as well here
153149
if ('sampled' in spanContext) {
@@ -231,6 +227,24 @@ export class Span implements SpanInterface {
231227
this._spanId = spanId;
232228
}
233229

230+
/**
231+
* @inheritDoc
232+
*
233+
* @deprecated Use `startSpan` functions instead.
234+
*/
235+
public set parentSpanId(string) {
236+
this._parentSpanId = string;
237+
}
238+
239+
/**
240+
* @inheritDoc
241+
*
242+
* @deprecated Use `spanToJSON(span).parent_span_id` instead.
243+
*/
244+
public get parentSpanId(): string | undefined {
245+
return this._parentSpanId;
246+
}
247+
234248
/**
235249
* Was this span chosen to be sent as part of the sample?
236250
* @deprecated Use `isRecording()` instead.
@@ -531,7 +545,7 @@ export class Span implements SpanInterface {
531545
endTimestamp: this._endTime,
532546
// eslint-disable-next-line deprecation/deprecation
533547
op: this.op,
534-
parentSpanId: this.parentSpanId,
548+
parentSpanId: this._parentSpanId,
535549
sampled: this._sampled,
536550
spanId: this._spanId,
537551
startTimestamp: this._startTime,
@@ -555,7 +569,7 @@ export class Span implements SpanInterface {
555569
this._endTime = spanContext.endTimestamp;
556570
// eslint-disable-next-line deprecation/deprecation
557571
this.op = spanContext.op;
558-
this.parentSpanId = spanContext.parentSpanId;
572+
this._parentSpanId = spanContext.parentSpanId;
559573
this._sampled = spanContext.sampled;
560574
this._spanId = spanContext.spanId || this._spanId;
561575
this._startTime = spanContext.startTimestamp || this._startTime;
@@ -589,7 +603,7 @@ export class Span implements SpanInterface {
589603
data: this._getData(),
590604
description: this._name,
591605
op: this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] as string | undefined,
592-
parent_span_id: this.parentSpanId,
606+
parent_span_id: this._parentSpanId,
593607
span_id: this._spanId,
594608
start_timestamp: this._startTime,
595609
status: this._status,

‎packages/core/test/lib/scope.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
getCurrentScope,
88
getIsolationScope,
99
makeMain,
10+
spanToJSON,
1011
startInactiveSpan,
1112
startSpan,
1213
withIsolationScope,
@@ -543,12 +544,14 @@ describe('withActiveSpan()', () => {
543544
});
544545

545546
it('should create child spans when calling startSpan within the callback', done => {
546-
expect.assertions(1);
547+
expect.assertions(2);
547548
const inactiveSpan = startInactiveSpan({ name: 'inactive-span' });
548549

549550
withActiveSpan(inactiveSpan!, () => {
550551
startSpan({ name: 'child-span' }, childSpan => {
552+
// eslint-disable-next-line deprecation/deprecation
551553
expect(childSpan?.parentSpanId).toBe(inactiveSpan?.spanContext().spanId);
554+
expect(spanToJSON(childSpan!).parent_span_id).toBe(inactiveSpan?.spanContext().spanId);
552555
done();
553556
});
554557
});

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,8 @@ describe('startSpan', () => {
276276
expect(getCurrentScope()).toBe(manualScope);
277277
expect(getActiveSpan()).toBe(span);
278278

279+
expect(spanToJSON(span!).parent_span_id).toBe('parent-span-id');
280+
// eslint-disable-next-line deprecation/deprecation
279281
expect(span?.parentSpanId).toBe('parent-span-id');
280282
});
281283

@@ -323,6 +325,8 @@ describe('startSpanManual', () => {
323325
expect(getCurrentScope()).not.toBe(initialScope);
324326
expect(getCurrentScope()).toBe(manualScope);
325327
expect(getActiveSpan()).toBe(span);
328+
expect(spanToJSON(span!).parent_span_id).toBe('parent-span-id');
329+
// eslint-disable-next-line deprecation/deprecation
326330
expect(span?.parentSpanId).toBe('parent-span-id');
327331

328332
finish();
@@ -377,6 +381,8 @@ describe('startInactiveSpan', () => {
377381
const span = startInactiveSpan({ name: 'GET users/[id]', scope: manualScope });
378382

379383
expect(span).toBeDefined();
384+
expect(spanToJSON(span!).parent_span_id).toBe('parent-span-id');
385+
// eslint-disable-next-line deprecation/deprecation
380386
expect(span?.parentSpanId).toBe('parent-span-id');
381387
expect(getActiveSpan()).toBeUndefined();
382388

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@ describe('SentrySpanProcessor', () => {
9393
expect(spanToJSON(sentrySpanTransaction!).description).toBe('GET /users');
9494
expect(spanToJSON(sentrySpanTransaction!).start_timestamp).toEqual(startTimestampMs / 1000);
9595
expect(sentrySpanTransaction?.spanContext().traceId).toEqual(otelSpan.spanContext().traceId);
96+
expect(spanToJSON(sentrySpanTransaction!).parent_span_id).toEqual(otelSpan.parentSpanId);
97+
// eslint-disable-next-line deprecation/deprecation
9698
expect(sentrySpanTransaction?.parentSpanId).toEqual(otelSpan.parentSpanId);
9799
expect(sentrySpanTransaction?.spanContext().spanId).toEqual(otelSpan.spanContext().spanId);
98100

@@ -121,6 +123,9 @@ describe('SentrySpanProcessor', () => {
121123
expect(sentrySpan ? spanToJSON(sentrySpan).description : undefined).toBe('SELECT * FROM users;');
122124
expect(spanToJSON(sentrySpan!).start_timestamp).toEqual(startTimestampMs / 1000);
123125
expect(sentrySpan?.spanContext().spanId).toEqual(childOtelSpan.spanContext().spanId);
126+
127+
expect(spanToJSON(sentrySpan!).parent_span_id).toEqual(sentrySpanTransaction?.spanContext().spanId);
128+
// eslint-disable-next-line deprecation/deprecation
124129
expect(sentrySpan?.parentSpanId).toEqual(sentrySpanTransaction?.spanContext().spanId);
125130

126131
// eslint-disable-next-line deprecation/deprecation
@@ -162,6 +167,9 @@ describe('SentrySpanProcessor', () => {
162167
expect(spanToJSON(sentrySpan!).description).toBe('SELECT * FROM users;');
163168
expect(spanToJSON(sentrySpan!).start_timestamp).toEqual(startTimestampMs / 1000);
164169
expect(sentrySpan?.spanContext().spanId).toEqual(childOtelSpan.spanContext().spanId);
170+
171+
expect(spanToJSON(sentrySpan!).parent_span_id).toEqual(parentOtelSpan.spanContext().spanId);
172+
// eslint-disable-next-line deprecation/deprecation
165173
expect(sentrySpan?.parentSpanId).toEqual(parentOtelSpan.spanContext().spanId);
166174

167175
// eslint-disable-next-line deprecation/deprecation
@@ -194,8 +202,16 @@ describe('SentrySpanProcessor', () => {
194202
const sentrySpan2 = getSpanForOtelSpan(span2);
195203
const sentrySpan3 = getSpanForOtelSpan(span3);
196204

205+
expect(spanToJSON(sentrySpan1!).parent_span_id).toEqual(sentrySpanTransaction?.spanContext().spanId);
206+
// eslint-disable-next-line deprecation/deprecation
197207
expect(sentrySpan1?.parentSpanId).toEqual(sentrySpanTransaction?.spanContext().spanId);
208+
209+
expect(spanToJSON(sentrySpan2!).parent_span_id).toEqual(sentrySpanTransaction?.spanContext().spanId);
210+
// eslint-disable-next-line deprecation/deprecation
198211
expect(sentrySpan2?.parentSpanId).toEqual(sentrySpanTransaction?.spanContext().spanId);
212+
213+
expect(spanToJSON(sentrySpan3!).parent_span_id).toEqual(sentrySpanTransaction?.spanContext().spanId);
214+
// eslint-disable-next-line deprecation/deprecation
199215
expect(sentrySpan3?.parentSpanId).toEqual(sentrySpanTransaction?.spanContext().spanId);
200216

201217
expect(spanToJSON(sentrySpan1!).description).toEqual('SELECT * FROM users;');
@@ -255,7 +271,14 @@ describe('SentrySpanProcessor', () => {
255271
expect(childSpan).toBeInstanceOf(Transaction);
256272
expect(spanToJSON(parentSpan!).timestamp).toBeDefined();
257273
expect(spanToJSON(childSpan!).timestamp).toBeDefined();
274+
expect(spanToJSON(parentSpan!).parent_span_id).toBeUndefined();
275+
276+
expect(spanToJSON(parentSpan!).parent_span_id).toBeUndefined();
277+
// eslint-disable-next-line deprecation/deprecation
258278
expect(parentSpan?.parentSpanId).toBeUndefined();
279+
280+
expect(spanToJSON(childSpan!).parent_span_id).toEqual(parentSpan?.spanContext().spanId);
281+
// eslint-disable-next-line deprecation/deprecation
259282
expect(childSpan?.parentSpanId).toEqual(parentSpan?.spanContext().spanId);
260283
});
261284
});

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
addTracingExtensions,
77
getActiveTransaction,
88
spanIsSampled,
9+
spanToJSON,
910
startIdleTransaction,
1011
} from '@sentry/core';
1112
import type { EventProcessor, Integration, Transaction, TransactionContext, TransactionSource } from '@sentry/types';
@@ -396,7 +397,7 @@ export class BrowserTracing implements Integration {
396397
scope.setPropagationContext({
397398
traceId: idleTransaction.spanContext().traceId,
398399
spanId: idleTransaction.spanContext().spanId,
399-
parentSpanId: idleTransaction.parentSpanId,
400+
parentSpanId: spanToJSON(idleTransaction).parent_span_id,
400401
sampled: spanIsSampled(idleTransaction),
401402
});
402403
}

0 commit comments

Comments
 (0)