From a56fc29884d905f469df3873e3178f2c2fc6f3af Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 10 Jan 2024 10:15:38 +0100 Subject: [PATCH] feat(core): Deprecate span `tags`, `data`, `context` & setters (#10053) Also deprecate direct access to `span.attributes` (instead use `spanToJSON(span)`). There are a few usages that we still need to figure out how we will replace them...! --- MIGRATION.md | 5 ++ .../startTransaction/basic_usage/test.ts | 1 + .../nextjs-app-dir/tests/transactions.test.ts | 6 +- .../mysql/withoutCallback/scenario.ts | 4 +- .../mysql/withoutCallback/test.ts | 12 ++-- docs/v8-new-performance-apis.md | 10 +-- .../browser/src/profiling/hubextensions.ts | 2 + packages/bun/src/integrations/bunserver.ts | 6 +- .../bun/test/integrations/bunserver.test.ts | 2 + packages/core/src/tracing/idletransaction.ts | 2 +- packages/core/src/tracing/span.ts | 63 ++++++++++++++----- packages/core/src/tracing/transaction.ts | 12 ++-- packages/core/src/utils/prepareEvent.ts | 11 +++- packages/core/test/lib/tracing/span.test.ts | 25 +++++--- .../common/withServerActionInstrumentation.ts | 13 ++-- packages/node/src/handlers.ts | 2 + packages/node/src/integrations/hapi/types.ts | 1 + packages/node/test/handlers.test.ts | 5 +- packages/node/test/integrations/http.test.ts | 21 ++++--- .../opentelemetry-node/src/spanprocessor.ts | 20 +++--- .../test/spanprocessor.test.ts | 26 ++++---- packages/opentelemetry/src/spanExporter.ts | 22 ++++--- packages/sveltekit/src/client/router.ts | 1 + packages/sveltekit/test/client/router.test.ts | 2 + .../src/browser/backgroundtab.ts | 2 + .../src/browser/browsertracing.ts | 1 + .../src/browser/metrics/index.ts | 20 +++++- .../tracing-internal/src/browser/request.ts | 7 ++- packages/tracing-internal/src/common/fetch.ts | 2 +- .../src/node/integrations/mysql.ts | 4 +- .../test/browser/backgroundtab.test.ts | 1 + .../test/browser/request.test.ts | 2 +- packages/types/src/span.ts | 13 +++- packages/types/src/transaction.ts | 12 ++-- packages/utils/src/requestdata.ts | 7 ++- packages/vue/src/router.ts | 3 + 36 files changed, 234 insertions(+), 114 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index c7587fc10c76..57fd79bcb18e 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -100,6 +100,11 @@ In v8, the Span class is heavily reworked. The following properties & methods ar * `span.description`: Use `spanToJSON(span).description` instead. * `transaction.setMetadata()`: Use attributes instead, or set data on the scope. * `transaction.metadata`: Use attributes instead, or set data on the scope. +* `span.tags`: Set tags on the surrounding scope instead, or use attributes. +* `span.data`: Use `spanToJSON(span).data` instead. +* `span.setTag()`: Use `span.setAttribute()` instead or set tags on the surrounding scope. +* `span.setData()`: Use `span.setAttribute()` instead. +* `transaction.setContext()`: Set context on the surrounding scope instead. ## Deprecate `pushScope` & `popScope` in favor of `withScope` diff --git a/dev-packages/browser-integration-tests/suites/public-api/startTransaction/basic_usage/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startTransaction/basic_usage/test.ts index 04e39e012639..1012ae61e1ff 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startTransaction/basic_usage/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startTransaction/basic_usage/test.ts @@ -31,6 +31,7 @@ sentryTest('should report finished spans as children of the root transaction', a const span_1 = transaction.spans?.[0]; expect(span_1?.op).toBe('span_1'); expect(span_1?.parentSpanId).toEqual(rootSpanId); + // eslint-disable-next-line deprecation/deprecation expect(span_1?.data).toMatchObject({ foo: 'bar', baz: [1, 2, 3] }); const span_3 = transaction.spans?.[1]; diff --git a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts index ca4f4cb0d1ff..f975ec49e606 100644 --- a/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts @@ -130,9 +130,9 @@ test('Should send a transaction for instrumented server actions', async ({ page await page.getByText('Run Action').click(); expect(await serverComponentTransactionPromise).toBeDefined(); - expect((await serverComponentTransactionPromise).contexts?.trace?.data?.['server_action_form_data']).toEqual( - expect.objectContaining({ 'some-text-value': 'some-default-value' }), - ); + expect( + (await serverComponentTransactionPromise).contexts?.trace?.data?.['server_action_form_data.some-text-value'], + ).toEqual('some-default-value'); expect((await serverComponentTransactionPromise).contexts?.trace?.data?.['server_action_result']).toEqual({ city: 'Vienna', }); diff --git a/dev-packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/withoutCallback/scenario.ts b/dev-packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/withoutCallback/scenario.ts index 9a436695d63f..2b06970b5243 100644 --- a/dev-packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/withoutCallback/scenario.ts +++ b/dev-packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/withoutCallback/scenario.ts @@ -32,10 +32,10 @@ const query = connection.query('SELECT 1 + 1 AS solution'); const query2 = connection.query('SELECT NOW()', ['1', '2']); query.on('end', () => { - transaction.setTag('result_done', 'yes'); + transaction.setAttribute('result_done', 'yes'); query2.on('end', () => { - transaction.setTag('result_done2', 'yes'); + transaction.setAttribute('result_done2', 'yes'); // Wait a bit to ensure the queries completed setTimeout(() => { diff --git a/dev-packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/withoutCallback/test.ts b/dev-packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/withoutCallback/test.ts index 0f6dee99d59b..f83e4297b8ba 100644 --- a/dev-packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/withoutCallback/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/withoutCallback/test.ts @@ -7,11 +7,15 @@ test('should auto-instrument `mysql` package when using query without callback', expect(envelope).toHaveLength(3); assertSentryTransaction(envelope[2], { - transaction: 'Test Transaction', - tags: { - result_done: 'yes', - result_done2: 'yes', + contexts: { + trace: { + data: { + result_done: 'yes', + result_done2: 'yes', + }, + }, }, + transaction: 'Test Transaction', spans: [ { description: 'SELECT 1 + 1 AS solution', diff --git a/docs/v8-new-performance-apis.md b/docs/v8-new-performance-apis.md index e7ec274bbb10..2f531858dd2c 100644 --- a/docs/v8-new-performance-apis.md +++ b/docs/v8-new-performance-apis.md @@ -50,8 +50,8 @@ below to see which things used to exist, and how they can/should be mapped going | `status` | use utility method TODO | | `sampled` | `spanIsSampled(span)` | | `startTimestamp` | `startTime` - note that this has a different format! | -| `tags` | `spanGetAttributes(span)`, or set tags on the scope | -| `data` | `spanGetAttributes(span)` | +| `tags` | use attributes, or set tags on the scope | +| `data` | `spanToJSON(span).data` | | `transaction` | ??? Removed | | `instrumenter` | Removed | | `finish()` | `end()` | @@ -72,13 +72,13 @@ In addition, a transaction has this API: | Old name | Replace with | | --------------------------- | ------------------------------------------------ | -| `name` | `spanGetName(span)` (TODO) | +| `name` | `spanToJSON(span).description` | | `trimEnd` | Removed | | `parentSampled` | `spanIsSampled(span)` & `spanContext().isRemote` | -| `metadata` | `spanGetMetadata(span)` | +| `metadata` | Use attributes instead or set on scope | | `setContext()` | Set context on scope instead | | `setMeasurement()` | ??? TODO | -| `setMetadata()` | `spanSetMetadata(span, metadata)` | +| `setMetadata()` | Use attributes instead or set on scope | | `getDynamicSamplingContext` | ??? TODO | ### Attributes vs. Data vs. Tags vs. Context diff --git a/packages/browser/src/profiling/hubextensions.ts b/packages/browser/src/profiling/hubextensions.ts index 01a3bfc2bfac..9fd156a1b90b 100644 --- a/packages/browser/src/profiling/hubextensions.ts +++ b/packages/browser/src/profiling/hubextensions.ts @@ -156,6 +156,8 @@ export function startProfileForTransaction(transaction: Transaction): Transactio // Always call onProfileHandler to ensure stopProfiling is called and the timeout is cleared. void onProfileHandler().then( () => { + // TODO: Can we rewrite this to use attributes? + // eslint-disable-next-line deprecation/deprecation transaction.setContext('profile', { profile_id: profileId, start_timestamp: startTimestamp }); originalEnd(); }, diff --git a/packages/bun/src/integrations/bunserver.ts b/packages/bun/src/integrations/bunserver.ts index 695b26edc144..fec3aae439af 100644 --- a/packages/bun/src/integrations/bunserver.ts +++ b/packages/bun/src/integrations/bunserver.ts @@ -4,6 +4,7 @@ import { captureException, continueTrace, convertIntegrationFnToClass, + getCurrentScope, runWithAsyncContext, startSpan, } from '@sentry/core'; @@ -90,9 +91,10 @@ function instrumentBunServeOptions(serveOptions: Parameters[0] >); if (response && response.status) { span?.setHttpStatus(response.status); - span?.setData('http.response.status_code', response.status); + span?.setAttribute('http.response.status_code', response.status); if (span instanceof Transaction) { - span.setContext('response', { + const scope = getCurrentScope(); + scope.setContext('response', { headers: response.headers.toJSON(), status_code: response.status, }); diff --git a/packages/bun/test/integrations/bunserver.test.ts b/packages/bun/test/integrations/bunserver.test.ts index 59b242b5ea7c..6356300562ac 100644 --- a/packages/bun/test/integrations/bunserver.test.ts +++ b/packages/bun/test/integrations/bunserver.test.ts @@ -26,6 +26,7 @@ describe('Bun Serve Integration', () => { test('generates a transaction around a request', async () => { client.on('finishTransaction', transaction => { expect(transaction.status).toBe('ok'); + // eslint-disable-next-line deprecation/deprecation expect(transaction.tags).toEqual({ 'http.status_code': '200', }); @@ -48,6 +49,7 @@ describe('Bun Serve Integration', () => { test('generates a post transaction', async () => { client.on('finishTransaction', transaction => { expect(transaction.status).toBe('ok'); + // eslint-disable-next-line deprecation/deprecation expect(transaction.tags).toEqual({ 'http.status_code': '200', }); diff --git a/packages/core/src/tracing/idletransaction.ts b/packages/core/src/tracing/idletransaction.ts index af567d5dcd22..2735146dbbc4 100644 --- a/packages/core/src/tracing/idletransaction.ts +++ b/packages/core/src/tracing/idletransaction.ts @@ -146,7 +146,7 @@ export class IdleTransaction extends Transaction { this.activities = {}; if (this.op === 'ui.action.click') { - this.setTag(FINISH_REASON_TAG, this._finishReason); + this.setAttribute(FINISH_REASON_TAG, this._finishReason); } if (this.spanRecorder) { diff --git a/packages/core/src/tracing/span.ts b/packages/core/src/tracing/span.ts index 80a1a3da4d32..10a4d97efa08 100644 --- a/packages/core/src/tracing/span.ts +++ b/packages/core/src/tracing/span.ts @@ -86,21 +86,18 @@ export class Span implements SpanInterface { public op?: string; /** - * @inheritDoc + * Tags for the span. + * @deprecated Use `getSpanAttributes(span)` instead. */ public tags: { [key: string]: Primitive }; /** - * @inheritDoc + * Data for the span. + * @deprecated Use `getSpanAttributes(span)` instead. */ // eslint-disable-next-line @typescript-eslint/no-explicit-any public data: { [key: string]: any }; - /** - * @inheritDoc - */ - public attributes: SpanAttributes; - /** * List of spans that were finalized */ @@ -125,6 +122,7 @@ export class Span implements SpanInterface { protected _spanId: string; protected _sampled: boolean | undefined; protected _name?: string; + protected _attributes: SpanAttributes; private _logMessage?: string; @@ -139,9 +137,11 @@ export class Span implements SpanInterface { this._traceId = spanContext.traceId || uuid4(); this._spanId = spanContext.spanId || uuid4().substring(16); this.startTimestamp = spanContext.startTimestamp || timestampInSeconds(); + // eslint-disable-next-line deprecation/deprecation this.tags = spanContext.tags ? { ...spanContext.tags } : {}; + // eslint-disable-next-line deprecation/deprecation this.data = spanContext.data ? { ...spanContext.data } : {}; - this.attributes = spanContext.attributes ? { ...spanContext.attributes } : {}; + this._attributes = spanContext.attributes ? { ...spanContext.attributes } : {}; this.instrumenter = spanContext.instrumenter || 'sentry'; this.origin = spanContext.origin || 'manual'; // eslint-disable-next-line deprecation/deprecation @@ -165,7 +165,7 @@ export class Span implements SpanInterface { } } - // This rule conflicts with another rule :( + // This rule conflicts with another eslint rule :( /* eslint-disable @typescript-eslint/member-ordering */ /** @@ -175,6 +175,7 @@ export class Span implements SpanInterface { public get name(): string { return this._name || ''; } + /** * Update the name of the span. * @deprecated Use `spanToJSON(span).description` instead. @@ -247,6 +248,22 @@ export class Span implements SpanInterface { this._sampled = sampled; } + /** + * Attributes for the span. + * @deprecated Use `getSpanAttributes(span)` instead. + */ + public get attributes(): SpanAttributes { + return this._attributes; + } + + /** + * Attributes for the span. + * @deprecated Use `setAttributes()` instead. + */ + public set attributes(attributes: SpanAttributes) { + this._attributes = attributes; + } + /* eslint-enable @typescript-eslint/member-ordering */ /** @inheritdoc */ @@ -296,18 +313,29 @@ export class Span implements SpanInterface { } /** - * @inheritDoc + * Sets the tag attribute on the current span. + * + * Can also be used to unset a tag, by passing `undefined`. + * + * @param key Tag key + * @param value Tag value + * @deprecated Use `setAttribute()` instead. */ public setTag(key: string, value: Primitive): this { + // eslint-disable-next-line deprecation/deprecation this.tags = { ...this.tags, [key]: value }; return this; } /** - * @inheritDoc + * Sets the data attribute on the current span + * @param key Data key + * @param value Data value + * @deprecated Use `setAttribute()` instead. */ // eslint-disable-next-line @typescript-eslint/no-explicit-any public setData(key: string, value: any): this { + // eslint-disable-next-line deprecation/deprecation this.data = { ...this.data, [key]: value }; return this; } @@ -316,9 +344,9 @@ export class Span implements SpanInterface { public setAttribute(key: string, value: SpanAttributeValue | undefined): void { if (value === undefined) { // eslint-disable-next-line @typescript-eslint/no-dynamic-delete - delete this.attributes[key]; + delete this._attributes[key]; } else { - this.attributes[key] = value; + this._attributes[key] = value; } } @@ -339,7 +367,9 @@ export class Span implements SpanInterface { * @inheritDoc */ public setHttpStatus(httpStatus: number): this { + // eslint-disable-next-line deprecation/deprecation this.setTag('http.status_code', String(httpStatus)); + // eslint-disable-next-line deprecation/deprecation this.setData('http.response.status_code', httpStatus); const spanStatus = spanStatusfromHttpCode(httpStatus); if (spanStatus !== 'unknown_error') { @@ -415,6 +445,7 @@ export class Span implements SpanInterface { spanId: this._spanId, startTimestamp: this.startTimestamp, status: this.status, + // eslint-disable-next-line deprecation/deprecation tags: this.tags, traceId: this._traceId, }); @@ -424,6 +455,7 @@ export class Span implements SpanInterface { * @inheritDoc */ public updateWithContext(spanContext: SpanContext): this { + // eslint-disable-next-line deprecation/deprecation this.data = spanContext.data || {}; // eslint-disable-next-line deprecation/deprecation this._name = spanContext.name || spanContext.description; @@ -434,6 +466,7 @@ export class Span implements SpanInterface { this._spanId = spanContext.spanId || this._spanId; this.startTimestamp = spanContext.startTimestamp || this.startTimestamp; this.status = spanContext.status; + // eslint-disable-next-line deprecation/deprecation this.tags = spanContext.tags || {}; this._traceId = spanContext.traceId || this._traceId; @@ -459,6 +492,7 @@ export class Span implements SpanInterface { span_id: this._spanId, start_timestamp: this.startTimestamp, status: this.status, + // eslint-disable-next-line deprecation/deprecation tags: Object.keys(this.tags).length > 0 ? this.tags : undefined, timestamp: this.endTimestamp, trace_id: this._traceId, @@ -490,7 +524,8 @@ export class Span implements SpanInterface { [key: string]: any; } | undefined { - const { data, attributes } = this; + // eslint-disable-next-line deprecation/deprecation + const { data, _attributes: attributes } = this; const hasData = Object.keys(data).length > 0; const hasAttributes = Object.keys(attributes).length > 0; diff --git a/packages/core/src/tracing/transaction.ts b/packages/core/src/tracing/transaction.ts index 7d13038de6e0..c68d9e6fbeeb 100644 --- a/packages/core/src/tracing/transaction.ts +++ b/packages/core/src/tracing/transaction.ts @@ -110,11 +110,11 @@ export class Transaction extends SpanClass implements TransactionInterface { ...this._metadata, // From attributes - ...(this.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] && { - source: this.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] as TransactionMetadata['source'], + ...(this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] && { + source: this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] as TransactionMetadata['source'], }), - ...(this.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] && { - sampleRate: this.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as TransactionMetadata['sampleRate'], + ...(this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] && { + sampleRate: this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as TransactionMetadata['sampleRate'], }), }; } @@ -157,7 +157,8 @@ export class Transaction extends SpanClass implements TransactionInterface { } /** - * @inheritDoc + * Set the context of a transaction event. + * @deprecated Use either `.setAttribute()`, or set the context on the scope before creating the transaction. */ public setContext(key: string, context: Context | null): void { if (context === null) { @@ -334,6 +335,7 @@ export class Transaction extends SpanClass implements TransactionInterface { // TODO: Pass spans serialized via `spanToJSON()` here instead in v8. spans: finishedSpans, start_timestamp: this.startTimestamp, + // eslint-disable-next-line deprecation/deprecation tags: this.tags, timestamp: this.endTimestamp, transaction: this._name, diff --git a/packages/core/src/utils/prepareEvent.ts b/packages/core/src/utils/prepareEvent.ts index 6f448df8496d..17be8c5354de 100644 --- a/packages/core/src/utils/prepareEvent.ts +++ b/packages/core/src/utils/prepareEvent.ts @@ -15,6 +15,7 @@ import { DEFAULT_ENVIRONMENT } from '../constants'; import { getGlobalEventProcessors, notifyEventProcessors } from '../eventProcessors'; import { Scope, getGlobalScope } from '../scope'; import { applyScopeDataToEvent, mergeScopeData } from './applyScopeDataToEvent'; +import { spanToJSON } from './spanUtils'; /** * This type makes sure that we get either a CaptureContext, OR an EventHint. @@ -326,10 +327,14 @@ function normalizeEvent(event: Event | null, depth: number, maxBreadth: number): // event.spans[].data may contain circular/dangerous data so we need to normalize it if (event.spans) { normalized.spans = event.spans.map(span => { - // We cannot use the spread operator here because `toJSON` on `span` is non-enumerable - if (span.data) { - span.data = normalize(span.data, depth, maxBreadth); + const data = spanToJSON(span).data; + + if (data) { + // This is a bit weird, as we generally have `Span` instances here, but to be safe we do not assume so + // eslint-disable-next-line deprecation/deprecation + span.data = normalize(data, depth, maxBreadth); } + return span; }); } diff --git a/packages/core/test/lib/tracing/span.test.ts b/packages/core/test/lib/tracing/span.test.ts index 3b48cb3d8640..1adff93123ac 100644 --- a/packages/core/test/lib/tracing/span.test.ts +++ b/packages/core/test/lib/tracing/span.test.ts @@ -74,7 +74,7 @@ describe('span', () => { span.setAttribute('boolArray', [true, false]); span.setAttribute('arrayWithUndefined', [1, undefined, 2]); - expect(span.attributes).toEqual({ + expect(span['_attributes']).toEqual({ str: 'bar', num: 1, zero: 0, @@ -92,11 +92,11 @@ describe('span', () => { span.setAttribute('str', 'bar'); - expect(Object.keys(span.attributes).length).toEqual(1); + expect(Object.keys(span['_attributes']).length).toEqual(1); span.setAttribute('str', undefined); - expect(Object.keys(span.attributes).length).toEqual(0); + expect(Object.keys(span['_attributes']).length).toEqual(0); }); it('disallows invalid attribute types', () => { @@ -117,7 +117,7 @@ describe('span', () => { it('allows to set attributes', () => { const span = new Span(); - const initialAttributes = span.attributes; + const initialAttributes = span['_attributes']; expect(initialAttributes).toEqual({}); @@ -135,7 +135,7 @@ describe('span', () => { }; span.setAttributes(newAttributes); - expect(span.attributes).toEqual({ + expect(span['_attributes']).toEqual({ str: 'bar', num: 1, zero: 0, @@ -147,14 +147,14 @@ describe('span', () => { arrayWithUndefined: [1, undefined, 2], }); - expect(span.attributes).not.toBe(newAttributes); + expect(span['_attributes']).not.toBe(newAttributes); span.setAttributes({ num: 2, numArray: [3, 4], }); - expect(span.attributes).toEqual({ + expect(span['_attributes']).toEqual({ str: 'bar', num: 2, zero: 0, @@ -172,11 +172,11 @@ describe('span', () => { span.setAttribute('str', 'bar'); - expect(Object.keys(span.attributes).length).toEqual(1); + expect(Object.keys(span['_attributes']).length).toEqual(1); span.setAttributes({ str: undefined }); - expect(Object.keys(span.attributes).length).toEqual(0); + expect(Object.keys(span['_attributes']).length).toEqual(0); }); }); @@ -270,9 +270,11 @@ describe('span', () => { it('works with data only', () => { const span = new Span(); + // eslint-disable-next-line deprecation/deprecation span.setData('foo', 'bar'); expect(span['_getData']()).toEqual({ foo: 'bar' }); + // eslint-disable-next-line deprecation/deprecation expect(span['_getData']()).toBe(span.data); }); @@ -281,6 +283,7 @@ describe('span', () => { span.setAttribute('foo', 'bar'); expect(span['_getData']()).toEqual({ foo: 'bar' }); + // eslint-disable-next-line deprecation/deprecation expect(span['_getData']()).toBe(span.attributes); }); @@ -288,11 +291,15 @@ describe('span', () => { const span = new Span(); span.setAttribute('foo', 'foo'); span.setAttribute('bar', 'bar'); + // eslint-disable-next-line deprecation/deprecation span.setData('foo', 'foo2'); + // eslint-disable-next-line deprecation/deprecation span.setData('baz', 'baz'); expect(span['_getData']()).toEqual({ foo: 'foo', bar: 'bar', baz: 'baz' }); + // eslint-disable-next-line deprecation/deprecation expect(span['_getData']()).not.toBe(span.attributes); + // eslint-disable-next-line deprecation/deprecation expect(span['_getData']()).not.toBe(span.data); }); }); diff --git a/packages/nextjs/src/common/withServerActionInstrumentation.ts b/packages/nextjs/src/common/withServerActionInstrumentation.ts index 85872ac7d703..01e1c75d6f3f 100644 --- a/packages/nextjs/src/common/withServerActionInstrumentation.ts +++ b/packages/nextjs/src/common/withServerActionInstrumentation.ts @@ -104,19 +104,16 @@ async function withServerActionInstrumentationImplementation = {}; options.formData.forEach((value, key) => { - if (typeof value === 'string') { - formDataObject[key] = value; - } else { - formDataObject[key] = '[non-string value]'; - } + span?.setAttribute( + `server_action_form_data.${key}`, + typeof value === 'string' ? value : '[non-string value]', + ); }); - span?.setData('server_action_form_data', formDataObject); } return result; diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index 3a4738d8d2f3..892aabd2dd84 100644 --- a/packages/node/src/handlers.ts +++ b/packages/node/src/handlers.ts @@ -351,6 +351,8 @@ export function trpcMiddleware(options: SentryTrpcMiddlewareOptions = {}) { trpcContext.input = normalize(rawInput); } + // TODO: Can we rewrite this to an attribute? Or set this on the scope? + // eslint-disable-next-line deprecation/deprecation sentryTransaction.setContext('trpc', trpcContext); } diff --git a/packages/node/src/integrations/hapi/types.ts b/packages/node/src/integrations/hapi/types.ts index d74c171ef441..a650667fe362 100644 --- a/packages/node/src/integrations/hapi/types.ts +++ b/packages/node/src/integrations/hapi/types.ts @@ -3,6 +3,7 @@ /* eslint-disable @typescript-eslint/unified-signatures */ /* eslint-disable @typescript-eslint/no-empty-interface */ /* eslint-disable @typescript-eslint/no-namespace */ +/* eslint-disable @typescript-eslint/no-explicit-any */ // Vendored and simplified from: // - @types/hapi__hapi diff --git a/packages/node/test/handlers.test.ts b/packages/node/test/handlers.test.ts index 6a25a1bcc4b0..888991de439c 100644 --- a/packages/node/test/handlers.test.ts +++ b/packages/node/test/handlers.test.ts @@ -372,8 +372,11 @@ describe('tracingHandler', () => { setImmediate(() => { expect(finishTransaction).toHaveBeenCalled(); expect(transaction.status).toBe('ok'); + // eslint-disable-next-line deprecation/deprecation expect(transaction.tags).toEqual(expect.objectContaining({ 'http.status_code': '200' })); - expect(transaction.data).toEqual(expect.objectContaining({ 'http.response.status_code': 200 })); + expect(sentryCore.spanToJSON(transaction).data).toEqual( + expect.objectContaining({ 'http.response.status_code': 200 }), + ); done(); }); }); diff --git a/packages/node/test/integrations/http.test.ts b/packages/node/test/integrations/http.test.ts index 5a1603988ee2..0f1ad687684d 100644 --- a/packages/node/test/integrations/http.test.ts +++ b/packages/node/test/integrations/http.test.ts @@ -300,10 +300,13 @@ describe('tracing', () => { // our span is at index 1 because the transaction itself is at index 0 expect(sentryCore.spanToJSON(spans[1]).description).toEqual('GET http://dogs.are.great/spaniel'); expect(spans[1].op).toEqual('http.client'); - expect(spans[1].data['http.method']).toEqual('GET'); - expect(spans[1].data.url).toEqual('http://dogs.are.great/spaniel'); - expect(spans[1].data['http.query']).toEqual('tail=wag&cute=true'); - expect(spans[1].data['http.fragment']).toEqual('learn-more'); + + const spanAttributes = sentryCore.spanToJSON(spans[1]).data || {}; + + expect(spanAttributes['http.method']).toEqual('GET'); + expect(spanAttributes.url).toEqual('http://dogs.are.great/spaniel'); + expect(spanAttributes['http.query']).toEqual('tail=wag&cute=true'); + expect(spanAttributes['http.fragment']).toEqual('learn-more'); }); it('fills in span data from http.RequestOptions object', () => { @@ -316,13 +319,15 @@ describe('tracing', () => { expect(spans.length).toEqual(2); + const spanAttributes = sentryCore.spanToJSON(spans[1]).data || {}; + // our span is at index 1 because the transaction itself is at index 0 expect(sentryCore.spanToJSON(spans[1]).description).toEqual('GET http://dogs.are.great/spaniel'); expect(spans[1].op).toEqual('http.client'); - expect(spans[1].data['http.method']).toEqual('GET'); - expect(spans[1].data.url).toEqual('http://dogs.are.great/spaniel'); - expect(spans[1].data['http.query']).toEqual('tail=wag&cute=true'); - expect(spans[1].data['http.fragment']).toEqual('learn-more'); + expect(spanAttributes['http.method']).toEqual('GET'); + expect(spanAttributes.url).toEqual('http://dogs.are.great/spaniel'); + expect(spanAttributes['http.query']).toEqual('tail=wag&cute=true'); + expect(spanAttributes['http.fragment']).toEqual('learn-more'); }); it.each([ diff --git a/packages/opentelemetry-node/src/spanprocessor.ts b/packages/opentelemetry-node/src/spanprocessor.ts index b6ca1fe53254..dcec2ebeef43 100644 --- a/packages/opentelemetry-node/src/spanprocessor.ts +++ b/packages/opentelemetry-node/src/spanprocessor.ts @@ -194,14 +194,13 @@ function updateSpanWithOtelData(sentrySpan: SentrySpan, otelSpan: OtelSpan): voi const { op, description, data } = parseOtelSpanDescription(otelSpan); sentrySpan.setStatus(mapOtelStatus(otelSpan)); - sentrySpan.setData('otel.kind', SpanKind[kind]); - const allData = { ...attributes, ...data }; - - Object.keys(allData).forEach(prop => { - const value = allData[prop]; - sentrySpan.setData(prop, value); - }); + const allData = { + ...attributes, + ...data, + 'otel.kind': SpanKind[kind], + }; + sentrySpan.setAttributes(allData); sentrySpan.op = op; sentrySpan.updateName(description); @@ -210,17 +209,14 @@ function updateSpanWithOtelData(sentrySpan: SentrySpan, otelSpan: OtelSpan): voi function updateTransactionWithOtelData(transaction: Transaction, otelSpan: OtelSpan): void { const { op, description, source, data } = parseOtelSpanDescription(otelSpan); + // eslint-disable-next-line deprecation/deprecation transaction.setContext('otel', { attributes: otelSpan.attributes, resource: otelSpan.resource.attributes, }); const allData = data || {}; - - Object.keys(allData).forEach(prop => { - const value = allData[prop]; - transaction.setData(prop, value); - }); + transaction.setAttributes(allData); transaction.setStatus(mapOtelStatus(otelSpan)); diff --git a/packages/opentelemetry-node/test/spanprocessor.test.ts b/packages/opentelemetry-node/test/spanprocessor.test.ts index c1d3c209ecca..f4bc26041ceb 100644 --- a/packages/opentelemetry-node/test/spanprocessor.test.ts +++ b/packages/opentelemetry-node/test/spanprocessor.test.ts @@ -320,11 +320,11 @@ describe('SentrySpanProcessor', () => { const sentrySpan = getSpanForOtelSpan(child); - expect(sentrySpan?.data).toEqual({}); + expect(spanToJSON(sentrySpan!).data).toEqual(undefined); child.end(); - expect(sentrySpan?.data).toEqual({ + expect(spanToJSON(sentrySpan!).data).toEqual({ 'otel.kind': 'INTERNAL', 'test-attribute': 'test-value', 'test-attribute-2': [1, 2, 3], @@ -539,8 +539,10 @@ describe('SentrySpanProcessor', () => { child.end(); - expect(sentrySpan ? spanToJSON(sentrySpan).description : undefined).toBe('GET /my/route/{id}'); - expect(sentrySpan?.data).toEqual({ + const { description, data } = spanToJSON(sentrySpan!); + + expect(description).toBe('GET /my/route/{id}'); + expect(data).toEqual({ 'http.method': 'GET', 'http.route': '/my/route/{id}', 'http.target': '/my/route/123', @@ -567,10 +569,10 @@ describe('SentrySpanProcessor', () => { child.end(); - expect(sentrySpan ? spanToJSON(sentrySpan).description : undefined).toBe( - 'GET http://example.com/my/route/123', - ); - expect(sentrySpan?.data).toEqual({ + const { description, data } = spanToJSON(sentrySpan!); + + expect(description).toBe('GET http://example.com/my/route/123'); + expect(data).toEqual({ 'http.method': 'GET', 'http.target': '/my/route/123', 'http.url': 'http://example.com/my/route/123', @@ -596,10 +598,10 @@ describe('SentrySpanProcessor', () => { child.end(); - expect(sentrySpan ? spanToJSON(sentrySpan).description : undefined).toBe( - 'GET http://example.com/my/route/123', - ); - expect(sentrySpan?.data).toEqual({ + const { description, data } = spanToJSON(sentrySpan!); + + expect(description).toBe('GET http://example.com/my/route/123'); + expect(data).toEqual({ 'http.method': 'GET', 'http.target': '/my/route/123', 'http.url': 'http://example.com/my/route/123?what=123#myHash', diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index 78557e50d33c..7394e413e7ad 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -3,8 +3,8 @@ import type { ExportResult } from '@opentelemetry/core'; import { ExportResultCode } from '@opentelemetry/core'; import type { ReadableSpan, SpanExporter } from '@opentelemetry/sdk-trace-base'; import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; -import { flush } from '@sentry/core'; -import type { DynamicSamplingContext, Span as SentrySpan, SpanOrigin, TransactionSource } from '@sentry/types'; +import { flush, getCurrentScope } from '@sentry/core'; +import type { DynamicSamplingContext, Scope, Span as SentrySpan, SpanOrigin, TransactionSource } from '@sentry/types'; import { logger } from '@sentry/utils'; import { getCurrentHub } from './custom/hub'; @@ -110,7 +110,7 @@ function maybeSend(spans: ReadableSpan[]): ReadableSpan[] { // Now finish the transaction, which will send it together with all the spans // We make sure to use the finish scope - const scope = getSpanFinishScope(span); + const scope = getScopeForTransactionFinish(span); transaction.finishWithScope(convertOtelTimeToSeconds(span.endTime), scope); }); @@ -119,6 +119,17 @@ function maybeSend(spans: ReadableSpan[]): ReadableSpan[] { .filter((span): span is ReadableSpan => !!span); } +function getScopeForTransactionFinish(span: ReadableSpan): Scope { + // The finish scope should normally always be there (and it is already a clone), + // but for the sake of type safety we fall back to a clone of the current scope + const scope = getSpanFinishScope(span) || getCurrentScope().clone(); + scope.setContext('otel', { + attributes: removeSentryAttributes(span.attributes), + resource: span.resource.attributes, + }); + return scope; +} + function getCompletedRootNodes(nodes: SpanNode[]): SpanNodeCompleted[] { return nodes.filter((node): node is SpanNodeCompleted => !!node.span && !node.parentNode); } @@ -176,11 +187,6 @@ function createTransactionForOtelSpan(span: ReadableSpan): OpenTelemetryTransact sampled: true, }) as OpenTelemetryTransaction; - transaction.setContext('otel', { - attributes: removeSentryAttributes(span.attributes), - resource: span.resource.attributes, - }); - return transaction; } diff --git a/packages/sveltekit/src/client/router.ts b/packages/sveltekit/src/client/router.ts index f6601eb940b2..2b36d4adb4f2 100644 --- a/packages/sveltekit/src/client/router.ts +++ b/packages/sveltekit/src/client/router.ts @@ -124,6 +124,7 @@ function instrumentNavigations(startTransactionFn: (context: TransactionContext) description: 'SvelteKit Route Change', origin: 'auto.ui.sveltekit', }); + // eslint-disable-next-line deprecation/deprecation activeTransaction.setTag('from', parameterizedRouteOrigin); } }); diff --git a/packages/sveltekit/test/client/router.test.ts b/packages/sveltekit/test/client/router.test.ts index 8c4c9187a7ff..29037c28461f 100644 --- a/packages/sveltekit/test/client/router.test.ts +++ b/packages/sveltekit/test/client/router.test.ts @@ -123,6 +123,7 @@ describe('sveltekitRoutingInstrumentation', () => { description: 'SvelteKit Route Change', }); + // eslint-disable-next-line deprecation/deprecation expect(returnedTransaction?.setTag).toHaveBeenCalledWith('from', '/users'); // We emit `null` here to simulate the end of the navigation lifecycle @@ -173,6 +174,7 @@ describe('sveltekitRoutingInstrumentation', () => { description: 'SvelteKit Route Change', }); + // eslint-disable-next-line deprecation/deprecation expect(returnedTransaction?.setTag).toHaveBeenCalledWith('from', '/users/[id]'); }); diff --git a/packages/tracing-internal/src/browser/backgroundtab.ts b/packages/tracing-internal/src/browser/backgroundtab.ts index 67385b665be4..849f20ad20de 100644 --- a/packages/tracing-internal/src/browser/backgroundtab.ts +++ b/packages/tracing-internal/src/browser/backgroundtab.ts @@ -26,6 +26,8 @@ export function registerBackgroundTabDetection(): void { if (!activeTransaction.status) { activeTransaction.setStatus(statusType); } + // TODO: Can we rewrite this to an attribute? + // eslint-disable-next-line deprecation/deprecation activeTransaction.setTag('visibilitychange', 'document.hidden'); activeTransaction.end(); } diff --git a/packages/tracing-internal/src/browser/browsertracing.ts b/packages/tracing-internal/src/browser/browsertracing.ts index 1c8723a9705b..083e76b08ecf 100644 --- a/packages/tracing-internal/src/browser/browsertracing.ts +++ b/packages/tracing-internal/src/browser/browsertracing.ts @@ -343,6 +343,7 @@ export class BrowserTracing implements Integration { this._latestRouteName = finalContext.name; + // eslint-disable-next-line deprecation/deprecation const sourceFromData = context.data && context.data[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]; // eslint-disable-next-line deprecation/deprecation const sourceFromMetadata = finalContext.metadata && finalContext.metadata.source; diff --git a/packages/tracing-internal/src/browser/metrics/index.ts b/packages/tracing-internal/src/browser/metrics/index.ts index 72cce4f7aad6..df7cde3e1703 100644 --- a/packages/tracing-internal/src/browser/metrics/index.ts +++ b/packages/tracing-internal/src/browser/metrics/index.ts @@ -115,7 +115,7 @@ export function startTrackingInteractions(): void { const componentName = getComponentName(entry.target); if (componentName) { - span.data = { 'ui.component_name': componentName }; + span.attributes = { 'ui.component_name': componentName }; } // eslint-disable-next-line deprecation/deprecation @@ -448,10 +448,14 @@ function _trackNavigator(transaction: Transaction): void { const connection = navigator.connection; if (connection) { if (connection.effectiveType) { + // TODO: Can we rewrite this to an attribute? + // eslint-disable-next-line deprecation/deprecation transaction.setTag('effectiveConnectionType', connection.effectiveType); } if (connection.type) { + // TODO: Can we rewrite this to an attribute? + // eslint-disable-next-line deprecation/deprecation transaction.setTag('connectionType', connection.type); } @@ -461,10 +465,14 @@ function _trackNavigator(transaction: Transaction): void { } if (isMeasurementValue(navigator.deviceMemory)) { + // TODO: Can we rewrite this to an attribute? + // eslint-disable-next-line deprecation/deprecation transaction.setTag('deviceMemory', `${navigator.deviceMemory} GB`); } if (isMeasurementValue(navigator.hardwareConcurrency)) { + // TODO: Can we rewrite this to an attribute? + // eslint-disable-next-line deprecation/deprecation transaction.setTag('hardwareConcurrency', String(navigator.hardwareConcurrency)); } } @@ -477,18 +485,26 @@ function _tagMetricInfo(transaction: Transaction): void { // Capture Properties of the LCP element that contributes to the LCP. if (_lcpEntry.element) { + // TODO: Can we rewrite this to an attribute? + // eslint-disable-next-line deprecation/deprecation transaction.setTag('lcp.element', htmlTreeAsString(_lcpEntry.element)); } if (_lcpEntry.id) { + // TODO: Can we rewrite this to an attribute? + // eslint-disable-next-line deprecation/deprecation transaction.setTag('lcp.id', _lcpEntry.id); } if (_lcpEntry.url) { // Trim URL to the first 200 characters. + // TODO: Can we rewrite this to an attribute? + // eslint-disable-next-line deprecation/deprecation transaction.setTag('lcp.url', _lcpEntry.url.trim().slice(0, 200)); } + // TODO: Can we rewrite this to an attribute? + // eslint-disable-next-line deprecation/deprecation transaction.setTag('lcp.size', _lcpEntry.size); } @@ -496,6 +512,8 @@ function _tagMetricInfo(transaction: Transaction): void { if (_clsEntry && _clsEntry.sources) { DEBUG_BUILD && logger.log('[Measurements] Adding CLS Data'); _clsEntry.sources.forEach((source, index) => + // TODO: Can we rewrite this to an attribute? + // eslint-disable-next-line deprecation/deprecation transaction.setTag(`cls.source.${index + 1}`, htmlTreeAsString(source.node)), ); } diff --git a/packages/tracing-internal/src/browser/request.ts b/packages/tracing-internal/src/browser/request.ts index d7c4c7892189..9fb5a5cc3439 100644 --- a/packages/tracing-internal/src/browser/request.ts +++ b/packages/tracing-internal/src/browser/request.ts @@ -5,6 +5,7 @@ import { getCurrentScope, getDynamicSamplingContextFromClient, hasTracingEnabled, + spanToJSON, spanToTraceHeader, } from '@sentry/core'; import type { HandlerDataXhr, SentryWrappedXMLHttpRequest, Span } from '@sentry/types'; @@ -146,9 +147,9 @@ function isPerformanceResourceTiming(entry: PerformanceEntry): entry is Performa * @param span A span that has yet to be finished, must contain `url` on data. */ function addHTTPTimings(span: Span): void { - const url = span.data.url; + const { url } = spanToJSON(span).data || {}; - if (!url) { + if (!url || typeof url !== 'string') { return; } @@ -156,7 +157,7 @@ function addHTTPTimings(span: Span): void { entries.forEach(entry => { if (isPerformanceResourceTiming(entry) && entry.name.endsWith(url)) { const spanData = resourceTimingEntryToSpanData(entry); - spanData.forEach(data => span.setData(...data)); + spanData.forEach(data => span.setAttribute(...data)); // In the next tick, clean this handler up // We have to wait here because otherwise this cleans itself up before it is fully done setTimeout(cleanup); diff --git a/packages/tracing-internal/src/common/fetch.ts b/packages/tracing-internal/src/common/fetch.ts index cc04885e4436..d6cf469cadc8 100644 --- a/packages/tracing-internal/src/common/fetch.ts +++ b/packages/tracing-internal/src/common/fetch.ts @@ -58,7 +58,7 @@ export function instrumentFetchRequest( if (contentLength) { const contentLengthNum = parseInt(contentLength); if (contentLengthNum > 0) { - span.setData('http.response_content_length', contentLengthNum); + span.setAttribute('http.response_content_length', contentLengthNum); } } } else if (handlerData.error) { diff --git a/packages/tracing-internal/src/node/integrations/mysql.ts b/packages/tracing-internal/src/node/integrations/mysql.ts index 16753c4a9e08..9756b200e06b 100644 --- a/packages/tracing-internal/src/node/integrations/mysql.ts +++ b/packages/tracing-internal/src/node/integrations/mysql.ts @@ -73,7 +73,7 @@ export class Mysql implements LazyLoadedIntegration { DEBUG_BUILD && logger.error('Mysql Integration was unable to instrument `mysql` config.'); } - function spanDataFromConfig(): Record { + function spanDataFromConfig(): Record { if (!mySqlConfig) { return {}; } @@ -91,7 +91,7 @@ export class Mysql implements LazyLoadedIntegration { const data = spanDataFromConfig(); Object.keys(data).forEach(key => { - span.setData(key, data[key]); + span.setAttribute(key, data[key]); }); span.end(); diff --git a/packages/tracing-internal/test/browser/backgroundtab.test.ts b/packages/tracing-internal/test/browser/backgroundtab.test.ts index dcb423fd094a..215a9c5d6583 100644 --- a/packages/tracing-internal/test/browser/backgroundtab.test.ts +++ b/packages/tracing-internal/test/browser/backgroundtab.test.ts @@ -55,6 +55,7 @@ conditionalTest({ min: 10 })('registerBackgroundTabDetection', () => { events.visibilitychange(); expect(span?.status).toBe('cancelled'); + // eslint-disable-next-line deprecation/deprecation expect(span?.tags.visibilitychange).toBe('document.hidden'); expect(span?.endTimestamp).toBeDefined(); }); diff --git a/packages/tracing-internal/test/browser/request.test.ts b/packages/tracing-internal/test/browser/request.test.ts index 0f3ce191278a..3dabe104c8f6 100644 --- a/packages/tracing-internal/test/browser/request.test.ts +++ b/packages/tracing-internal/test/browser/request.test.ts @@ -256,7 +256,7 @@ describe('callbacks', () => { expect(finishedSpan).toBeDefined(); expect(finishedSpan).toBeInstanceOf(Span); - expect(finishedSpan.data).toEqual({ + expect(sentryCore.spanToJSON(finishedSpan).data).toEqual({ 'http.response_content_length': 123, 'http.method': 'GET', 'http.response.status_code': 404, diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index 1538fc343e52..3f54322886bb 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -129,11 +129,13 @@ export interface SpanContext { /** * Tags of the Span. + * @deprecated Pass `attributes` instead. */ tags?: { [key: string]: Primitive }; /** * Data of the Span. + * @deprecated Pass `attributes` instead. */ data?: { [key: string]: any }; @@ -195,17 +197,20 @@ export interface Span extends SpanContext { startTimestamp: number; /** - * @inheritDoc + * Tags for the span. + * @deprecated Use `getSpanAttributes(span)` instead. */ tags: { [key: string]: Primitive }; /** - * @inheritDoc + * Data for the span. + * @deprecated Use `getSpanAttributes(span)` instead. */ data: { [key: string]: any }; /** - * @inheritDoc + * Attributes for the span. + * @deprecated Use `getSpanAttributes(span)` instead. */ attributes: SpanAttributes; @@ -243,6 +248,7 @@ export interface Span extends SpanContext { * * @param key Tag key * @param value Tag value + * @deprecated Use `setAttribute()` instead. */ setTag(key: string, value: Primitive): this; @@ -250,6 +256,7 @@ export interface Span extends SpanContext { * Sets the data attribute on the current span * @param key Data key * @param value Data value + * @deprecated Use `setAttribute()` instead. */ setData(key: string, value: any): this; diff --git a/packages/types/src/transaction.ts b/packages/types/src/transaction.ts index d07c5ce435c1..586523625710 100644 --- a/packages/types/src/transaction.ts +++ b/packages/types/src/transaction.ts @@ -74,17 +74,20 @@ export interface Transaction extends TransactionContext, Omit