From 18848c8621e4ae0ade2ec958842ad7e29c66c246 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 18 Mar 2024 16:30:56 -0400 Subject: [PATCH 1/4] feat(v8): Remove deprecated span id fields Removes `span.spanId`, `span.traceId`, and `span.parentSpanId`. --- packages/core/src/tracing/sentrySpan.ts | 50 ------------------- .../core/test/lib/tracing/sentrySpan.test.ts | 27 +++++++--- packages/types/src/envelope.ts | 3 +- packages/types/src/transaction.ts | 14 +----- 4 files changed, 21 insertions(+), 73 deletions(-) diff --git a/packages/core/src/tracing/sentrySpan.ts b/packages/core/src/tracing/sentrySpan.ts index 87b57d729960..68570997131d 100644 --- a/packages/core/src/tracing/sentrySpan.ts +++ b/packages/core/src/tracing/sentrySpan.ts @@ -97,56 +97,6 @@ export class SentrySpan implements Span { // This rule conflicts with another eslint rule :( /* eslint-disable @typescript-eslint/member-ordering */ - /** - * The ID of the trace. - * @deprecated Use `spanContext().traceId` instead. - */ - public get traceId(): string { - return this._traceId; - } - - /** - * The ID of the trace. - * @deprecated You cannot update the traceId of a span after span creation. - */ - public set traceId(traceId: string) { - this._traceId = traceId; - } - - /** - * The ID of the span. - * @deprecated Use `spanContext().spanId` instead. - */ - public get spanId(): string { - return this._spanId; - } - - /** - * The ID of the span. - * @deprecated You cannot update the spanId of a span after span creation. - */ - public set spanId(spanId: string) { - this._spanId = spanId; - } - - /** - * @inheritDoc - * - * @deprecated Use `startSpan` functions instead. - */ - public set parentSpanId(string) { - this._parentSpanId = string; - } - - /** - * @inheritDoc - * - * @deprecated Use `spanToJSON(span).parent_span_id` instead. - */ - public get parentSpanId(): string | undefined { - return this._parentSpanId; - } - /** * Was this span chosen to be sent as part of the sample? * @deprecated Use `isRecording()` instead. diff --git a/packages/core/test/lib/tracing/sentrySpan.test.ts b/packages/core/test/lib/tracing/sentrySpan.test.ts index e065aeff33aa..f63423be6c6d 100644 --- a/packages/core/test/lib/tracing/sentrySpan.test.ts +++ b/packages/core/test/lib/tracing/sentrySpan.test.ts @@ -1,7 +1,13 @@ import { timestampInSeconds } from '@sentry/utils'; import { SentrySpan } from '../../../src/tracing/sentrySpan'; import { SPAN_STATUS_ERROR } from '../../../src/tracing/spanstatus'; -import { TRACE_FLAG_NONE, TRACE_FLAG_SAMPLED, spanToJSON, spanToTraceContext } from '../../../src/utils/spanUtils'; +import { + TRACE_FLAG_NONE, + TRACE_FLAG_SAMPLED, + spanIsSampled, + spanToJSON, + spanToTraceContext, +} from '../../../src/utils/spanUtils'; describe('SentrySpan', () => { describe('name', () => { @@ -25,9 +31,9 @@ describe('SentrySpan', () => { const span = new SentrySpan({ sampled: true }); // eslint-disable-next-line deprecation/deprecation const span2 = span.startChild(); - expect((span2 as any).parentSpanId).toBe((span as any).spanId); - expect((span2 as any).traceId).toBe((span as any).traceId); - expect((span2 as any).sampled).toBe((span as any).sampled); + expect(spanToJSON(span2).parent_span_id).toBe(span.spanContext().spanId); + expect(span.spanContext().traceId).toBe(span.spanContext().traceFlags); + expect(spanIsSampled(span2)).toBe(spanIsSampled(span)); }); }); @@ -58,8 +64,13 @@ describe('SentrySpan', () => { }); test('with parent', () => { - const spanA = new SentrySpan({ traceId: 'a', spanId: 'b' }) as any; - const spanB = new SentrySpan({ traceId: 'c', spanId: 'd', sampled: false, parentSpanId: spanA.spanId }); + const spanA = new SentrySpan({ traceId: 'a', spanId: 'b' }); + const spanB = new SentrySpan({ + traceId: 'c', + spanId: 'd', + sampled: false, + parentSpanId: spanA.spanContext().spanId, + }); const serialized = spanToJSON(spanB); expect(serialized).toHaveProperty('parent_span_id', 'b'); expect(serialized).toHaveProperty('span_id', 'd'); @@ -67,9 +78,9 @@ describe('SentrySpan', () => { }); test('should drop all `undefined` values', () => { - const spanA = new SentrySpan({ traceId: 'a', spanId: 'b' }) as any; + const spanA = new SentrySpan({ traceId: 'a', spanId: 'b' }); const spanB = new SentrySpan({ - parentSpanId: spanA.spanId, + parentSpanId: spanA.spanContext().spanId, spanId: 'd', traceId: 'c', }); diff --git a/packages/types/src/envelope.ts b/packages/types/src/envelope.ts index 721c13323bdb..7a9ad2e4d5c3 100644 --- a/packages/types/src/envelope.ts +++ b/packages/types/src/envelope.ts @@ -8,13 +8,12 @@ import type { Profile } from './profiling'; import type { ReplayEvent, ReplayRecordingData } from './replay'; import type { SdkInfo } from './sdkinfo'; import type { SerializedSession, Session, SessionAggregates } from './session'; -import type { Transaction } from './transaction'; // Based on: https://develop.sentry.dev/sdk/envelopes/ // Based on https://github.com/getsentry/relay/blob/b23b8d3b2360a54aaa4d19ecae0231201f31df5e/relay-sampling/src/lib.rs#L685-L707 export type DynamicSamplingContext = { - trace_id: Transaction['traceId']; + trace_id: string; public_key: DsnComponents['publicKey']; sample_rate?: string; release?: string; diff --git a/packages/types/src/transaction.ts b/packages/types/src/transaction.ts index be482a2c196e..533401455488 100644 --- a/packages/types/src/transaction.ts +++ b/packages/types/src/transaction.ts @@ -57,19 +57,7 @@ export interface TraceparentData { /** * Transaction "Class", inherits Span only has `setName` */ -export interface Transaction extends Omit, Span { - /** - * The ID of the transaction. - * @deprecated Use `spanContext().spanId` instead. - */ - spanId: string; - - /** - * The ID of the trace. - * @deprecated Use `spanContext().traceId` instead. - */ - traceId: string; - +export interface Transaction extends Omit, Span { /** * Was this transaction chosen to be sent as part of the sample? * @deprecated Use `spanIsSampled(transaction)` instead. From 730532fa9bc5be7db4fefb7b27aada8fe461040f Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Mon, 18 Mar 2024 19:03:02 -0400 Subject: [PATCH 2/4] test fixes --- .../express/sentry-trace/baggage-header-out/server.ts | 9 ++++----- .../express/sentry-trace/baggage-header-out/test.ts | 2 +- .../sentry-trace/baggage-transaction-name/server.ts | 8 ++------ packages/core/test/lib/tracing/sentrySpan.test.ts | 2 +- 4 files changed, 8 insertions(+), 13 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/server.ts b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/server.ts index 64f697ca086c..483446312561 100644 --- a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/server.ts +++ b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/server.ts @@ -28,12 +28,11 @@ app.use(cors()); app.get('/test/express', (_req, res) => { // eslint-disable-next-line deprecation/deprecation const transaction = Sentry.getCurrentScope().getTransaction(); - if (transaction) { - // eslint-disable-next-line deprecation/deprecation - transaction.traceId = '86f39e84263a4de99c326acab3bfe3bd'; - } + const traceId = transaction?.spanContext().traceId; const headers = http.get('http://somewhere.not.sentry/').getHeaders(); - + if (traceId) { + headers['baggage'] = (headers['baggage'] as string).replace(traceId, '__SENTRY_TRACE_ID__') + } // Responding with the headers outgoing request headers back to the assertions. res.send({ test_data: headers }); }); diff --git a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts index 3cd2e9984685..c3add50d89cd 100644 --- a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts +++ b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/test.ts @@ -16,7 +16,7 @@ test('should attach a baggage header to an outgoing request.', async () => { host: 'somewhere.not.sentry', baggage: 'sentry-environment=prod,sentry-release=1.0,sentry-public_key=public' + - ',sentry-trace_id=86f39e84263a4de99c326acab3bfe3bd,sentry-sample_rate=1,sentry-transaction=GET%20%2Ftest%2Fexpress' + + ',sentry-trace_id=__SENTRY_TRACE_ID__,sentry-sample_rate=1,sentry-transaction=GET%20%2Ftest%2Fexpress' + ',sentry-sampled=true', }, }); diff --git a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/server.ts b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/server.ts index 8616908dd6d5..2e5cfedb8046 100644 --- a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/server.ts +++ b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/server.ts @@ -32,13 +32,9 @@ app.use(cors()); app.get('/test/express', (_req, res) => { // eslint-disable-next-line deprecation/deprecation const transaction = Sentry.getCurrentScope().getTransaction(); - if (transaction) { - // eslint-disable-next-line deprecation/deprecation - transaction.traceId = '86f39e84263a4de99c326acab3bfe3bd'; - transaction.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); - } - const headers = http.get('http://somewhere.not.sentry/').getHeaders(); + transaction?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + const headers = http.get('http://somewhere.not.sentry/').getHeaders(); // Responding with the headers outgoing request headers back to the assertions. res.send({ test_data: headers }); }); diff --git a/packages/core/test/lib/tracing/sentrySpan.test.ts b/packages/core/test/lib/tracing/sentrySpan.test.ts index f63423be6c6d..e7a971d0bdcf 100644 --- a/packages/core/test/lib/tracing/sentrySpan.test.ts +++ b/packages/core/test/lib/tracing/sentrySpan.test.ts @@ -32,7 +32,7 @@ describe('SentrySpan', () => { // eslint-disable-next-line deprecation/deprecation const span2 = span.startChild(); expect(spanToJSON(span2).parent_span_id).toBe(span.spanContext().spanId); - expect(span.spanContext().traceId).toBe(span.spanContext().traceFlags); + expect(span.spanContext().traceId).toBe(span.spanContext().traceId); expect(spanIsSampled(span2)).toBe(spanIsSampled(span)); }); }); From 84c495254f4092a757062e98c11825dc8ae3dcf2 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Tue, 19 Mar 2024 11:55:20 -0400 Subject: [PATCH 3/4] fix tests --- .../sentry-trace/baggage-header-out/server.ts | 2 +- packages/node/test/handlers.test.ts | 21 +++++++++++-------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/server.ts b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/server.ts index 483446312561..e849eca7b9f2 100644 --- a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/server.ts +++ b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-header-out/server.ts @@ -31,7 +31,7 @@ app.get('/test/express', (_req, res) => { const traceId = transaction?.spanContext().traceId; const headers = http.get('http://somewhere.not.sentry/').getHeaders(); if (traceId) { - headers['baggage'] = (headers['baggage'] as string).replace(traceId, '__SENTRY_TRACE_ID__') + headers['baggage'] = (headers['baggage'] as string).replace(traceId, '__SENTRY_TRACE_ID__'); } // Responding with the headers outgoing request headers back to the assertions. res.send({ test_data: headers }); diff --git a/packages/node/test/handlers.test.ts b/packages/node/test/handlers.test.ts index 8a84cffd758a..eef7fb79a6a9 100644 --- a/packages/node/test/handlers.test.ts +++ b/packages/node/test/handlers.test.ts @@ -9,6 +9,7 @@ import { getMainCarrier, mergeScopeData, setCurrentClient, + spanIsSampled, spanToJSON, withScope, } from '@sentry/core'; @@ -322,12 +323,13 @@ describe('tracingHandler', () => { dsc: { version: '1.0', environment: 'production' }, }); - const transaction = (res as any).__sentry_transaction; + const transaction = (res as any).__sentry_transaction as Transaction; // since we have no tracesSampler defined, the default behavior (inherit if possible) applies - expect(transaction.traceId).toEqual('12312012123120121231201212312012'); - expect(transaction.parentSpanId).toEqual('1121201211212012'); - expect(transaction.sampled).toEqual(true); + expect(transaction.spanContext().traceId).toEqual('12312012123120121231201212312012'); + expect(spanToJSON(transaction).parent_span_id).toEqual('1121201211212012'); + expect(spanIsSampled(transaction)).toEqual(true); + // eslint-disable-next-line deprecation/deprecation expect(transaction.metadata?.dynamicSamplingContext).toStrictEqual({ version: '1.0', environment: 'production' }); }); @@ -341,7 +343,8 @@ describe('tracingHandler', () => { expect(getPropagationContext().dsc).toEqual({ version: '1.0', environment: 'production' }); - const transaction = (res as any).__sentry_transaction; + const transaction = (res as any).__sentry_transaction as Transaction; + // eslint-disable-next-line deprecation/deprecation expect(transaction.metadata?.dynamicSamplingContext).toStrictEqual({ version: '1.0', environment: 'production' }); }); @@ -364,7 +367,7 @@ describe('tracingHandler', () => { it('puts its transaction on the response object', () => { sentryTracingMiddleware(req, res, next); - const transaction = (res as any).__sentry_transaction; + const transaction = (res as any).__sentry_transaction as Transaction; expect(transaction).toBeDefined(); @@ -396,7 +399,7 @@ describe('tracingHandler', () => { sentryTracingMiddleware(req, res, next); - const transaction = (res as any).__sentry_transaction; + const transaction = (res as any).__sentry_transaction as Transaction; expect(spanToJSON(transaction).description).toBe(`${method.toUpperCase()} ${path}`); }); @@ -406,7 +409,7 @@ describe('tracingHandler', () => { sentryTracingMiddleware(req, res, next); - const transaction = (res as any).__sentry_transaction; + const transaction = (res as any).__sentry_transaction as Transaction; expect(spanToJSON(transaction).description).toBe(`${method.toUpperCase()} ${path}`); }); @@ -416,7 +419,7 @@ describe('tracingHandler', () => { sentryTracingMiddleware(req, res, next); - const transaction = (res as any).__sentry_transaction; + const transaction = (res as any).__sentry_transaction as Transaction; expect(spanToJSON(transaction).description).toBe(`${method.toUpperCase()} ${path}`); }); From 93b4f9c683a6ee648bde6689ccfaaf634beb0f1a Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Fri, 22 Mar 2024 09:43:34 -0400 Subject: [PATCH 4/4] fix tests --- packages/node/test/handlers.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/node/test/handlers.test.ts b/packages/node/test/handlers.test.ts index eef7fb79a6a9..239b79c52564 100644 --- a/packages/node/test/handlers.test.ts +++ b/packages/node/test/handlers.test.ts @@ -290,7 +290,7 @@ describe('tracingHandler', () => { sentryTracingMiddleware(req, res, next); - const transaction = (res as any).__sentry_transaction; + const transaction = (res as any).__sentry_transaction as Transaction; expect(getPropagationContext()).toEqual({ traceId: '12312012123120121231201212312012', @@ -301,9 +301,10 @@ describe('tracingHandler', () => { }); // since we have no tracesSampler defined, the default behavior (inherit if possible) applies - expect(transaction.traceId).toEqual('12312012123120121231201212312012'); - expect(transaction.parentSpanId).toEqual('1121201211212012'); - expect(transaction.sampled).toEqual(false); + expect(transaction.spanContext().traceId).toEqual('12312012123120121231201212312012'); + expect(spanToJSON(transaction).parent_span_id).toEqual('1121201211212012'); + expect(spanIsSampled(transaction)).toEqual(false); + // eslint-disable-next-line deprecation/deprecation expect(transaction.metadata?.dynamicSamplingContext).toStrictEqual({}); });