From 8a8dee10448d38a9dfea74d76d78d92b288235a3 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 16 Jan 2024 14:43:07 +0100 Subject: [PATCH] feat(core): Deprecate `Span.spanRecorder` --- MIGRATION.md | 1 + packages/core/src/tracing/idletransaction.ts | 5 +++++ packages/core/src/tracing/span.ts | 6 ++++++ packages/core/src/tracing/transaction.ts | 7 ++++++- packages/node/test/integrations/http.test.ts | 7 +++++++ packages/node/test/integrations/undici.test.ts | 7 +++++++ packages/opentelemetry/test/custom/transaction.test.ts | 2 ++ 7 files changed, 34 insertions(+), 1 deletion(-) diff --git a/MIGRATION.md b/MIGRATION.md index f459f5a6bd03..bf4a7683e40f 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -177,6 +177,7 @@ In v8, the Span class is heavily reworked. The following properties & methods ar - `span.setData()`: Use `span.setAttribute()` instead. - `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. - `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/idletransaction.ts b/packages/core/src/tracing/idletransaction.ts index 156bb88dd5ba..f7ff2e48e9ad 100644 --- a/packages/core/src/tracing/idletransaction.ts +++ b/packages/core/src/tracing/idletransaction.ts @@ -152,6 +152,7 @@ export class IdleTransaction extends Transaction { this.setAttribute(FINISH_REASON_TAG, this._finishReason); } + // eslint-disable-next-line deprecation/deprecation if (this.spanRecorder) { DEBUG_BUILD && logger.log('[Tracing] finishing IdleTransaction', new Date(endTimestampInS * 1000).toISOString(), this.op); @@ -160,6 +161,7 @@ export class IdleTransaction extends Transaction { callback(this, endTimestampInS); } + // eslint-disable-next-line deprecation/deprecation this.spanRecorder.spans = this.spanRecorder.spans.filter((span: Span) => { // If we are dealing with the transaction itself, we just return it if (span.spanContext().spanId === this.spanContext().spanId) { @@ -227,6 +229,7 @@ export class IdleTransaction extends Transaction { * @inheritDoc */ public initSpanRecorder(maxlen?: number): void { + // eslint-disable-next-line deprecation/deprecation if (!this.spanRecorder) { const pushActivity = (id: string): void => { if (this._finished) { @@ -241,12 +244,14 @@ export class IdleTransaction extends Transaction { this._popActivity(id); }; + // eslint-disable-next-line deprecation/deprecation this.spanRecorder = new IdleTransactionSpanRecorder(pushActivity, popActivity, this.spanContext().spanId, maxlen); // Start heartbeat so that transactions do not run forever. DEBUG_BUILD && logger.log('Starting heartbeat'); this._pingHeartbeat(); } + // eslint-disable-next-line deprecation/deprecation this.spanRecorder.add(this); } diff --git a/packages/core/src/tracing/span.ts b/packages/core/src/tracing/span.ts index b00ad88cb9e6..2d6037aff9f3 100644 --- a/packages/core/src/tracing/span.ts +++ b/packages/core/src/tracing/span.ts @@ -50,6 +50,7 @@ export class SpanRecorder { */ public add(span: Span): void { if (this.spans.length > this._maxlen) { + // eslint-disable-next-line deprecation/deprecation span.spanRecorder = undefined; } else { this.spans.push(span); @@ -91,6 +92,8 @@ export class Span implements SpanInterface { /** * List of spans that were finalized + * + * @deprecated This property will no longer be public. Span recording will be handled internally. */ public spanRecorder?: SpanRecorder; @@ -327,8 +330,11 @@ export class Span implements SpanInterface { traceId: this._traceId, }); + // eslint-disable-next-line deprecation/deprecation childSpan.spanRecorder = this.spanRecorder; + // eslint-disable-next-line deprecation/deprecation if (childSpan.spanRecorder) { + // eslint-disable-next-line deprecation/deprecation childSpan.spanRecorder.add(childSpan); } diff --git a/packages/core/src/tracing/transaction.ts b/packages/core/src/tracing/transaction.ts index 28b5f1cbfb2b..eb6736d8f59b 100644 --- a/packages/core/src/tracing/transaction.ts +++ b/packages/core/src/tracing/transaction.ts @@ -155,9 +155,12 @@ export class Transaction extends SpanClass implements TransactionInterface { * @param maxlen maximum number of spans that can be recorded */ public initSpanRecorder(maxlen: number = 1000): void { + // eslint-disable-next-line deprecation/deprecation if (!this.spanRecorder) { + // eslint-disable-next-line deprecation/deprecation this.spanRecorder = new SpanRecorder(maxlen); } + // eslint-disable-next-line deprecation/deprecation this.spanRecorder.add(this); } @@ -286,8 +289,10 @@ export class Transaction extends SpanClass implements TransactionInterface { return undefined; } + // eslint-disable-next-line deprecation/deprecation const finishedSpans = this.spanRecorder - ? this.spanRecorder.spans.filter(span => span !== this && spanToJSON(span).timestamp) + ? // eslint-disable-next-line deprecation/deprecation + this.spanRecorder.spans.filter(span => span !== this && spanToJSON(span).timestamp) : []; if (this._trimEnd && finishedSpans.length > 0) { diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index 6a94d19a73f4..b768d60100a9 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -70,6 +70,7 @@ describe('tracing', () => { nock('http://dogs.are.great').get('/').reply(200); const transaction = createTransactionOnScope(); + // eslint-disable-next-line deprecation/deprecation const spans = (transaction as unknown as Span).spanRecorder?.spans as Span[]; http.get('http://dogs.are.great/'); @@ -85,6 +86,7 @@ describe('tracing', () => { nock('http://squirrelchasers.ingest.sentry.io').get('/api/12312012/store/').reply(200); const transaction = createTransactionOnScope(); + // eslint-disable-next-line deprecation/deprecation const spans = (transaction as unknown as Span).spanRecorder?.spans as Span[]; http.get('http://squirrelchasers.ingest.sentry.io/api/12312012/store/'); @@ -272,6 +274,7 @@ describe('tracing', () => { nock('http://dogs.are.great').get('/spaniel?tail=wag&cute=true#learn-more').reply(200); const transaction = createTransactionOnScope(); + // eslint-disable-next-line deprecation/deprecation const spans = (transaction as unknown as Span).spanRecorder?.spans as Span[]; http.get('http://dogs.are.great/spaniel?tail=wag&cute=true#learn-more'); @@ -294,6 +297,7 @@ describe('tracing', () => { nock('http://dogs.are.great').get('/spaniel?tail=wag&cute=true#learn-more').reply(200); const transaction = createTransactionOnScope(); + // eslint-disable-next-line deprecation/deprecation const spans = (transaction as unknown as Span).spanRecorder?.spans as Span[]; http.request({ method: 'GET', host: 'dogs.are.great', path: '/spaniel?tail=wag&cute=true#learn-more' }); @@ -321,6 +325,7 @@ describe('tracing', () => { nock(`http://${auth}@dogs.are.great`).get('/').reply(200); const transaction = createTransactionOnScope(); + // eslint-disable-next-line deprecation/deprecation const spans = (transaction as unknown as Span).spanRecorder?.spans as Span[]; http.get(`http://${auth}@dogs.are.great/`); @@ -380,6 +385,7 @@ describe('tracing', () => { ); const transaction = createTransactionAndPutOnScope(); + // eslint-disable-next-line deprecation/deprecation const spans = (transaction as unknown as Span).spanRecorder?.spans as Span[]; const request = http.get(url); @@ -485,6 +491,7 @@ describe('tracing', () => { ); const transaction = createTransactionAndPutOnScope(); + // eslint-disable-next-line deprecation/deprecation const spans = (transaction as unknown as Span).spanRecorder?.spans as Span[]; const request = http.get(url); diff --git a/packages/node/test/integrations/undici.test.ts b/packages/node/test/integrations/undici.test.ts index 7705b988f721..abda44026fcb 100644 --- a/packages/node/test/integrations/undici.test.ts +++ b/packages/node/test/integrations/undici.test.ts @@ -111,6 +111,7 @@ conditionalTest({ min: 16 })('Undici integration', () => { await fetch(request, requestInit); expect(outerSpan).toBeInstanceOf(Transaction); + // eslint-disable-next-line deprecation/deprecation const spans = (outerSpan as Transaction).spanRecorder?.spans || []; expect(spans.length).toBe(2); @@ -129,6 +130,7 @@ conditionalTest({ min: 16 })('Undici integration', () => { } expect(outerSpan).toBeInstanceOf(Transaction); + // eslint-disable-next-line deprecation/deprecation const spans = (outerSpan as Transaction).spanRecorder?.spans || []; expect(spans.length).toBe(2); @@ -149,6 +151,7 @@ conditionalTest({ min: 16 })('Undici integration', () => { } expect(outerSpan).toBeInstanceOf(Transaction); + // eslint-disable-next-line deprecation/deprecation const spans = (outerSpan as Transaction).spanRecorder?.spans || []; expect(spans.length).toBe(2); @@ -170,6 +173,7 @@ conditionalTest({ min: 16 })('Undici integration', () => { } expect(outerSpan).toBeInstanceOf(Transaction); + // eslint-disable-next-line deprecation/deprecation const spans = (outerSpan as Transaction).spanRecorder?.spans || []; expect(spans.length).toBe(1); @@ -189,6 +193,7 @@ conditionalTest({ min: 16 })('Undici integration', () => { it('does create a span if `shouldCreateSpanForRequest` is defined', async () => { await startSpan({ name: 'outer-span' }, async outerSpan => { expect(outerSpan).toBeInstanceOf(Transaction); + // eslint-disable-next-line deprecation/deprecation const spans = (outerSpan as Transaction).spanRecorder?.spans || []; const undoPatch = patchUndici({ shouldCreateSpanForRequest: url => url.includes('yes') }); @@ -213,6 +218,7 @@ conditionalTest({ min: 16 })('Undici integration', () => { await runWithAsyncContext(async () => { await startSpan({ name: 'outer-span' }, async outerSpan => { expect(outerSpan).toBeInstanceOf(Transaction); + // eslint-disable-next-line deprecation/deprecation const spans = (outerSpan as Transaction).spanRecorder?.spans || []; await fetch('http://localhost:18100', { method: 'POST' }); @@ -287,6 +293,7 @@ conditionalTest({ min: 16 })('Undici integration', () => { await startSpan({ name: 'outer-span' }, async outerSpan => { expect(outerSpan).toBeInstanceOf(Transaction); + // eslint-disable-next-line deprecation/deprecation const spans = (outerSpan as Transaction).spanRecorder?.spans || []; expect(spans.length).toBe(1); diff --git a/packages/opentelemetry/test/custom/transaction.test.ts b/packages/opentelemetry/test/custom/transaction.test.ts index 8626422e5d78..b949ee82d91d 100644 --- a/packages/opentelemetry/test/custom/transaction.test.ts +++ b/packages/opentelemetry/test/custom/transaction.test.ts @@ -154,7 +154,9 @@ describe('startTranscation', () => { expect(transaction).toBeInstanceOf(OpenTelemetryTransaction); expect(transaction['_sampled']).toBe(undefined); + // eslint-disable-next-line deprecation/deprecation expect(transaction.spanRecorder).toBeDefined(); + // eslint-disable-next-line deprecation/deprecation expect(transaction.spanRecorder?.spans).toHaveLength(1); // eslint-disable-next-line deprecation/deprecation expect(transaction.metadata).toEqual({