Skip to content

Commit a56fc29

Browse files
mydeamydea
authored
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...!
1 parent a157d98 commit a56fc29

File tree

36 files changed

+234
-114
lines changed

36 files changed

+234
-114
lines changed

‎MIGRATION.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,11 @@ In v8, the Span class is heavily reworked. The following properties & methods ar
100100
* `span.description`: Use `spanToJSON(span).description` instead.
101101
* `transaction.setMetadata()`: Use attributes instead, or set data on the scope.
102102
* `transaction.metadata`: Use attributes instead, or set data on the scope.
103+
* `span.tags`: Set tags on the surrounding scope instead, or use attributes.
104+
* `span.data`: Use `spanToJSON(span).data` instead.
105+
* `span.setTag()`: Use `span.setAttribute()` instead or set tags on the surrounding scope.
106+
* `span.setData()`: Use `span.setAttribute()` instead.
107+
* `transaction.setContext()`: Set context on the surrounding scope instead.
103108

104109
## Deprecate `pushScope` & `popScope` in favor of `withScope`
105110

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ sentryTest('should report finished spans as children of the root transaction', a
3131
const span_1 = transaction.spans?.[0];
3232
expect(span_1?.op).toBe('span_1');
3333
expect(span_1?.parentSpanId).toEqual(rootSpanId);
34+
// eslint-disable-next-line deprecation/deprecation
3435
expect(span_1?.data).toMatchObject({ foo: 'bar', baz: [1, 2, 3] });
3536

3637
const span_3 = transaction.spans?.[1];

‎dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,9 @@ test('Should send a transaction for instrumented server actions', async ({ page
130130
await page.getByText('Run Action').click();
131131

132132
expect(await serverComponentTransactionPromise).toBeDefined();
133-
expect((await serverComponentTransactionPromise).contexts?.trace?.data?.['server_action_form_data']).toEqual(
134-
expect.objectContaining({ 'some-text-value': 'some-default-value' }),
135-
);
133+
expect(
134+
(await serverComponentTransactionPromise).contexts?.trace?.data?.['server_action_form_data.some-text-value'],
135+
).toEqual('some-default-value');
136136
expect((await serverComponentTransactionPromise).contexts?.trace?.data?.['server_action_result']).toEqual({
137137
city: 'Vienna',
138138
});

‎dev-packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/withoutCallback/scenario.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,10 @@ const query = connection.query('SELECT 1 + 1 AS solution');
3232
const query2 = connection.query('SELECT NOW()', ['1', '2']);
3333

3434
query.on('end', () => {
35-
transaction.setTag('result_done', 'yes');
35+
transaction.setAttribute('result_done', 'yes');
3636

3737
query2.on('end', () => {
38-
transaction.setTag('result_done2', 'yes');
38+
transaction.setAttribute('result_done2', 'yes');
3939

4040
// Wait a bit to ensure the queries completed
4141
setTimeout(() => {

‎dev-packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/withoutCallback/test.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,15 @@ test('should auto-instrument `mysql` package when using query without callback',
77
expect(envelope).toHaveLength(3);
88

99
assertSentryTransaction(envelope[2], {
10-
transaction: 'Test Transaction',
11-
tags: {
12-
result_done: 'yes',
13-
result_done2: 'yes',
10+
contexts: {
11+
trace: {
12+
data: {
13+
result_done: 'yes',
14+
result_done2: 'yes',
15+
},
16+
},
1417
},
18+
transaction: 'Test Transaction',
1519
spans: [
1620
{
1721
description: 'SELECT 1 + 1 AS solution',

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ below to see which things used to exist, and how they can/should be mapped going
5050
| `status` | use utility method TODO |
5151
| `sampled` | `spanIsSampled(span)` |
5252
| `startTimestamp` | `startTime` - note that this has a different format! |
53-
| `tags` | `spanGetAttributes(span)`, or set tags on the scope |
54-
| `data` | `spanGetAttributes(span)` |
53+
| `tags` | use attributes, or set tags on the scope |
54+
| `data` | `spanToJSON(span).data` |
5555
| `transaction` | ??? Removed |
5656
| `instrumenter` | Removed |
5757
| `finish()` | `end()` |
@@ -72,13 +72,13 @@ In addition, a transaction has this API:
7272

7373
| Old name | Replace with |
7474
| --------------------------- | ------------------------------------------------ |
75-
| `name` | `spanGetName(span)` (TODO) |
75+
| `name` | `spanToJSON(span).description` |
7676
| `trimEnd` | Removed |
7777
| `parentSampled` | `spanIsSampled(span)` & `spanContext().isRemote` |
78-
| `metadata` | `spanGetMetadata(span)` |
78+
| `metadata` | Use attributes instead or set on scope |
7979
| `setContext()` | Set context on scope instead |
8080
| `setMeasurement()` | ??? TODO |
81-
| `setMetadata()` | `spanSetMetadata(span, metadata)` |
81+
| `setMetadata()` | Use attributes instead or set on scope |
8282
| `getDynamicSamplingContext` | ??? TODO |
8383

8484
### Attributes vs. Data vs. Tags vs. Context

‎packages/browser/src/profiling/hubextensions.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,8 @@ export function startProfileForTransaction(transaction: Transaction): Transactio
156156
// Always call onProfileHandler to ensure stopProfiling is called and the timeout is cleared.
157157
void onProfileHandler().then(
158158
() => {
159+
// TODO: Can we rewrite this to use attributes?
160+
// eslint-disable-next-line deprecation/deprecation
159161
transaction.setContext('profile', { profile_id: profileId, start_timestamp: startTimestamp });
160162
originalEnd();
161163
},

‎packages/bun/src/integrations/bunserver.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
captureException,
55
continueTrace,
66
convertIntegrationFnToClass,
7+
getCurrentScope,
78
runWithAsyncContext,
89
startSpan,
910
} from '@sentry/core';
@@ -90,9 +91,10 @@ function instrumentBunServeOptions(serveOptions: Parameters<typeof Bun.serve>[0]
9091
>);
9192
if (response && response.status) {
9293
span?.setHttpStatus(response.status);
93-
span?.setData('http.response.status_code', response.status);
94+
span?.setAttribute('http.response.status_code', response.status);
9495
if (span instanceof Transaction) {
95-
span.setContext('response', {
96+
const scope = getCurrentScope();
97+
scope.setContext('response', {
9698
headers: response.headers.toJSON(),
9799
status_code: response.status,
98100
});

‎packages/bun/test/integrations/bunserver.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ describe('Bun Serve Integration', () => {
2626
test('generates a transaction around a request', async () => {
2727
client.on('finishTransaction', transaction => {
2828
expect(transaction.status).toBe('ok');
29+
// eslint-disable-next-line deprecation/deprecation
2930
expect(transaction.tags).toEqual({
3031
'http.status_code': '200',
3132
});
@@ -48,6 +49,7 @@ describe('Bun Serve Integration', () => {
4849
test('generates a post transaction', async () => {
4950
client.on('finishTransaction', transaction => {
5051
expect(transaction.status).toBe('ok');
52+
// eslint-disable-next-line deprecation/deprecation
5153
expect(transaction.tags).toEqual({
5254
'http.status_code': '200',
5355
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ export class IdleTransaction extends Transaction {
146146
this.activities = {};
147147

148148
if (this.op === 'ui.action.click') {
149-
this.setTag(FINISH_REASON_TAG, this._finishReason);
149+
this.setAttribute(FINISH_REASON_TAG, this._finishReason);
150150
}
151151

152152
if (this.spanRecorder) {

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

Lines changed: 49 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -86,21 +86,18 @@ export class Span implements SpanInterface {
8686
public op?: string;
8787

8888
/**
89-
* @inheritDoc
89+
* Tags for the span.
90+
* @deprecated Use `getSpanAttributes(span)` instead.
9091
*/
9192
public tags: { [key: string]: Primitive };
9293

9394
/**
94-
* @inheritDoc
95+
* Data for the span.
96+
* @deprecated Use `getSpanAttributes(span)` instead.
9597
*/
9698
// eslint-disable-next-line @typescript-eslint/no-explicit-any
9799
public data: { [key: string]: any };
98100

99-
/**
100-
* @inheritDoc
101-
*/
102-
public attributes: SpanAttributes;
103-
104101
/**
105102
* List of spans that were finalized
106103
*/
@@ -125,6 +122,7 @@ export class Span implements SpanInterface {
125122
protected _spanId: string;
126123
protected _sampled: boolean | undefined;
127124
protected _name?: string;
125+
protected _attributes: SpanAttributes;
128126

129127
private _logMessage?: string;
130128

@@ -139,9 +137,11 @@ export class Span implements SpanInterface {
139137
this._traceId = spanContext.traceId || uuid4();
140138
this._spanId = spanContext.spanId || uuid4().substring(16);
141139
this.startTimestamp = spanContext.startTimestamp || timestampInSeconds();
140+
// eslint-disable-next-line deprecation/deprecation
142141
this.tags = spanContext.tags ? { ...spanContext.tags } : {};
142+
// eslint-disable-next-line deprecation/deprecation
143143
this.data = spanContext.data ? { ...spanContext.data } : {};
144-
this.attributes = spanContext.attributes ? { ...spanContext.attributes } : {};
144+
this._attributes = spanContext.attributes ? { ...spanContext.attributes } : {};
145145
this.instrumenter = spanContext.instrumenter || 'sentry';
146146
this.origin = spanContext.origin || 'manual';
147147
// eslint-disable-next-line deprecation/deprecation
@@ -165,7 +165,7 @@ export class Span implements SpanInterface {
165165
}
166166
}
167167

168-
// This rule conflicts with another rule :(
168+
// This rule conflicts with another eslint rule :(
169169
/* eslint-disable @typescript-eslint/member-ordering */
170170

171171
/**
@@ -175,6 +175,7 @@ export class Span implements SpanInterface {
175175
public get name(): string {
176176
return this._name || '';
177177
}
178+
178179
/**
179180
* Update the name of the span.
180181
* @deprecated Use `spanToJSON(span).description` instead.
@@ -247,6 +248,22 @@ export class Span implements SpanInterface {
247248
this._sampled = sampled;
248249
}
249250

251+
/**
252+
* Attributes for the span.
253+
* @deprecated Use `getSpanAttributes(span)` instead.
254+
*/
255+
public get attributes(): SpanAttributes {
256+
return this._attributes;
257+
}
258+
259+
/**
260+
* Attributes for the span.
261+
* @deprecated Use `setAttributes()` instead.
262+
*/
263+
public set attributes(attributes: SpanAttributes) {
264+
this._attributes = attributes;
265+
}
266+
250267
/* eslint-enable @typescript-eslint/member-ordering */
251268

252269
/** @inheritdoc */
@@ -296,18 +313,29 @@ export class Span implements SpanInterface {
296313
}
297314

298315
/**
299-
* @inheritDoc
316+
* Sets the tag attribute on the current span.
317+
*
318+
* Can also be used to unset a tag, by passing `undefined`.
319+
*
320+
* @param key Tag key
321+
* @param value Tag value
322+
* @deprecated Use `setAttribute()` instead.
300323
*/
301324
public setTag(key: string, value: Primitive): this {
325+
// eslint-disable-next-line deprecation/deprecation
302326
this.tags = { ...this.tags, [key]: value };
303327
return this;
304328
}
305329

306330
/**
307-
* @inheritDoc
331+
* Sets the data attribute on the current span
332+
* @param key Data key
333+
* @param value Data value
334+
* @deprecated Use `setAttribute()` instead.
308335
*/
309336
// eslint-disable-next-line @typescript-eslint/no-explicit-any
310337
public setData(key: string, value: any): this {
338+
// eslint-disable-next-line deprecation/deprecation
311339
this.data = { ...this.data, [key]: value };
312340
return this;
313341
}
@@ -316,9 +344,9 @@ export class Span implements SpanInterface {
316344
public setAttribute(key: string, value: SpanAttributeValue | undefined): void {
317345
if (value === undefined) {
318346
// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
319-
delete this.attributes[key];
347+
delete this._attributes[key];
320348
} else {
321-
this.attributes[key] = value;
349+
this._attributes[key] = value;
322350
}
323351
}
324352

@@ -339,7 +367,9 @@ export class Span implements SpanInterface {
339367
* @inheritDoc
340368
*/
341369
public setHttpStatus(httpStatus: number): this {
370+
// eslint-disable-next-line deprecation/deprecation
342371
this.setTag('http.status_code', String(httpStatus));
372+
// eslint-disable-next-line deprecation/deprecation
343373
this.setData('http.response.status_code', httpStatus);
344374
const spanStatus = spanStatusfromHttpCode(httpStatus);
345375
if (spanStatus !== 'unknown_error') {
@@ -415,6 +445,7 @@ export class Span implements SpanInterface {
415445
spanId: this._spanId,
416446
startTimestamp: this.startTimestamp,
417447
status: this.status,
448+
// eslint-disable-next-line deprecation/deprecation
418449
tags: this.tags,
419450
traceId: this._traceId,
420451
});
@@ -424,6 +455,7 @@ export class Span implements SpanInterface {
424455
* @inheritDoc
425456
*/
426457
public updateWithContext(spanContext: SpanContext): this {
458+
// eslint-disable-next-line deprecation/deprecation
427459
this.data = spanContext.data || {};
428460
// eslint-disable-next-line deprecation/deprecation
429461
this._name = spanContext.name || spanContext.description;
@@ -434,6 +466,7 @@ export class Span implements SpanInterface {
434466
this._spanId = spanContext.spanId || this._spanId;
435467
this.startTimestamp = spanContext.startTimestamp || this.startTimestamp;
436468
this.status = spanContext.status;
469+
// eslint-disable-next-line deprecation/deprecation
437470
this.tags = spanContext.tags || {};
438471
this._traceId = spanContext.traceId || this._traceId;
439472

@@ -459,6 +492,7 @@ export class Span implements SpanInterface {
459492
span_id: this._spanId,
460493
start_timestamp: this.startTimestamp,
461494
status: this.status,
495+
// eslint-disable-next-line deprecation/deprecation
462496
tags: Object.keys(this.tags).length > 0 ? this.tags : undefined,
463497
timestamp: this.endTimestamp,
464498
trace_id: this._traceId,
@@ -490,7 +524,8 @@ export class Span implements SpanInterface {
490524
[key: string]: any;
491525
}
492526
| undefined {
493-
const { data, attributes } = this;
527+
// eslint-disable-next-line deprecation/deprecation
528+
const { data, _attributes: attributes } = this;
494529

495530
const hasData = Object.keys(data).length > 0;
496531
const hasAttributes = Object.keys(attributes).length > 0;

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,11 @@ export class Transaction extends SpanClass implements TransactionInterface {
110110
...this._metadata,
111111

112112
// From attributes
113-
...(this.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] && {
114-
source: this.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] as TransactionMetadata['source'],
113+
...(this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] && {
114+
source: this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] as TransactionMetadata['source'],
115115
}),
116-
...(this.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] && {
117-
sampleRate: this.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as TransactionMetadata['sampleRate'],
116+
...(this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] && {
117+
sampleRate: this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as TransactionMetadata['sampleRate'],
118118
}),
119119
};
120120
}
@@ -157,7 +157,8 @@ export class Transaction extends SpanClass implements TransactionInterface {
157157
}
158158

159159
/**
160-
* @inheritDoc
160+
* Set the context of a transaction event.
161+
* @deprecated Use either `.setAttribute()`, or set the context on the scope before creating the transaction.
161162
*/
162163
public setContext(key: string, context: Context | null): void {
163164
if (context === null) {
@@ -334,6 +335,7 @@ export class Transaction extends SpanClass implements TransactionInterface {
334335
// TODO: Pass spans serialized via `spanToJSON()` here instead in v8.
335336
spans: finishedSpans,
336337
start_timestamp: this.startTimestamp,
338+
// eslint-disable-next-line deprecation/deprecation
337339
tags: this.tags,
338340
timestamp: this.endTimestamp,
339341
transaction: this._name,

‎packages/core/src/utils/prepareEvent.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { DEFAULT_ENVIRONMENT } from '../constants';
1515
import { getGlobalEventProcessors, notifyEventProcessors } from '../eventProcessors';
1616
import { Scope, getGlobalScope } from '../scope';
1717
import { applyScopeDataToEvent, mergeScopeData } from './applyScopeDataToEvent';
18+
import { spanToJSON } from './spanUtils';
1819

1920
/**
2021
* 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):
326327
// event.spans[].data may contain circular/dangerous data so we need to normalize it
327328
if (event.spans) {
328329
normalized.spans = event.spans.map(span => {
329-
// We cannot use the spread operator here because `toJSON` on `span` is non-enumerable
330-
if (span.data) {
331-
span.data = normalize(span.data, depth, maxBreadth);
330+
const data = spanToJSON(span).data;
331+
332+
if (data) {
333+
// This is a bit weird, as we generally have `Span` instances here, but to be safe we do not assume so
334+
// eslint-disable-next-line deprecation/deprecation
335+
span.data = normalize(data, depth, maxBreadth);
332336
}
337+
333338
return span;
334339
});
335340
}

0 commit comments

Comments
 (0)