Skip to content

Commit deab246

Browse files
committed
ref: Refactor most usage of tags on spans
Only missing are browser metrics, where we need to verify this can be refactored already.
1 parent d570594 commit deab246

File tree

42 files changed

+42
-296
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+42
-296
lines changed

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ test('Should trace outgoing fetch requests inside middleware and create breadcru
7878
span_id: expect.any(String),
7979
start_timestamp: expect.any(Number),
8080
status: 'ok',
81-
tags: { 'http.status_code': '200' },
8281
timestamp: expect.any(Number),
8382
trace_id: expect.any(String),
8483
},

dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/propagation.test.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,6 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => {
7070
op: 'http.server',
7171
span_id: expect.any(String),
7272
status: 'ok',
73-
tags: {
74-
'http.status_code': '200',
75-
},
7673
trace_id: traceId,
7774
origin: 'auto.http.otel.http',
7875
},
@@ -96,9 +93,6 @@ test('Propagates trace for outgoing http requests', async ({ baseURL }) => {
9693
parent_span_id: outgoingHttpSpanId,
9794
span_id: expect.any(String),
9895
status: 'ok',
99-
tags: {
100-
'http.status_code': '200',
101-
},
10296
trace_id: traceId,
10397
origin: 'auto.http.otel.http',
10498
},
@@ -169,9 +163,6 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => {
169163
op: 'http.server',
170164
span_id: expect.any(String),
171165
status: 'ok',
172-
tags: {
173-
'http.status_code': '200',
174-
},
175166
trace_id: traceId,
176167
origin: 'auto.http.otel.http',
177168
},
@@ -195,9 +186,6 @@ test('Propagates trace for outgoing fetch requests', async ({ baseURL }) => {
195186
parent_span_id: outgoingHttpSpanId,
196187
span_id: expect.any(String),
197188
status: 'ok',
198-
tags: {
199-
'http.status_code': '200',
200-
},
201189
trace_id: traceId,
202190
origin: 'auto.http.otel.http',
203191
},

dev-packages/e2e-tests/test-applications/node-experimental-fastify-app/tests/transactions.test.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,6 @@ test('Sends an API route transaction', async ({ baseURL }) => {
3535
op: 'http.server',
3636
span_id: expect.any(String),
3737
status: 'ok',
38-
tags: {
39-
'http.status_code': '200',
40-
},
4138
trace_id: expect.any(String),
4239
origin: 'auto.http.otel.http',
4340
},
@@ -90,9 +87,6 @@ test('Sends an API route transaction', async ({ baseURL }) => {
9087
origin: 'manual',
9188
},
9289
],
93-
tags: {
94-
'http.status_code': '200',
95-
},
9690
transaction: 'GET /test-transaction',
9791
type: 'transaction',
9892
transaction_info: {

dev-packages/e2e-tests/test-applications/sveltekit/test/performance.test.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,6 @@ test('sends a pageload transaction', async ({ page }) => {
2222
origin: 'auto.pageload.sveltekit',
2323
},
2424
},
25-
tags: {
26-
'routing.instrumentation': '@sentry/sveltekit',
27-
},
2825
});
2926
});
3027

dev-packages/node-integration-tests/suites/express/tracing-experimental/test.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,6 @@ describe('express tracing experimental', () => {
2121
},
2222
op: 'http.server',
2323
status: 'ok',
24-
tags: {
25-
'http.status_code': '200',
26-
},
2724
},
2825
},
2926
spans: expect.arrayContaining([
@@ -61,9 +58,6 @@ describe('express tracing experimental', () => {
6158
},
6259
op: 'http.server',
6360
status: 'ok',
64-
tags: {
65-
'http.status_code': '200',
66-
},
6761
},
6862
},
6963
},
@@ -93,9 +87,6 @@ describe('express tracing experimental', () => {
9387
},
9488
op: 'http.server',
9589
status: 'ok',
96-
tags: {
97-
'http.status_code': '200',
98-
},
9990
},
10091
},
10192
},
@@ -133,9 +124,6 @@ describe('express tracing experimental', () => {
133124
},
134125
op: 'http.server',
135126
status: 'ok',
136-
tags: {
137-
'http.status_code': '200',
138-
},
139127
},
140128
},
141129
},

dev-packages/node-integration-tests/suites/express/tracing/test.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,6 @@ test('should create and send transactions for Express routes and spans for middl
1919
},
2020
op: 'http.server',
2121
status: 'ok',
22-
tags: {
23-
'http.status_code': '200',
24-
},
2522
},
2623
},
2724
spans: [
@@ -55,9 +52,6 @@ test('should set a correct transaction name for routes specified in RegEx', done
5552
},
5653
op: 'http.server',
5754
status: 'ok',
58-
tags: {
59-
'http.status_code': '200',
60-
},
6155
},
6256
},
6357
},
@@ -87,9 +81,6 @@ test.each([['array1'], ['array5']])(
8781
},
8882
op: 'http.server',
8983
status: 'ok',
90-
tags: {
91-
'http.status_code': '200',
92-
},
9384
},
9485
},
9586
},
@@ -127,9 +118,6 @@ test.each([
127118
},
128119
op: 'http.server',
129120
status: 'ok',
130-
tags: {
131-
'http.status_code': '200',
132-
},
133121
},
134122
},
135123
},

