Skip to content

Commit b9cab8f

Browse files
mydeamydea
committed
feat(core): Deprecate transaction metadata in favor of attributes
better semantic attributes & fix tests
1 parent a1b1929 commit b9cab8f

File tree

51 files changed

+403
-189
lines changed

Some content is hidden

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

51 files changed

+403
-189
lines changed

‎MIGRATION.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,8 @@ In v8, the Span class is heavily reworked. The following properties & methods ar
8585
* `span.traceId`: Use `span.spanContext().traceId` instead.
8686
* `span.name`: Use `spanToJSON(span).description` instead.
8787
* `span.description`: Use `spanToJSON(span).description` instead.
88+
* `transaction.setMetadata()`: Use attributes instead, or set data on the scope.
89+
* `transaction.metadata`: Use attributes instead, or set data on the scope.
8890

8991
## Deprecate `pushScope` & `popScope` in favor of `withScope`
9092

‎dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/server.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import http from 'http';
2+
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
23
import * as Sentry from '@sentry/node';
34
import * as Tracing from '@sentry/tracing';
45
import cors from 'cors';
@@ -33,7 +34,7 @@ app.get('/test/express', (_req, res) => {
3334
if (transaction) {
3435
// eslint-disable-next-line deprecation/deprecation
3536
transaction.traceId = '86f39e84263a4de99c326acab3bfe3bd';
36-
transaction.setMetadata({ source: 'route' });
37+
transaction.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route');
3738
}
3839
const headers = http.get('http://somewhere.not.sentry/').getHeaders();
3940

‎packages/angular-ivy/ng-package.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,10 @@
55
"entryFile": "src/index.ts",
66
"umdModuleIds": {
77
"@sentry/browser": "Sentry",
8-
"@sentry/utils": "Sentry.util"
8+
"@sentry/utils": "Sentry.util",
9+
"@sentry/core": "Sentry.core"
910
}
1011
},
11-
"allowedNonPeerDependencies": ["@sentry/browser", "@sentry/utils", "@sentry/types", "tslib"],
12+
"allowedNonPeerDependencies": ["@sentry/browser", "@sentry/core", "@sentry/utils", "@sentry/types", "tslib"],
1213
"assets": ["README.md", "LICENSE"]
1314
}

‎packages/angular-ivy/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
},
2323
"dependencies": {
2424
"@sentry/browser": "7.92.0",
25+
"@sentry/core": "7.92.0",
2526
"@sentry/types": "7.92.0",
2627
"@sentry/utils": "7.92.0",
2728
"tslib": "^2.4.1"

‎packages/angular/ng-package.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,10 @@
55
"entryFile": "src/index.ts",
66
"umdModuleIds": {
77
"@sentry/browser": "Sentry",
8-
"@sentry/utils": "Sentry.util"
8+
"@sentry/utils": "Sentry.util",
9+
"@sentry/core": "Sentry.core"
910
}
1011
},
11-
"whitelistedNonPeerDependencies": ["@sentry/browser", "@sentry/utils", "@sentry/types", "tslib"],
12+
"whitelistedNonPeerDependencies": ["@sentry/browser", "@sentry/core", "@sentry/utils", "@sentry/types", "tslib"],
1213
"assets": ["README.md", "LICENSE"]
1314
}

‎packages/angular/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
},
2323
"dependencies": {
2424
"@sentry/browser": "7.92.0",
25+
"@sentry/core": "7.92.0",
2526
"@sentry/types": "7.92.0",
2627
"@sentry/utils": "7.92.0",
2728
"tslib": "^2.4.1"

‎packages/angular/src/tracing.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type { ActivatedRouteSnapshot, Event, RouterState } from '@angular/router
88
import { NavigationCancel, NavigationError, Router } from '@angular/router';
99
import { NavigationEnd, NavigationStart, ResolveEnd } from '@angular/router';
1010
import { WINDOW, getCurrentScope } from '@sentry/browser';
11+
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, spanToJSON } from '@sentry/core';
1112
import type { Span, Transaction, TransactionContext } from '@sentry/types';
1213
import { logger, stripUrlQueryAndFragment, timestampInSeconds } from '@sentry/utils';
1314
import type { Observable } from 'rxjs';
@@ -39,7 +40,9 @@ export function routingInstrumentation(
3940
name: WINDOW.location.pathname,
4041
op: 'pageload',
4142
origin: 'auto.pageload.angular',
42-
metadata: { source: 'url' },
43+
data: {
44+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
45+
},
4346
});
4447
}
4548
}
@@ -76,7 +79,9 @@ export class TraceService implements OnDestroy {
7679
name: strippedUrl,
7780
op: 'navigation',
7881
origin: 'auto.navigation.angular',
79-
metadata: { source: 'url' },
82+
data: {
83+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
84+
},
8085
});
8186
}
8287

