Skip to content

Commit 66333e6

Browse files
Lms24Lms24
authored
feat(core): Deprecate Span.spanRecorder (#10199)
Deprecate the `spanRecorder` field on the `Span` class. For now there's no replacement yet. I'm not entirely sure how we're going to keep track of spans within the SDK. #9972 shows an alternative for `IdleSpan` which no longer seems to rely on the `spanRecorder`. Another possibility might be to make the recorder private which might be enough, too.
1 parent 0bcf0fb commit 66333e6

File tree

7 files changed

+34
-1
lines changed

7 files changed

+34
-1
lines changed

‎MIGRATION.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@ In v8, the Span class is heavily reworked. The following properties & methods ar
177177
- `span.setData()`: Use `span.setAttribute()` instead.
178178
- `span.instrumenter` This field was removed and will be replaced internally.
179179
- `span.transaction`: Use `getRootSpan` utility function instead.
180+
- `span.spanRecorder`: Span recording will be handled internally by the SDK.
180181
- `transaction.setMetadata()`: Use attributes instead, or set data on the scope.
181182
- `transaction.metadata`: Use attributes instead, or set data on the scope.
182183
- `transaction.setContext()`: Set context on the surrounding scope instead.

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ export class IdleTransaction extends Transaction {
152152
this.setAttribute(FINISH_REASON_TAG, this._finishReason);
153153
}
154154

155+
// eslint-disable-next-line deprecation/deprecation
155156
if (this.spanRecorder) {
156157
DEBUG_BUILD &&
157158
logger.log('[Tracing] finishing IdleTransaction', new Date(endTimestampInS * 1000).toISOString(), this.op);
@@ -160,6 +161,7 @@ export class IdleTransaction extends Transaction {
160161
callback(this, endTimestampInS);
161162
}
162163

164+
// eslint-disable-next-line deprecation/deprecation
163165
this.spanRecorder.spans = this.spanRecorder.spans.filter((span: Span) => {
164166
// If we are dealing with the transaction itself, we just return it
165167
if (span.spanContext().spanId === this.spanContext().spanId) {
@@ -227,6 +229,7 @@ export class IdleTransaction extends Transaction {
227229
* @inheritDoc
228230
*/
229231
public initSpanRecorder(maxlen?: number): void {
232+
// eslint-disable-next-line deprecation/deprecation
230233
if (!this.spanRecorder) {
231234
const pushActivity = (id: string): void => {
232235
if (this._finished) {
@@ -241,12 +244,14 @@ export class IdleTransaction extends Transaction {
241244
this._popActivity(id);
242245
};
243246

247+
// eslint-disable-next-line deprecation/deprecation
244248
this.spanRecorder = new IdleTransactionSpanRecorder(pushActivity, popActivity, this.spanContext().spanId, maxlen);
245249

246250
// Start heartbeat so that transactions do not run forever.
247251
DEBUG_BUILD && logger.log('Starting heartbeat');
248252
this._pingHeartbeat();
249253
}
254+
// eslint-disable-next-line deprecation/deprecation
250255
this.spanRecorder.add(this);
251256
}
252257

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ export class SpanRecorder {
5050
*/
5151
public add(span: Span): void {
5252
if (this.spans.length > this._maxlen) {
53+
// eslint-disable-next-line deprecation/deprecation
5354
span.spanRecorder = undefined;
5455
} else {
5556
this.spans.push(span);
@@ -91,6 +92,8 @@ export class Span implements SpanInterface {
9192

9293
/**
9394
* List of spans that were finalized
95+
*
96+
* @deprecated This property will no longer be public. Span recording will be handled internally.
9497
*/
9598
public spanRecorder?: SpanRecorder;
9699

@@ -327,8 +330,11 @@ export class Span implements SpanInterface {
327330
traceId: this._traceId,
328331
});
329332

333+
// eslint-disable-next-line deprecation/deprecation
330334
childSpan.spanRecorder = this.spanRecorder;
335+
// eslint-disable-next-line deprecation/deprecation
331336
if (childSpan.spanRecorder) {
337+
// eslint-disable-next-line deprecation/deprecation
332338
childSpan.spanRecorder.add(childSpan);
333339
}
334340

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,9 +155,12 @@ export class Transaction extends SpanClass implements TransactionInterface {
155155
* @param maxlen maximum number of spans that can be recorded
156156
*/
157157
public initSpanRecorder(maxlen: number = 1000): void {
158+
// eslint-disable-next-line deprecation/deprecation
158159
if (!this.spanRecorder) {
160+
// eslint-disable-next-line deprecation/deprecation
159161
this.spanRecorder = new SpanRecorder(maxlen);
160162
}
163+
// eslint-disable-next-line deprecation/deprecation
161164
this.spanRecorder.add(this);
162165
}
163166

@@ -286,8 +289,10 @@ export class Transaction extends SpanClass implements TransactionInterface {
286289
return undefined;
287290
}
288291

292+
// eslint-disable-next-line deprecation/deprecation
289293
const finishedSpans = this.spanRecorder
290-
? this.spanRecorder.spans.filter(span => span !== this && spanToJSON(span).timestamp)
294+
? // eslint-disable-next-line deprecation/deprecation
295+
this.spanRecorder.spans.filter(span => span !== this && spanToJSON(span).timestamp)
291296
: [];
292297

293298
if (this._trimEnd && finishedSpans.length > 0) {

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ describe('tracing', () => {
7070
nock('http://dogs.are.great').get('/').reply(200);
7171

7272
const transaction = createTransactionOnScope();
73+
// eslint-disable-next-line deprecation/deprecation
7374
const spans = (transaction as unknown as Span).spanRecorder?.spans as Span[];
7475

7576
http.get('http://dogs.are.great/');
@@ -85,6 +86,7 @@ describe('tracing', () => {
8586
nock('http://squirrelchasers.ingest.sentry.io').get('/api/12312012/store/').reply(200);
8687

8788
const transaction = createTransactionOnScope();
89+
// eslint-disable-next-line deprecation/deprecation
8890
const spans = (transaction as unknown as Span).spanRecorder?.spans as Span[];
8991

9092
http.get('http://squirrelchasers.ingest.sentry.io/api/12312012/store/');
@@ -272,6 +274,7 @@ describe('tracing', () => {
272274
nock('http://dogs.are.great').get('/spaniel?tail=wag&cute=true#learn-more').reply(200);
273275

274276
const transaction = createTransactionOnScope();
277+
// eslint-disable-next-line deprecation/deprecation
275278
const spans = (transaction as unknown as Span).spanRecorder?.spans as Span[];
276279

277280
http.get('http://dogs.are.great/spaniel?tail=wag&cute=true#learn-more');
@@ -294,6 +297,7 @@ describe('tracing', () => {
294297
nock('http://dogs.are.great').get('/spaniel?tail=wag&cute=true#learn-more').reply(200);
295298

296299
const transaction = createTransactionOnScope();
300+
// eslint-disable-next-line deprecation/deprecation
297301
const spans = (transaction as unknown as Span).spanRecorder?.spans as Span[];
298302

299303
http.request({ method: 'GET', host: 'dogs.are.great', path: '/spaniel?tail=wag&cute=true#learn-more' });
@@ -321,6 +325,7 @@ describe('tracing', () => {
321325
nock(`http://${auth}@dogs.are.great`).get('/').reply(200);
322326

323327
const transaction = createTransactionOnScope();
328+
// eslint-disable-next-line deprecation/deprecation
324329
const spans = (transaction as unknown as Span).spanRecorder?.spans as Span[];
325330

326331
http.get(`http://${auth}@dogs.are.great/`);
@@ -380,6 +385,7 @@ describe('tracing', () => {
380385
);
381386

382387
const transaction = createTransactionAndPutOnScope();
388+
// eslint-disable-next-line deprecation/deprecation
383389
const spans = (transaction as unknown as Span).spanRecorder?.spans as Span[];
384390

385391
const request = http.get(url);
@@ -485,6 +491,7 @@ describe('tracing', () => {
485491
);
486492

487493
const transaction = createTransactionAndPutOnScope();
494+
// eslint-disable-next-line deprecation/deprecation
488495
const spans = (transaction as unknown as Span).spanRecorder?.spans as Span[];
489496

490497
const request = http.get(url);

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ conditionalTest({ min: 16 })('Undici integration', () => {
111111
await fetch(request, requestInit);
112112

113113
expect(outerSpan).toBeInstanceOf(Transaction);
114+
// eslint-disable-next-line deprecation/deprecation
114115
const spans = (outerSpan as Transaction).spanRecorder?.spans || [];
115116

116117
expect(spans.length).toBe(2);
@@ -129,6 +130,7 @@ conditionalTest({ min: 16 })('Undici integration', () => {
129130
}
130131

131132
expect(outerSpan).toBeInstanceOf(Transaction);
133+
// eslint-disable-next-line deprecation/deprecation
132134
const spans = (outerSpan as Transaction).spanRecorder?.spans || [];
133135

134136
expect(spans.length).toBe(2);
@@ -149,6 +151,7 @@ conditionalTest({ min: 16 })('Undici integration', () => {
149151
}
150152

151153
expect(outerSpan).toBeInstanceOf(Transaction);
154+
// eslint-disable-next-line deprecation/deprecation
152155
const spans = (outerSpan as Transaction).spanRecorder?.spans || [];
153156

154157
expect(spans.length).toBe(2);
@@ -170,6 +173,7 @@ conditionalTest({ min: 16 })('Undici integration', () => {
170173
}
171174

172175
expect(outerSpan).toBeInstanceOf(Transaction);
176+
// eslint-disable-next-line deprecation/deprecation
173177
const spans = (outerSpan as Transaction).spanRecorder?.spans || [];
174178

175179
expect(spans.length).toBe(1);
@@ -189,6 +193,7 @@ conditionalTest({ min: 16 })('Undici integration', () => {
189193
it('does create a span if `shouldCreateSpanForRequest` is defined', async () => {
190194
await startSpan({ name: 'outer-span' }, async outerSpan => {
191195
expect(outerSpan).toBeInstanceOf(Transaction);
196+
// eslint-disable-next-line deprecation/deprecation
192197
const spans = (outerSpan as Transaction).spanRecorder?.spans || [];
193198

194199
const undoPatch = patchUndici({ shouldCreateSpanForRequest: url => url.includes('yes') });
@@ -213,6 +218,7 @@ conditionalTest({ min: 16 })('Undici integration', () => {
213218
await runWithAsyncContext(async () => {
214219
await startSpan({ name: 'outer-span' }, async outerSpan => {
215220
expect(outerSpan).toBeInstanceOf(Transaction);
221+
// eslint-disable-next-line deprecation/deprecation
216222
const spans = (outerSpan as Transaction).spanRecorder?.spans || [];
217223

218224
await fetch('http://localhost:18100', { method: 'POST' });
@@ -287,6 +293,7 @@ conditionalTest({ min: 16 })('Undici integration', () => {
287293

288294
await startSpan({ name: 'outer-span' }, async outerSpan => {
289295
expect(outerSpan).toBeInstanceOf(Transaction);
296+
// eslint-disable-next-line deprecation/deprecation
290297
const spans = (outerSpan as Transaction).spanRecorder?.spans || [];
291298

292299
expect(spans.length).toBe(1);

‎packages/opentelemetry/test/custom/transaction.test.ts

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

155155
expect(transaction).toBeInstanceOf(OpenTelemetryTransaction);
156156
expect(transaction['_sampled']).toBe(undefined);
157+
// eslint-disable-next-line deprecation/deprecation
157158
expect(transaction.spanRecorder).toBeDefined();
159+
// eslint-disable-next-line deprecation/deprecation
158160
expect(transaction.spanRecorder?.spans).toHaveLength(1);
159161
// eslint-disable-next-line deprecation/deprecation
160162
expect(transaction.metadata).toEqual({

0 commit comments

Comments
 (0)