Skip to content

Commit e7552e3

Browse files
authored
feat(core): Deprecate span name and description (#10056)
Instead, users should use `spanToJSON(span).description`. In reality, users should mostly not actually need this - most usages are internal things, or tests, right now. While at this, I also updated the `TraceContext` type to not have a `description`. This started failing in a bunch of tests, because while in theory the trace context has a `description` field, in praxis we always used to set `description` to `undefined` for transactions, leading to this never being set. The work in this PR unifies the name/description handling between span and transaction, which lead to `description` not being undefined for transactions anymore, leading to this actually showing up in the trace context.
1 parent 73a8314 commit e7552e3

File tree

31 files changed

+232
-174
lines changed

31 files changed

+232
-174
lines changed

MIGRATION.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ In v8, the Span class is heavily reworked. The following properties & methods ar
7171
* `span.sampled`: Use `span.isRecording()` instead.
7272
* `span.spanId`: Use `span.spanContext().spanId` instead.
7373
* `span.traceId`: Use `span.spanContext().traceId` instead.
74+
* `span.name`: Use `spanToJSON(span).description` instead.
75+
* `span.description`: Use `spanToJSON(span).description` instead.
7476

7577
## Deprecate `pushScope` & `popScope` in favor of `withScope`
7678

dev-packages/browser-integration-tests/suites/public-api/startSpan/basic/test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ sentryTest('should report finished spans as children of the root transaction', a
2929
expect(transaction.spans).toHaveLength(1);
3030

3131
const span_1 = transaction.spans?.[0];
32+
// eslint-disable-next-line deprecation/deprecation
3233
expect(span_1?.description).toBe('child_span');
3334
expect(span_1?.parentSpanId).toEqual(rootSpanId);
3435
});

dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-fid/test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ sentryTest('should capture a FID vital.', async ({ browserName, getLocalTestPath
2121
expect(eventData.measurements).toBeDefined();
2222
expect(eventData.measurements?.fid?.value).toBeDefined();
2323

24+
// eslint-disable-next-line deprecation/deprecation
2425
const fidSpan = eventData.spans?.filter(({ description }) => description === 'first input delay')[0];
2526

2627
expect(fidSpan).toBeDefined();

dev-packages/browser-integration-tests/suites/tracing/metrics/web-vitals-fp-fcp/test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ sentryTest('should capture FP vital.', async ({ browserName, getLocalTestPath, p
1616
expect(eventData.measurements).toBeDefined();
1717
expect(eventData.measurements?.fp?.value).toBeDefined();
1818

19+
// eslint-disable-next-line deprecation/deprecation
1920
const fpSpan = eventData.spans?.filter(({ description }) => description === 'first-paint')[0];
2021

2122
expect(fpSpan).toBeDefined();
@@ -34,6 +35,7 @@ sentryTest('should capture FCP vital.', async ({ getLocalTestPath, page }) => {
3435
expect(eventData.measurements).toBeDefined();
3536
expect(eventData.measurements?.fcp?.value).toBeDefined();
3637

38+
// eslint-disable-next-line deprecation/deprecation
3739
const fcpSpan = eventData.spans?.filter(({ description }) => description === 'first-contentful-paint')[0];
3840

3941
expect(fcpSpan).toBeDefined();

packages/browser/src/profiling/hubextensions.ts

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
/* eslint-disable complexity */
2+
import { spanToJSON } from '@sentry/core';
23
import type { Transaction } from '@sentry/types';
34
import { logger, timestampInSeconds, uuid4 } from '@sentry/utils';
45

@@ -56,7 +57,7 @@ export function startProfileForTransaction(transaction: Transaction): Transactio
5657
}
5758

5859
if (DEBUG_BUILD) {
59-
logger.log(`[Profiling] started profiling transaction: ${transaction.name || transaction.description}`);
60+
logger.log(`[Profiling] started profiling transaction: ${spanToJSON(transaction).description}`);
6061
}
6162

6263
// We create "unique" transaction names to avoid concurrent transactions with same names
@@ -87,11 +88,7 @@ export function startProfileForTransaction(transaction: Transaction): Transactio
8788
}
8889
if (processedProfile) {
8990
if (DEBUG_BUILD) {
90-
logger.log(
91-
'[Profiling] profile for:',
92-
transaction.name || transaction.description,
93-
'already exists, returning early',
94-
);
91+
logger.log('[Profiling] profile for:', spanToJSON(transaction).description, 'already exists, returning early');
9592
}
9693
return null;
9794
}
@@ -105,14 +102,14 @@ export function startProfileForTransaction(transaction: Transaction): Transactio
105102
}
106103

107104
if (DEBUG_BUILD) {
108-
logger.log(`[Profiling] stopped profiling of transaction: ${transaction.name || transaction.description}`);
105+
logger.log(`[Profiling] stopped profiling of transaction: ${spanToJSON(transaction).description}`);
109106
}
110107

111108
// In case of an overlapping transaction, stopProfiling may return null and silently ignore the overlapping profile.
112109
if (!profile) {
113110
if (DEBUG_BUILD) {
114111
logger.log(
115-
`[Profiling] profiler returned null profile for: ${transaction.name || transaction.description}`,
112+
`[Profiling] profiler returned null profile for: ${spanToJSON(transaction).description}`,
116113
'this may indicate an overlapping transaction or a call to stopProfiling with a profile title that was never started',
117114
);
118115
}
@@ -135,7 +132,7 @@ export function startProfileForTransaction(transaction: Transaction): Transactio
135132
if (DEBUG_BUILD) {
136133
logger.log(
137134
'[Profiling] max profile duration elapsed, stopping profiling for:',
138-
transaction.name || transaction.description,
135+
spanToJSON(transaction).description,
139136
);
140137
}
141138
// If the timeout exceeds, we want to stop profiling, but not finish the transaction

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { beforeAll, beforeEach, describe, expect, test } from 'bun:test';
2-
import { Hub, makeMain } from '@sentry/core';
2+
import { Hub, makeMain, spanToJSON } from '@sentry/core';
33

44
import { BunClient } from '../../src/client';
55
import { instrumentBunServe } from '../../src/integrations/bunserver';
@@ -30,7 +30,7 @@ describe('Bun Serve Integration', () => {
3030
'http.status_code': '200',
3131
});
3232
expect(transaction.op).toEqual('http.server');
33-
expect(transaction.name).toEqual('GET /');
33+
expect(spanToJSON(transaction).description).toEqual('GET /');
3434
});
3535

3636
const server = Bun.serve({
@@ -52,7 +52,7 @@ describe('Bun Serve Integration', () => {
5252
'http.status_code': '200',
5353
});
5454
expect(transaction.op).toEqual('http.server');
55-
expect(transaction.name).toEqual('POST /');
55+
expect(spanToJSON(transaction).description).toEqual('POST /');
5656
});
5757

5858
const server = Bun.serve({

packages/core/src/integrations/requestdata.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { Client, IntegrationFn, Transaction } from '@sentry/types';
22
import type { AddRequestDataToEventOptions, TransactionNamingScheme } from '@sentry/utils';
33
import { addRequestDataToEvent, extractPathForTransaction } from '@sentry/utils';
44
import { convertIntegrationFnToClass } from '../integration';
5+
import { spanToJSON } from '../utils/spanUtils';
56

67
export type RequestDataIntegrationOptions = {
78
/**
@@ -105,18 +106,20 @@ const requestDataIntegration: IntegrationFn = (options: RequestDataIntegrationOp
105106
const reqWithTransaction = req as { _sentryTransaction?: Transaction };
106107
const transaction = reqWithTransaction._sentryTransaction;
107108
if (transaction) {
109+
const name = spanToJSON(transaction).description || '';
110+
108111
// TODO (v8): Remove the nextjs check and just base it on `transactionNamingScheme` for all SDKs. (We have to
109112
// keep it the way it is for the moment, because changing the names of transactions in Sentry has the potential
110113
// to break things like alert rules.)
111114
const shouldIncludeMethodInTransactionName =
112115
getSDKName(client) === 'sentry.javascript.nextjs'
113-
? transaction.name.startsWith('/api')
116+
? name.startsWith('/api')
114117
: transactionNamingScheme !== 'path';
115118

116119
const [transactionValue] = extractPathForTransaction(req, {
117120
path: true,
118121
method: shouldIncludeMethodInTransactionName,
119-
customRoute: transaction.name,
122+
customRoute: name,
120123
});
121124

122125
processedEvent.transaction = transactionValue;

packages/core/src/metrics/exports.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { logger } from '@sentry/utils';
33
import type { BaseClient } from '../baseclient';
44
import { DEBUG_BUILD } from '../debug-build';
55
import { getClient, getCurrentScope } from '../exports';
6+
import { spanToJSON } from '../utils/spanUtils';
67
import { COUNTER_METRIC_TYPE, DISTRIBUTION_METRIC_TYPE, GAUGE_METRIC_TYPE, SET_METRIC_TYPE } from './constants';
78
import { MetricsAggregator } from './integration';
89
import type { MetricType } from './types';
@@ -38,7 +39,7 @@ function addToMetricsAggregator(
3839
metricTags.environment = environment;
3940
}
4041
if (transaction) {
41-
metricTags.transaction = transaction.name;
42+
metricTags.transaction = spanToJSON(transaction).description || '';
4243
}
4344

4445
DEBUG_BUILD && logger.log(`Adding value of ${value} to ${metricType} metric ${name}`);

packages/core/src/tracing/sampling.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { isNaN, logger } from '@sentry/utils';
33

44
import { DEBUG_BUILD } from '../debug-build';
55
import { hasTracingEnabled } from '../utils/hasTracingEnabled';
6+
import { spanToJSON } from '../utils/spanUtils';
67
import type { Transaction } from './transaction';
78

89
/**
@@ -100,7 +101,8 @@ export function sampleTransaction<T extends Transaction>(
100101
return transaction;
101102
}
102103

103-
DEBUG_BUILD && logger.log(`[Tracing] starting ${transaction.op} transaction - ${transaction.name}`);
104+
DEBUG_BUILD &&
105+
logger.log(`[Tracing] starting ${transaction.op} transaction - ${spanToJSON(transaction).description}`);
104106
return transaction;
105107
}
106108

packages/core/src/tracing/span.ts

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
TRACE_FLAG_NONE,
2121
TRACE_FLAG_SAMPLED,
2222
spanTimeInputToSeconds,
23+
spanToJSON,
2324
spanToTraceContext,
2425
spanToTraceHeader,
2526
} from '../utils/spanUtils';
@@ -84,11 +85,6 @@ export class Span implements SpanInterface {
8485
*/
8586
public op?: string;
8687

87-
/**
88-
* @inheritDoc
89-
*/
90-
public description?: string;
91-
9288
/**
9389
* @inheritDoc
9490
*/
@@ -128,6 +124,7 @@ export class Span implements SpanInterface {
128124
protected _traceId: string;
129125
protected _spanId: string;
130126
protected _sampled: boolean | undefined;
127+
protected _name?: string;
131128

132129
/**
133130
* You should never call the constructor manually, always use `Sentry.startTransaction()`
@@ -145,6 +142,8 @@ export class Span implements SpanInterface {
145142
this.attributes = spanContext.attributes || {};
146143
this.instrumenter = spanContext.instrumenter || 'sentry';
147144
this.origin = spanContext.origin || 'manual';
145+
// eslint-disable-next-line deprecation/deprecation
146+
this._name = spanContext.name || spanContext.description;
148147

149148
if (spanContext.parentSpanId) {
150149
this.parentSpanId = spanContext.parentSpanId;
@@ -156,12 +155,6 @@ export class Span implements SpanInterface {
156155
if (spanContext.op) {
157156
this.op = spanContext.op;
158157
}
159-
if (spanContext.description) {
160-
this.description = spanContext.description;
161-
}
162-
if (spanContext.name) {
163-
this.description = spanContext.name;
164-
}
165158
if (spanContext.status) {
166159
this.status = spanContext.status;
167160
}
@@ -170,20 +163,40 @@ export class Span implements SpanInterface {
170163
}
171164
}
172165

173-
// This conflicts with another eslint rule :(
166+
// This rule conflicts with another rule :(
174167
/* eslint-disable @typescript-eslint/member-ordering */
175168

176-
/** An alias for `description` of the Span. */
169+
/**
170+
* An alias for `description` of the Span.
171+
* @deprecated Use `spanToJSON(span).description` instead.
172+
*/
177173
public get name(): string {
178-
return this.description || '';
174+
return this._name || '';
179175
}
180176
/**
181177
* Update the name of the span.
178+
* @deprecated Use `spanToJSON(span).description` instead.
182179
*/
183180
public set name(name: string) {
184181
this.updateName(name);
185182
}
186183

184+
/**
185+
* Get the description of the Span.
186+
* @deprecated Use `spanToJSON(span).description` instead.
187+
*/
188+
public get description(): string | undefined {
189+
return this._name;
190+
}
191+
192+
/**
193+
* Get the description of the Span.
194+
* @deprecated Use `spanToJSON(span).description` instead.
195+
*/
196+
public set description(description: string | undefined) {
197+
this._name = description;
198+
}
199+
187200
/**
188201
* The ID of the trace.
189202
* @deprecated Use `spanContext().traceId` instead.
@@ -269,7 +282,7 @@ export class Span implements SpanInterface {
269282

270283
if (DEBUG_BUILD && childSpan.transaction) {
271284
const opStr = (spanContext && spanContext.op) || '< unknown op >';
272-
const nameStr = childSpan.transaction.name || '< unknown name >';
285+
const nameStr = spanToJSON(childSpan).description || '< unknown name >';
273286
const idStr = childSpan.transaction.spanContext().spanId;
274287

275288
const logMessage = `[Tracing] Starting '${opStr}' span on transaction '${nameStr}' (${idStr}).`;
@@ -342,7 +355,7 @@ export class Span implements SpanInterface {
342355
* @inheritDoc
343356
*/
344357
public updateName(name: string): this {
345-
this.description = name;
358+
this._name = name;
346359
return this;
347360
}
348361

@@ -392,7 +405,7 @@ export class Span implements SpanInterface {
392405
public toContext(): SpanContext {
393406
return dropUndefinedKeys({
394407
data: this._getData(),
395-
description: this.description,
408+
description: this._name,
396409
endTimestamp: this.endTimestamp,
397410
op: this.op,
398411
parentSpanId: this.parentSpanId,
@@ -410,7 +423,8 @@ export class Span implements SpanInterface {
410423
*/
411424
public updateWithContext(spanContext: SpanContext): this {
412425
this.data = spanContext.data || {};
413-
this.description = spanContext.description;
426+
// eslint-disable-next-line deprecation/deprecation
427+
this._name = spanContext.name || spanContext.description;
414428
this.endTimestamp = spanContext.endTimestamp;
415429
this.op = spanContext.op;
416430
this.parentSpanId = spanContext.parentSpanId;
@@ -437,7 +451,7 @@ export class Span implements SpanInterface {
437451
public getSpanJSON(): SpanJSON {
438452
return dropUndefinedKeys({
439453
data: this._getData(),
440-
description: this.description,
454+
description: this._name,
441455
op: this.op,
442456
parent_span_id: this.parentSpanId,
443457
span_id: this._spanId,

packages/core/src/tracing/trace.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,11 @@ export function trace<T>(
3939
// eslint-disable-next-line @typescript-eslint/no-empty-function
4040
afterFinish: () => void = () => {},
4141
): T {
42-
const ctx = normalizeContext(context);
43-
4442
const hub = getCurrentHub();
4543
const scope = getCurrentScope();
4644
const parentSpan = scope.getSpan();
4745

46+
const ctx = normalizeContext(context);
4847
const activeSpan = createChildSpanOrTransaction(hub, parentSpan, ctx);
4948

5049
scope.setSpan(activeSpan);
@@ -259,16 +258,12 @@ function createChildSpanOrTransaction(
259258
* Eventually the StartSpanOptions will be more aligned with OpenTelemetry.
260259
*/
261260
function normalizeContext(context: StartSpanOptions): TransactionContext {
262-
const ctx = { ...context };
263-
// If a name is set and a description is not, set the description to the name.
264-
if (ctx.name !== undefined && ctx.description === undefined) {
265-
ctx.description = ctx.name;
266-
}
267-
268261
if (context.startTime) {
262+
const ctx = { ...context };
269263
ctx.startTimestamp = spanTimeInputToSeconds(context.startTime);
270264
delete ctx.startTime;
265+
return ctx;
271266
}
272267

273-
return ctx;
268+
return context;
274269
}

0 commit comments

Comments
 (0)