Skip to content

Commit e5f3428

Browse files
authored
feat(core): Add span.updateName() and deprecate span.setName() (#10018)
You can call `span.setMetadata()` if you need to update the source. This aligns with the OTEL API for spans.
1 parent 4c82160 commit e5f3428

File tree

25 files changed

+194
-67
lines changed

25 files changed

+194
-67
lines changed

MIGRATION.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ In v8, the Span class is heavily reworked. The following properties & methods ar
1414

1515
* `span.toContext()`: Access the fields directly instead.
1616
* `span.updateWithContext(newSpanContext)`: Update the fields directly instead.
17+
* `span.setName(newName)`: Use `span.updateName(newName)` instead.
1718

1819
## Deprecate `pushScope` & `popScope` in favor of `withScope`
1920

packages/angular/src/tracing.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,8 @@ export class TraceService implements OnDestroy {
118118
const transaction = getActiveTransaction();
119119
// TODO (v8 / #5416): revisit the source condition. Do we want to make the parameterized route the default?
120120
if (transaction && transaction.metadata.source === 'url') {
121-
transaction.setName(route, 'route');
121+
transaction.updateName(route);
122+
transaction.setMetadata({ source: 'route' });
122123
}
123124
}),
124125
);

packages/angular/test/tracing.test.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ let transaction: any;
1010
const defaultStartTransaction = (ctx: any) => {
1111
transaction = {
1212
...ctx,
13-
setName: jest.fn(name => (transaction.name = name)),
13+
updateName: jest.fn(name => (transaction.name = name)),
14+
setMetadata: jest.fn(),
1415
};
1516

1617
return transaction;
@@ -110,7 +111,7 @@ describe('Angular Tracing', () => {
110111
...ctx.metadata,
111112
source: 'custom',
112113
},
113-
setName: jest.fn(name => (transaction.name = name)),
114+
updateName: jest.fn(name => (transaction.name = name)),
114115
};
115116

116117
return transaction;
@@ -137,7 +138,7 @@ describe('Angular Tracing', () => {
137138
metadata: { source: 'url' },
138139
});
139140

140-
expect(transaction.setName).toHaveBeenCalledTimes(0);
141+
expect(transaction.updateName).toHaveBeenCalledTimes(0);
141142
expect(transaction.name).toEqual(url);
142143
expect(transaction.metadata.source).toBe('custom');
143144

@@ -327,7 +328,8 @@ describe('Angular Tracing', () => {
327328
origin: 'auto.navigation.angular',
328329
metadata: { source: 'url' },
329330
});
330-
expect(transaction.setName).toHaveBeenCalledWith(result, 'route');
331+
expect(transaction.updateName).toHaveBeenCalledWith(result);
332+
expect(transaction.setMetadata).toHaveBeenCalledWith({ source: 'route' });
331333

332334
env.destroy();
333335
});