@@ -118,9 +123,10 @@ export class TraceService implements OnDestroy {
118123

119124
const transaction = getActiveTransaction();
120125
// TODO (v8 / #5416): revisit the source condition. Do we want to make the parameterized route the default?
121-
if (transaction && transaction.metadata.source === 'url') {
126+
const attributes = (transaction && spanToJSON(transaction).data) || {};
127+
if (transaction && attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] === 'url') {
122128
transaction.updateName(route);
123-
transaction.setMetadata({ source: 'route' });
129+
transaction.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route');
124130
}
125131
}),
126132
);

‎packages/angular/test/tracing.test.ts

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Component } from '@angular/core';
22
import type { ActivatedRouteSnapshot, CanActivate, RouterStateSnapshot } from '@angular/router';
3+
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
34

45
import { TraceClassDecorator, TraceDirective, TraceMethodDecorator, instrumentAngularRouting } from '../src';
56
import { getParameterizedRouteFromSnapshot } from '../src/tracing';
@@ -11,7 +12,13 @@ const defaultStartTransaction = (ctx: any) => {
1112
transaction = {
1213
...ctx,
1314
updateName: jest.fn(name => (transaction.name = name)),
14-
setMetadata: jest.fn(),
15+
setAttribute: jest.fn(),
16+
toJSON: () => ({
17+
data: {
18+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom',
19+
...ctx.data,
20+
},
21+
}),
1522
};
1623

1724
return transaction;
@@ -45,7 +52,7 @@ describe('Angular Tracing', () => {
4552
name: '/',
4653
op: 'pageload',
4754
origin: 'auto.pageload.angular',
48-
metadata: { source: 'url' },
55+
data: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url' },
4956
});
5057
});
5158
});
@@ -107,11 +114,15 @@ describe('Angular Tracing', () => {
107114
const customStartTransaction = jest.fn((ctx: any) => {
108115
transaction = {
109116
...ctx,
110-
metadata: {
111-
...ctx.metadata,
112-
source: 'custom',
113-
},
117+
toJSON: () => ({
118+
data: {
119+
...ctx.data,
120+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom',
121+
},
122+
}),
123+
metadata: ctx.metadata,
114124
updateName: jest.fn(name => (transaction.name = name)),
125+
setAttribute: jest.fn(),
115126
};
116127

117128
return transaction;
@@ -135,12 +146,12 @@ describe('Angular Tracing', () => {
135146
name: url,
136147
op: 'pageload',
137148
origin: 'auto.pageload.angular',
138-
metadata: { source: 'url' },
149+
data: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url' },
139150
});
140151

141152
expect(transaction.updateName).toHaveBeenCalledTimes(0);
142153
expect(transaction.name).toEqual(url);
143-
expect(transaction.metadata.source).toBe('custom');
154+
expect(transaction.toJSON().data).toEqual({ [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'custom' });
144155

145156
env.destroy();
146157
});
@@ -326,10 +337,10 @@ describe('Angular Tracing', () => {
326337
name: url,
327338
op: 'navigation',
328339
origin: 'auto.navigation.angular',
329-
metadata: { source: 'url' },
340+
data: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url' },
330341
});
331342
expect(transaction.updateName).toHaveBeenCalledWith(result);
332-
expect(transaction.setMetadata).toHaveBeenCalledWith({ source: 'route' });
343+
expect(transaction.setAttribute).toHaveBeenCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route');
333344

334345
env.destroy();
335346
});

‎packages/astro/src/server/middleware.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
12
import {
23
captureException,
34
continueTrace,
@@ -111,6 +112,7 @@ async function instrumentRequest(
111112

112113
try {
113114
const interpolatedRoute = interpolateRouteFromUrlAndParams(ctx.url.pathname, ctx.params);
115+
const source = interpolatedRoute ? 'route' : 'url';
114116
// storing res in a variable instead of directly returning is necessary to
115117
// invoke the catch block if next() throws
116118
const res = await startSpan(
@@ -121,12 +123,13 @@ async function instrumentRequest(
121123
origin: 'auto.http.astro',
122124
status: 'ok',
123125
metadata: {
126+
// eslint-disable-next-line deprecation/deprecation
124127
...traceCtx?.metadata,
125-
source: interpolatedRoute ? 'route' : 'url',
126128
},
127129
data: {
128130
method,
129131
url: stripUrlQueryAndFragment(ctx.url.href),
132+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
130133
...(ctx.url.search && { 'http.query': ctx.url.search }),
131134
...(ctx.url.hash && { 'http.fragment': ctx.url.hash }),
132135
...(options.trackHeaders && { headers: allHeaders }),

‎packages/astro/test/server/middleware.test.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
12
import * as SentryNode from '@sentry/node';
23
import type { Client } from '@sentry/types';
34
import { vi } from 'vitest';
@@ -57,10 +58,9 @@ describe('sentryMiddleware', () => {
5758
data: {
5859
method: 'GET',
5960
url: 'https://mydomain.io/users/123/details',
61+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
6062
},
61-
metadata: {
62-
source: 'route',
63-
},
63+
metadata: {},
6464
name: 'GET /users/[id]/details',
6565
op: 'http.server',
6666
origin: 'auto.http.astro',
@@ -94,10 +94,9 @@ describe('sentryMiddleware', () => {
9494
data: {
9595
method: 'GET',
9696
url: 'http://localhost:1234/a%xx',
97+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
9798
},
98-
metadata: {
99-
source: 'url',
100-
},
99+
metadata: {},
101100
name: 'GET a%xx',
102101
op: 'http.server',
103102
origin: 'auto.http.astro',
@@ -159,8 +158,10 @@ describe('sentryMiddleware', () => {
159158

160159
expect(startSpanSpy).toHaveBeenCalledWith(
161160
expect.objectContaining({
161+
data: expect.objectContaining({
162+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
163+
}),
162164
metadata: {
163-
source: 'route',
164165
dynamicSamplingContext: {
165166
release: '1.0.0',
166167
},

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {
2+
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
23
Transaction,
34
captureException,
45
continueTrace,
@@ -54,6 +55,7 @@ function instrumentBunServeOptions(serveOptions: Parameters<typeof Bun.serve>[0]
5455
const parsedUrl = parseUrl(request.url);
5556
const data: Record<string, unknown> = {
5657
'http.request.method': request.method || 'GET',
58+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
5759
};
5860
if (parsedUrl.search) {
5961
data['http.query'] = parsedUrl.search;
@@ -72,8 +74,8 @@ function instrumentBunServeOptions(serveOptions: Parameters<typeof Bun.serve>[0]
7274
...ctx,
7375
data,
7476
metadata: {
77+
// eslint-disable-next-line deprecation/deprecation
7578
...ctx.metadata,
76-
source: 'url',
7779
request: {
7880
url,
7981
method: request.method,

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ describe('Bun Serve Integration', () => {
8282
expect(transaction.parentSpanId).toBe(PARENT_SPAN_ID);
8383
expect(transaction.isRecording()).toBe(true);
8484

85+
// eslint-disable-next-line deprecation/deprecation
8586
expect(transaction.metadata?.dynamicSamplingContext).toStrictEqual({ version: '1.0', environment: 'production' });
8687
});
8788

‎packages/core/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ export type { ServerRuntimeClientOptions } from './server-runtime-client';
55
export type { RequestDataIntegrationOptions } from './integrations/requestdata';
66

77
export * from './tracing';
8+
export * from './semanticAttributes';
89
export { createEventEnvelope, createSessionEnvelope } from './envelope';
910
export {
1011
addBreadcrumb,

‎packages/core/src/semanticAttributes.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
/**
2+
* Use this attribute to represent the source of a span.
3+
* Should be one of: custom, url, route, view, component, task, unknown
4+
*
5+
*/
6+
export const SEMANTIC_ATTRIBUTE_SENTRY_SOURCE = 'sentry.source';
7+
8+
/**
9+
* Use this attribute to represent the sample rate used for a span.
10+
*/
11+
export const SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE = 'sentry.sample_rate';

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

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { Options, SamplingContext } from '@sentry/types';
22
import { isNaN, logger } from '@sentry/utils';
33

44
import { DEBUG_BUILD } from '../debug-build';
5+
import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE } from '../semanticAttributes';
56
import { hasTracingEnabled } from '../utils/hasTracingEnabled';
67
import { spanToJSON } from '../utils/spanUtils';
78
import type { Transaction } from './transaction';
@@ -30,10 +31,8 @@ export function sampleTransaction<T extends Transaction>(
3031
// if the user has forced a sampling decision by passing a `sampled` value in their transaction context, go with that
3132
// eslint-disable-next-line deprecation/deprecation
3233
if (transaction.sampled !== undefined) {
33-
transaction.setMetadata({
34-
// eslint-disable-next-line deprecation/deprecation
35-
sampleRate: Number(transaction.sampled),
36-
});
34+
// eslint-disable-next-line deprecation/deprecation
35+
transaction.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, Number(transaction.sampled));
3736
return transaction;
3837
}
3938

@@ -42,22 +41,16 @@ export function sampleTransaction<T extends Transaction>(
4241
let sampleRate;
4342
if (typeof options.tracesSampler === 'function') {
4443
sampleRate = options.tracesSampler(samplingContext);
45-
transaction.setMetadata({
46-
sampleRate: Number(sampleRate),
47-
});
44+
transaction.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, Number(sampleRate));
4845
} else if (samplingContext.parentSampled !== undefined) {
4946
sampleRate = samplingContext.parentSampled;
5047
} else if (typeof options.tracesSampleRate !== 'undefined') {
5148
sampleRate = options.tracesSampleRate;
52-
transaction.setMetadata({
53-
sampleRate: Number(sampleRate),
54-
});
49+
transaction.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, Number(sampleRate));
5550
} else {
5651
// When `enableTracing === true`, we use a sample rate of 100%
5752
sampleRate = 1;
58-
transaction.setMetadata({
59-
sampleRate,
60-
});
53+
transaction.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, sampleRate);
6154
}
6255

6356
// Since this is coming from the user (or from a function provided by the user), who knows what we might get. (The

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,8 @@ export class Span implements SpanInterface {
126126
protected _sampled: boolean | undefined;
127127
protected _name?: string;
128128

129+
private _logMessage?: string;
130+
129131
/**
130132
* You should never call the constructor manually, always use `Sentry.startTransaction()`
131133
* or call `startChild()` on an existing span.
@@ -286,8 +288,8 @@ export class Span implements SpanInterface {
286288
const idStr = childSpan.transaction.spanContext().spanId;
287289

288290
const logMessage = `[Tracing] Starting '${opStr}' span on transaction '${nameStr}' (${idStr}).`;
289-
childSpan.transaction.metadata.spanMetadata[childSpan.spanContext().spanId] = { logMessage };
290291
logger.log(logMessage);
292+
this._logMessage = logMessage;
291293
}
292294

293295
return childSpan;
@@ -383,7 +385,7 @@ export class Span implements SpanInterface {
383385
this.transaction &&
384386
this.transaction.spanContext().spanId !== this._spanId
385387
) {
386-
const { logMessage } = this.transaction.metadata.spanMetadata[this._spanId];
388+
const logMessage = this._logMessage;
387389
if (logMessage) {
388390
logger.log((logMessage as string).replace('Starting', 'Finishing'));
389391
}

0 commit comments

Comments
 (0)