dev-packages/node-integration-tests/suites/tracing/spans/test.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,12 @@ test('should capture spans for outgoing http requests', async () => {
2222
op: 'http.client',
2323
origin: 'auto.http.node.http',
2424
status: 'ok',
25-
tags: {
26-
'http.status_code': '200',
27-
},
2825
},
2926
{
3027
description: 'GET http://match-this-url.com/api/v1',
3128
op: 'http.client',
3229
origin: 'auto.http.node.http',
3330
status: 'ok',
34-
tags: {
35-
'http.status_code': '200',
36-
},
3731
},
3832
],
3933
});

packages/angular/src/tracing.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,6 @@ export class TraceService implements OnDestroy {
144144
attributes: {
145145
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.angular',
146146
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
147-
},
148-
tags: {
149147
url: strippedUrl,
150148
...(navigationEvent.navigationTrigger && {
151149
navigationTrigger: navigationEvent.navigationTrigger,
@@ -176,8 +174,7 @@ export class TraceService implements OnDestroy {
176174
name: `${navigationEvent.url}`,
177175
op: ANGULAR_ROUTING_OP,
178176
origin: 'auto.ui.angular',
179-
tags: {
180-
'routing.instrumentation': '@sentry/angular',
177+
attributes: {
181178
url: strippedUrl,
182179
...(navigationEvent.navigationTrigger && {
183180
navigationTrigger: navigationEvent.navigationTrigger,

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,7 @@ describe('Bun Serve Integration', () => {
2525
test('generates a transaction around a request', async () => {
2626
client.on('finishTransaction', transaction => {
2727
expect(transaction.status).toBe('ok');
28-
// eslint-disable-next-line deprecation/deprecation
29-
expect(transaction.tags).toEqual({
30-
'http.status_code': '200',
31-
});
28+
expect(spanToJSON(transaction).data?.['http.response.status_code']).toEqual(200);
3229
expect(spanToJSON(transaction).op).toEqual('http.server');
3330
expect(spanToJSON(transaction).description).toEqual('GET /');
3431
});
@@ -48,10 +45,7 @@ describe('Bun Serve Integration', () => {
4845
test('generates a post transaction', async () => {
4946
client.on('finishTransaction', transaction => {
5047
expect(transaction.status).toBe('ok');
51-
// eslint-disable-next-line deprecation/deprecation
52-
expect(transaction.tags).toEqual({
53-
'http.status_code': '200',
54-
});
48+
expect(spanToJSON(transaction).data?.['http.response.status_code']).toEqual(200);
5549
expect(spanToJSON(transaction).op).toEqual('http.server');
5650
expect(spanToJSON(transaction).description).toEqual('POST /');
5751
});

packages/core/src/tracing/spanstatus.ts

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -128,17 +128,7 @@ export function getSpanStatusFromHttpCode(httpStatus: number): SpanStatusType {
128128
* Additionally, the span's status is updated, depending on the http code.
129129
*/
130130
export function setHttpStatus(span: Span, httpStatus: number): void {
131-
// TODO (v8): Remove these calls
132-
// Relay does not require us to send the status code as a tag
133-
// For now, just because users might expect it to land as a tag we keep sending it.
134-
// Same with data.
135-
// In v8, we replace both, simply with
136-
// span.setAttribute('http.response.status_code', httpStatus);
137-
138-
// eslint-disable-next-line deprecation/deprecation
139-
span.setTag('http.status_code', String(httpStatus));
140-
// eslint-disable-next-line deprecation/deprecation
141-
span.setData('http.response.status_code', httpStatus);
131+
span.setAttribute('http.response.status_code', httpStatus);
142132

143133
const spanStatus = getSpanStatusFromHttpCode(httpStatus);
144134
if (spanStatus !== 'unknown_error') {

packages/core/test/lib/tracing/spanstatus.test.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,22 +20,20 @@ describe('setHttpStatus', () => {
2020

2121
setHttpStatus(span!, code);
2222

23-
const { status: spanStatus, data, tags } = spanToJSON(span!);
23+
const { status: spanStatus, data } = spanToJSON(span!);
2424

2525
expect(spanStatus).toBe(status);
2626
expect(data).toMatchObject({ 'http.response.status_code': code });
27-
expect(tags).toMatchObject({ 'http.status_code': String(code) });
2827
});
2928

3029
it("doesn't set the status for an unknown http status code", () => {
3130
const span = new SentrySpan({ name: 'test' });
3231

3332
setHttpStatus(span!, 600);
3433

35-
const { status: spanStatus, data, tags } = spanToJSON(span!);
34+
const { status: spanStatus, data } = spanToJSON(span!);
3635

3736
expect(spanStatus).toBeUndefined();
3837
expect(data).toMatchObject({ 'http.response.status_code': 600 });
39-
expect(tags).toMatchObject({ 'http.status_code': '600' });
4038
});
4139
});

packages/core/test/lib/utils/spanUtils.test.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,6 @@ describe('spanToJSON', () => {
6868
parentSpanId: '1234',
6969
spanId: '5678',
7070
status: 'ok',
71-
tags: {
72-
foo: 'bar',
73-
},
7471
traceId: 'abcd',
7572
origin: 'auto',
7673
startTimestamp: 123,
@@ -83,9 +80,6 @@ describe('spanToJSON', () => {
8380
parent_span_id: '1234',
8481
span_id: '5678',
8582
status: 'ok',
86-
tags: {
87-
foo: 'bar',
88-
},
8983
trace_id: 'abcd',
9084
origin: 'auto',
9185
start_timestamp: 123,

packages/ember/addon/instance-initializers/sentry-performance.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -116,11 +116,8 @@ export function _instrumentEmberRouter(
116116
origin: 'auto.pageload.ember',
117117
attributes: {
118118
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
119-
},
120-
tags: {
121119
url,
122120
toRoute: routeInfo.name,
123-
'routing.instrumentation': '@sentry/ember',
124121
},
125122
});
126123
}
@@ -146,11 +143,8 @@ export function _instrumentEmberRouter(
146143
origin: 'auto.navigation.ember',
147144
attributes: {
148145
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
149-
},
150-
tags: {
151146
fromRoute,
152147
toRoute,
153-
'routing.instrumentation': '@sentry/ember',
154148
},
155149
});
156150

packages/ember/tests/acceptance/sentry-performance-test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ module('Acceptance | Sentry Performance', function (hooks) {
2121
'ui.ember.component.render | component:test-section',
2222
],
2323
transaction: 'route:tracing',
24-
tags: {
24+
attributes: {
2525
fromRoute: undefined,
2626
toRoute: 'tracing',
2727
},
@@ -50,7 +50,7 @@ module('Acceptance | Sentry Performance', function (hooks) {
5050
],
5151
transaction: 'route:slow-loading-route.index',
5252
durationCheck: duration => duration > SLOW_TRANSITION_WAIT,
53-
tags: {
53+
attributes: {
5454
fromRoute: 'tracing',
5555
toRoute: 'slow-loading-route.index',
5656
},

packages/ember/tests/helpers/utils.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export function assertSentryTransactions(
5151
options: {
5252
spans: string[];
5353
transaction: string;
54-
tags: Record<string, string | undefined>;
54+
attributes: Record<string, string | undefined>;
5555
durationCheck?: (duration: number) => boolean;
5656
},
5757
): void {
@@ -83,8 +83,10 @@ export function assertSentryTransactions(
8383
assert.deepEqual(filteredSpans, options.spans, 'Has correct spans');
8484

8585
assert.equal(event.transaction, options.transaction);
86-
assert.equal(event.tags?.fromRoute, options.tags.fromRoute);
87-
assert.equal(event.tags?.toRoute, options.tags.toRoute);
86+
87+
Object.keys(options.attributes).forEach(key => {
88+
assert.equal(event.contexts?.trace?.data?.[key], options.attributes[key]);
89+
});
8890

8991
if (options.durationCheck && event.timestamp && event.start_timestamp) {
9092
const duration = (event.timestamp - event.start_timestamp) * 1000;

packages/nextjs/src/client/routing/appRouterRoutingInstrumentation.ts

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,6 @@ import { addFetchInstrumentationHandler, browserPerformanceTimeOrigin } from '@s
99

1010
type StartSpanCb = (context: StartSpanOptions) => void;
1111

12-
const DEFAULT_TAGS = {
13-
'routing.instrumentation': 'next-app-router',
14-
} as const;
15-
1612
/**
1713
* Instruments the Next.js Client App Router.
1814
*/
@@ -29,7 +25,6 @@ export function appRouterInstrumentation(
2925
if (shouldInstrumentPageload) {
3026
startPageloadSpanCallback({
3127
name: currPathname,
32-
tags: DEFAULT_TAGS,
3328
// pageload should always start at timeOrigin (and needs to be in s, not ms)
3429
startTime: browserPerformanceTimeOrigin ? browserPerformanceTimeOrigin / 1000 : undefined,
3530
attributes: {
@@ -64,11 +59,8 @@ export function appRouterInstrumentation(
6459

6560
startNavigationSpanCallback({
6661
name: newPathname,
67-
tags: {
68-
...DEFAULT_TAGS,
69-
from: currPathname,
70-
},
7162
attributes: {
63+
from: currPathname,
7264
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
7365
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.nextjs.app_router_instrumentation',
7466
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',

0 commit comments

Comments
 (0)