packages/core/src/tracing/span.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,11 @@ export class Span implements SpanInterface {
176176
public get name(): string {
177177
return this.description || '';
178178
}
179-
/** Update the name of the span. */
179+
/**
180+
* Update the name of the span.
181+
*/
180182
public set name(name: string) {
181-
this.setName(name);
183+
this.updateName(name);
182184
}
183185

184186
/**
@@ -267,11 +269,17 @@ export class Span implements SpanInterface {
267269
return this;
268270
}
269271

272+
/** @inheritdoc */
273+
public setName(name: string): void {
274+
this.updateName(name);
275+
}
276+
270277
/**
271278
* @inheritDoc
272279
*/
273-
public setName(name: string): void {
280+
public updateName(name: string): this {
274281
this.description = name;
282+
return this;
275283
}
276284

277285
/**

packages/core/src/tracing/transaction.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,19 +82,30 @@ export class Transaction extends SpanClass implements TransactionInterface {
8282
return this._name;
8383
}
8484

85-
/** Setter for `name` property, which also sets `source` as custom */
85+
/**
86+
* Setter for `name` property, which also sets `source` as custom.
87+
*/
8688
public set name(newName: string) {
89+
// eslint-disable-next-line deprecation/deprecation
8790
this.setName(newName);
8891
}
8992

9093
/**
91-
* JSDoc
94+
* Setter for `name` property, which also sets `source` on the metadata.
95+
*
96+
* @deprecated Use `updateName()` and `setMetadata()` instead.
9297
*/
9398
public setName(name: string, source: TransactionMetadata['source'] = 'custom'): void {
9499
this._name = name;
95100
this.metadata.source = source;
96101
}
97102

103+
/** @inheritdoc */
104+
public updateName(name: string): this {
105+
this._name = name;
106+
return this;
107+
}
108+
98109
/**
99110
* Attaches SpanRecorder to the span itself
100111
* @param maxlen maximum number of spans that can be recorded

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

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ describe('span', () => {
1919
expect(span.description).toEqual(undefined);
2020
});
2121

22-
it('allows to update the name', () => {
22+
it('allows to update the name via setter', () => {
2323
const span = new Span({ name: 'span name' });
2424
expect(span.name).toEqual('span name');
2525
expect(span.description).toEqual('span name');
@@ -30,6 +30,29 @@ describe('span', () => {
3030
expect(span.description).toEqual('new name');
3131
});
3232

33+
it('allows to update the name via setName', () => {
34+
const span = new Span({ name: 'span name' });
35+
expect(span.name).toEqual('span name');
36+
expect(span.description).toEqual('span name');
37+
38+
// eslint-disable-next-line deprecation/deprecation
39+
span.setName('new name');
40+
41+
expect(span.name).toEqual('new name');
42+
expect(span.description).toEqual('new name');
43+
});
44+
45+
it('allows to update the name via updateName', () => {
46+
const span = new Span({ name: 'span name' });
47+
expect(span.name).toEqual('span name');
48+
expect(span.description).toEqual('span name');
49+
50+
span.updateName('new name');
51+
52+
expect(span.name).toEqual('new name');
53+
expect(span.description).toEqual('new name');
54+
});
55+
3356
describe('setAttribute', () => {
3457
it('allows to set attributes', () => {
3558
const span = new Span();

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

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,39 @@ describe('transaction', () => {
66
expect(transaction.name).toEqual('span name');
77
});
88

9-
it('allows to update the name', () => {
9+
it('allows to update the name via setter', () => {
1010
const transaction = new Transaction({ name: 'span name' });
11+
transaction.setMetadata({ source: 'route' });
1112
expect(transaction.name).toEqual('span name');
1213

1314
transaction.name = 'new name';
1415

1516
expect(transaction.name).toEqual('new name');
17+
expect(transaction.metadata.source).toEqual('custom');
18+
});
19+
20+
it('allows to update the name via setName', () => {
21+
const transaction = new Transaction({ name: 'span name' });
22+
transaction.setMetadata({ source: 'route' });
23+
expect(transaction.name).toEqual('span name');
24+
25+
transaction.setMetadata({ source: 'route' });
26+
27+
// eslint-disable-next-line deprecation/deprecation
28+
transaction.setName('new name');
29+
30+
expect(transaction.name).toEqual('new name');
31+
expect(transaction.metadata.source).toEqual('custom');
32+
});
33+
34+
it('allows to update the name via updateName', () => {
35+
const transaction = new Transaction({ name: 'span name' });
36+
transaction.setMetadata({ source: 'route' });
37+
expect(transaction.name).toEqual('span name');
38+
39+
transaction.updateName('new name');
40+
41+
expect(transaction.name).toEqual('new name');
42+
expect(transaction.metadata.source).toEqual('route');
1643
});
1744
});

packages/node/src/handlers.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,8 @@ export function trpcMiddleware(options: SentryTrpcMiddlewareOptions = {}) {
329329
const sentryTransaction = getCurrentScope().getTransaction();
330330

331331
if (sentryTransaction) {
332-
sentryTransaction.setName(`trpc/${path}`, 'route');
332+
sentryTransaction.updateName(`trpc/${path}`);
333+
sentryTransaction.setMetadata({ source: 'route' });
333334
sentryTransaction.op = 'rpc.server';
334335

335336
const trpcContext: Record<string, unknown> = {

packages/opentelemetry-node/src/spanprocessor.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,8 @@ function updateTransactionWithOtelData(transaction: Transaction, otelSpan: OtelS
216216
transaction.setStatus(mapOtelStatus(otelSpan));
217217

218218
transaction.op = op;
219-
transaction.setName(description, source);
219+
transaction.updateName(description);
220+
transaction.setMetadata({ source });
220221
}
221222

222223
function convertOtelTimeToSeconds([seconds, nano]: [number, number]): number {

packages/react/src/reactrouter.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,8 @@ export function withSentryRouting<P extends Record<string, any>, R extends React
165165

166166
const WrappedRoute: React.FC<P> = (props: P) => {
167167
if (activeTransaction && props && props.computedMatch && props.computedMatch.isExact) {
168-
activeTransaction.setName(props.computedMatch.path, 'route');
168+
activeTransaction.updateName(props.computedMatch.path);
169+
activeTransaction.setMetadata({ source: 'route' });
169170
}
170171

171172
// @ts-expect-error Setting more specific React Component typing for `R` generic above

packages/react/src/reactrouterv6.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,9 @@ function updatePageloadTransaction(
134134
: (_matchRoutes(routes, location, basename) as unknown as RouteMatch[]);
135135

136136
if (activeTransaction && branches) {
137-
activeTransaction.setName(...getNormalizedName(routes, location, branches, basename));
137+
const [name, source] = getNormalizedName(routes, location, branches, basename);
138+
activeTransaction.updateName(name);
139+
activeTransaction.setMetadata({ source });
138140
}
139141
}
140142

packages/react/test/reactrouterv4.test.tsx

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ describe('React Router v4', () => {
1212
startTransactionOnPageLoad?: boolean;
1313
startTransactionOnLocationChange?: boolean;
1414
routes?: RouteConfig[];
15-
}): [jest.Mock, any, { mockSetName: jest.Mock; mockFinish: jest.Mock }] {
15+
}): [jest.Mock, any, { mockUpdateName: jest.Mock; mockFinish: jest.Mock; mockSetMetadata: jest.Mock }] {
1616
const options = {
1717
matchPath: _opts && _opts.routes !== undefined ? matchPath : undefined,
1818
routes: undefined,
@@ -22,14 +22,17 @@ describe('React Router v4', () => {
2222
};
2323
const history = createMemoryHistory();
2424
const mockFinish = jest.fn();
25-
const mockSetName = jest.fn();
26-
const mockStartTransaction = jest.fn().mockReturnValue({ setName: mockSetName, end: mockFinish });
25+
const mockUpdateName = jest.fn();
26+
const mockSetMetadata = jest.fn();
27+
const mockStartTransaction = jest
28+
.fn()
29+
.mockReturnValue({ updateName: mockUpdateName, end: mockFinish, setMetadata: mockSetMetadata });
2730
reactRouterV4Instrumentation(history, options.routes, options.matchPath)(
2831
mockStartTransaction,
2932
options.startTransactionOnPageLoad,
3033
options.startTransactionOnLocationChange,
3134
);
32-
return [mockStartTransaction, history, { mockSetName, mockFinish }];
35+
return [mockStartTransaction, history, { mockUpdateName, mockFinish, mockSetMetadata }];
3336
}
3437

3538
it('starts a pageload transaction when instrumentation is started', () => {
@@ -166,7 +169,7 @@ describe('React Router v4', () => {
166169
});
167170

168171
it('normalizes transaction name with custom Route', () => {
169-
const [mockStartTransaction, history, { mockSetName }] = createInstrumentation();
172+
const [mockStartTransaction, history, { mockUpdateName, mockSetMetadata }] = createInstrumentation();
170173
const SentryRoute = withSentryRouting(Route);
171174
const { getByText } = render(
172175
<Router history={history}>
@@ -191,12 +194,13 @@ describe('React Router v4', () => {
191194
tags: { 'routing.instrumentation': 'react-router-v4' },
192195
metadata: { source: 'url' },
193196
});
194-
expect(mockSetName).toHaveBeenCalledTimes(2);
195-
expect(mockSetName).toHaveBeenLastCalledWith('/users/:userid', 'route');
197+
expect(mockUpdateName).toHaveBeenCalledTimes(2);
198+
expect(mockUpdateName).toHaveBeenLastCalledWith('/users/:userid');
199+
expect(mockSetMetadata).toHaveBeenCalledWith({ source: 'route' });
196200
});
197201

198202
it('normalizes nested transaction names with custom Route', () => {
199-
const [mockStartTransaction, history, { mockSetName }] = createInstrumentation();
203+
const [mockStartTransaction, history, { mockUpdateName, mockSetMetadata }] = createInstrumentation();
200204
const SentryRoute = withSentryRouting(Route);
201205
const { getByText } = render(
202206
<Router history={history}>
@@ -221,8 +225,9 @@ describe('React Router v4', () => {
221225
tags: { 'routing.instrumentation': 'react-router-v4' },
222226
metadata: { source: 'url' },
223227
});
224-
expect(mockSetName).toHaveBeenCalledTimes(2);
225-
expect(mockSetName).toHaveBeenLastCalledWith('/organizations/:orgid/v1/:teamid', 'route');
228+
expect(mockUpdateName).toHaveBeenCalledTimes(2);
229+
expect(mockUpdateName).toHaveBeenLastCalledWith('/organizations/:orgid/v1/:teamid');
230+
expect(mockSetMetadata).toHaveBeenLastCalledWith({ source: 'route' });
226231

227232
act(() => {
228233
history.push('/organizations/543');
@@ -237,8 +242,9 @@ describe('React Router v4', () => {
237242
tags: { 'routing.instrumentation': 'react-router-v4' },
238243
metadata: { source: 'url' },
239244
});
240-
expect(mockSetName).toHaveBeenCalledTimes(3);
241-
expect(mockSetName).toHaveBeenLastCalledWith('/organizations/:orgid', 'route');
245+
expect(mockUpdateName).toHaveBeenCalledTimes(3);
246+
expect(mockUpdateName).toHaveBeenLastCalledWith('/organizations/:orgid');
247+
expect(mockSetMetadata).toHaveBeenLastCalledWith({ source: 'route' });
242248
});
243249

244250
it('matches with route object', () => {

0 commit comments

Comments
 (0)