Skip to content

Commit f862dd6

Browse files
mydeamydea
authored
ref: Remove unnecessary checks for SpanJSON.data existence (#14731)
Since this was reworked in #14693 to always return something, we can safe some checks/fallbacks. This most likely does not change all the things, but focuses on the places that were easy to find/replace.
1 parent edbb214 commit f862dd6

File tree

24 files changed

+45
-44
lines changed

24 files changed

+45
-44
lines changed

‎packages/browser/src/tracing/request.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ function isPerformanceResourceTiming(entry: PerformanceEntry): entry is Performa
208208
* @param span A span that has yet to be finished, must contain `url` on data.
209209
*/
210210
function addHTTPTimings(span: Span): void {
211-
const { url } = spanToJSON(span).data || {};
211+
const { url } = spanToJSON(span).data;
212212

213213
if (!url || typeof url !== 'string') {
214214
return;

‎packages/browser/test/tracing/browserTracingIntegration.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ describe('browserTracingIntegration', () => {
426426
const pageloadSpan = getActiveSpan();
427427

428428
expect(spanToJSON(pageloadSpan!).description).toBe('changed');
429-
expect(spanToJSON(pageloadSpan!).data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]).toBe('custom');
429+
expect(spanToJSON(pageloadSpan!).data[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]).toBe('custom');
430430
});
431431

432432
describe('startBrowserTracingNavigationSpan', () => {
@@ -608,7 +608,7 @@ describe('browserTracingIntegration', () => {
608608
const pageloadSpan = getActiveSpan();
609609

610610
expect(spanToJSON(pageloadSpan!).description).toBe('changed');
611-
expect(spanToJSON(pageloadSpan!).data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]).toBe('custom');
611+
expect(spanToJSON(pageloadSpan!).data[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]).toBe('custom');
612612
});
613613

614614
it('sets the navigation span name on `scope.transactionName`', () => {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly<Partial<
9494
// Else, we generate it from the span
9595
const dsc = getDynamicSamplingContextFromClient(span.spanContext().traceId, client);
9696
const jsonSpan = spanToJSON(rootSpan);
97-
const attributes = jsonSpan.data || {};
97+
const attributes = jsonSpan.data;
9898
const maybeSampleRate = attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE];
9999

100100
if (maybeSampleRate != null) {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { getClient, getCurrentScope } from '../currentScopes';
2-
import type { Span, SpanAttributes, StartSpanOptions } from '../types-hoist';
2+
import type { Span, StartSpanOptions } from '../types-hoist';
33

44
import { DEBUG_BUILD } from '../debug-build';
55
import { SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON } from '../semanticAttributes';
@@ -255,7 +255,7 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
255255
return;
256256
}
257257

258-
const attributes: SpanAttributes = spanJSON.data || {};
258+
const attributes = spanJSON.data;
259259
if (!attributes[SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON]) {
260260
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON, _finishReason);
261261
}

‎packages/core/test/lib/tracing/sentrySpan.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ describe('SentrySpan', () => {
3030

3131
const spanJson = spanToJSON(span);
3232
expect(spanJson.description).toEqual('new name');
33-
expect(spanJson.data?.[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]).toEqual('custom');
33+
expect(spanJson.data[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]).toEqual('custom');
3434
});
3535
});
3636

‎packages/nestjs/src/sdk.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ export function init(options: NodeOptions | undefined = {}): NodeClient | undefi
3030
}
3131

3232
function addNestSpanAttributes(span: Span): void {
33-
const attributes = spanToJSON(span).data || {};
33+
const attributes = spanToJSON(span).data;
3434

3535
// this is one of: app_creation, request_context, handler
3636
const type = attributes['nestjs.type'];

‎packages/nestjs/src/setup.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ Module({
252252
export { SentryModule };
253253

254254
function addNestSpanAttributes(span: Span): void {
255-
const attributes = spanToJSON(span).data || {};
255+
const attributes = spanToJSON(span).data;
256256

257257
// this is one of: app_creation, request_context, handler
258258
const type = attributes['nestjs.type'];

‎packages/nextjs/src/server/index.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,6 @@ export function init(options: NodeOptions): NodeClient | undefined {
308308
event.type === 'transaction' &&
309309
event.contexts?.trace?.data?.['next.span_type'] === 'BaseServer.handleRequest'
310310
) {
311-
event.contexts.trace.data = event.contexts.trace.data || {};
312311
event.contexts.trace.data[SEMANTIC_ATTRIBUTE_SENTRY_OP] = 'http.server';
313312
event.contexts.trace.op = 'http.server';
314313

‎packages/node/src/integrations/tracing/connect.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ export const setupConnectErrorHandler = (app: ConnectApp): void => {
8989
};
9090

9191
function addConnectSpanAttributes(span: Span): void {
92-
const attributes = spanToJSON(span).data || {};
92+
const attributes = spanToJSON(span).data;
9393

9494
// this is one of: middleware, request_handler
9595
const type = attributes['connect.type'];

‎packages/node/src/integrations/tracing/express.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export const instrumentExpress = generateInstrumentOnce(
2626
requestHook(span) {
2727
addOriginToSpan(span, 'auto.http.otel.express');
2828

29-
const attributes = spanToJSON(span).data || {};
29+
const attributes = spanToJSON(span).data;
3030
// this is one of: middleware, request_handler, router
3131
const type = attributes['express.type'];
3232

‎packages/node/src/integrations/tracing/fastify.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ export function setupFastifyErrorHandler(fastify: Fastify): void {
136136
}
137137

138138
function addFastifySpanAttributes(span: Span): void {
139-
const attributes = spanToJSON(span).data || {};
139+
const attributes = spanToJSON(span).data;
140140

141141
// this is one of: middleware, request_handler
142142
const type = attributes['fastify.type'];

‎packages/node/src/integrations/tracing/graphql.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export const instrumentGraphql = generateInstrumentOnce<GraphqlOptions>(
4747
responseHook(span) {
4848
addOriginToSpan(span, 'auto.graphql.otel.graphql');
4949

50-
const attributes = spanToJSON(span).data || {};
50+
const attributes = spanToJSON(span).data;
5151

5252
// If operation.name is not set, we fall back to use operation.type only
5353
const operationType = attributes['graphql.operation.type'];
@@ -58,7 +58,7 @@ export const instrumentGraphql = generateInstrumentOnce<GraphqlOptions>(
5858

5959
// We guard to only do this on http.server spans
6060

61-
const rootSpanAttributes = spanToJSON(rootSpan).data || {};
61+
const rootSpanAttributes = spanToJSON(rootSpan).data;
6262

6363
const existingOperations = rootSpanAttributes[SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION] || [];
6464

‎packages/node/src/integrations/tracing/hapi/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ export async function setupHapiErrorHandler(server: Server): Promise<void> {
128128
}
129129

130130
function addHapiSpanAttributes(span: Span): void {
131-
const attributes = spanToJSON(span).data || {};
131+
const attributes = spanToJSON(span).data;
132132

133133
// this is one of: router, plugin, server.ext
134134
const type = attributes['hapi.type'];

‎packages/node/src/integrations/tracing/knex.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const _knexIntegration = (() => {
2222
const { data } = spanToJSON(span);
2323
// knex.version is always set in the span data
2424
// https://github.com/open-telemetry/opentelemetry-js-contrib/blob/0309caeafc44ac9cb13a3345b790b01b76d0497d/plugins/node/opentelemetry-instrumentation-knex/src/instrumentation.ts#L138
25-
if (data && 'knex.version' in data) {
25+
if ('knex.version' in data) {
2626
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.knex');
2727
}
2828
});

‎packages/node/src/integrations/tracing/koa.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ export const setupKoaErrorHandler = (app: { use: (arg0: (ctx: any, next: any) =>
105105
function addKoaSpanAttributes(span: Span): void {
106106
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.http.otel.koa');
107107

108-
const attributes = spanToJSON(span).data || {};
108+
const attributes = spanToJSON(span).data;
109109

110110
// this is one of: middleware, router
111111
const type = attributes['koa.type'];

‎packages/node/src/integrations/tracing/nest/nest.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ export function setupNestErrorHandler(app: MinimalNestJsApp, baseFilter: NestJsE
135135
}
136136

137137
function addNestSpanAttributes(span: Span): void {
138-
const attributes = spanToJSON(span).data || {};
138+
const attributes = spanToJSON(span).data;
139139

140140
// this is one of: app_creation, request_context, handler
141141
const type = attributes['nestjs.type'];

‎packages/node/src/integrations/tracing/redis.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ const cacheResponseHook: RedisResponseCustomAttributeFunction = (span: Span, red
4949

5050
// otel/ioredis seems to be using the old standard, as there was a change to those params: https://github.com/open-telemetry/opentelemetry-specification/issues/3199
5151
// We are using params based on the docs: https://opentelemetry.io/docs/specs/semconv/attributes-registry/network/
52-
const networkPeerAddress = spanToJSON(span).data?.['net.peer.name'];
53-
const networkPeerPort = spanToJSON(span).data?.['net.peer.port'];
52+
const networkPeerAddress = spanToJSON(span).data['net.peer.name'];
53+
const networkPeerPort = spanToJSON(span).data['net.peer.port'];
5454
if (networkPeerPort && networkPeerAddress) {
5555
span.setAttributes({ 'network.peer.address': networkPeerAddress, 'network.peer.port': networkPeerPort });
5656
}

‎packages/node/src/integrations/tracing/tedious.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const _tediousIntegration = (() => {
2727
client.on('spanStart', span => {
2828
const { description, data } = spanToJSON(span);
2929
// Tedius integration always set a span name and `db.system` attribute to `mssql`.
30-
if (!description || data?.['db.system'] !== 'mssql') {
30+
if (!description || data['db.system'] !== 'mssql') {
3131
return;
3232
}
3333

‎packages/node/src/integrations/tracing/vercelai/index.ts

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,24 +18,21 @@ const _vercelAIIntegration = (() => {
1818
for (const span of event.spans) {
1919
const { data: attributes, description: name } = span;
2020

21-
if (!attributes || !name || span.origin !== 'auto.vercelai.otel') {
21+
if (!name || span.origin !== 'auto.vercelai.otel') {
2222
continue;
2323
}
2424

25-
// attributes around token usage can only be set on span finish
26-
span.data = span.data || {};
27-
2825
if (attributes['ai.usage.completionTokens'] != undefined) {
29-
span.data['ai.completion_tokens.used'] = attributes['ai.usage.completionTokens'];
26+
attributes['ai.completion_tokens.used'] = attributes['ai.usage.completionTokens'];
3027
}
3128
if (attributes['ai.usage.promptTokens'] != undefined) {
32-
span.data['ai.prompt_tokens.used'] = attributes['ai.usage.promptTokens'];
29+
attributes['ai.prompt_tokens.used'] = attributes['ai.usage.promptTokens'];
3330
}
3431
if (
3532
typeof attributes['ai.usage.completionTokens'] == 'number' &&
3633
typeof attributes['ai.usage.promptTokens'] == 'number'
3734
) {
38-
span.data['ai.total_tokens.used'] =
35+
attributes['ai.total_tokens.used'] =
3936
attributes['ai.usage.completionTokens'] + attributes['ai.usage.promptTokens'];
4037
}
4138
}
@@ -51,7 +48,7 @@ const _vercelAIIntegration = (() => {
5148

5249
const { data: attributes, description: name } = spanToJSON(span);
5350

54-
if (!attributes || !name) {
51+
if (!name) {
5552
return;
5653
}
5754

‎packages/opentelemetry/src/propagator.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ function getCurrentURL(span: Span): string | undefined {
315315
const spanData = spanToJSON(span).data;
316316
// `ATTR_URL_FULL` is the new attribute, but we still support the old one, `SEMATTRS_HTTP_URL`, for now.
317317
// eslint-disable-next-line deprecation/deprecation
318-
const urlAttribute = spanData?.[SEMATTRS_HTTP_URL] || spanData?.[ATTR_URL_FULL];
318+
const urlAttribute = spanData[SEMATTRS_HTTP_URL] || spanData[ATTR_URL_FULL];
319319
if (typeof urlAttribute === 'string') {
320320
return urlAttribute;
321321
}

‎packages/opentelemetry/src/utils/enhanceDscWithOpenTelemetryRootSpanName.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ export function enhanceDscWithOpenTelemetryRootSpanName(client: Client): void {
2121
// This mutates the passed-in DSC
2222

2323
const jsonSpan = spanToJSON(rootSpan);
24-
const attributes = jsonSpan.data || {};
24+
const attributes = jsonSpan.data;
2525
const source = attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE];
2626

2727
const { description } = spanHasName(rootSpan) ? parseSpanDescription(rootSpan) : { description: undefined };

‎packages/remix/src/utils/integrations/opentelemetry.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ const _remixIntegration = (() => {
3434
}) satisfies IntegrationFn;
3535

3636
const addRemixSpanAttributes = (span: Span): void => {
37-
const attributes = spanToJSON(span).data || {};
37+
const attributes = spanToJSON(span).data;
3838

3939
// this is one of: loader, action, requestHandler
4040
const type = attributes['code.function'];

‎packages/vue/src/router.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ export function instrumentVueRouter(
105105
if (options.instrumentPageLoad && isPageLoadNavigation) {
106106
const activeRootSpan = getActiveRootSpan();
107107
if (activeRootSpan) {
108-
const existingAttributes = spanToJSON(activeRootSpan).data || {};
108+
const existingAttributes = spanToJSON(activeRootSpan).data;
109109
if (existingAttributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] !== 'custom') {
110110
activeRootSpan.updateName(spanName);
111111
activeRootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, transactionSource);

‎packages/vue/test/router.test.ts

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ import type { Span, SpanAttributes } from '@sentry/core';
88
import type { Route } from '../src/router';
99
import { instrumentVueRouter } from '../src/router';
1010

11+
const MOCK_SPAN = {
12+
spanContext: () => ({ traceId: '1234', spanId: '5678' }),
13+
};
14+
1115
const captureExceptionSpy = vi.spyOn(SentryBrowser, 'captureException');
1216
vi.mock('@sentry/core', async () => {
1317
const actual = await vi.importActual('@sentry/core');
@@ -76,7 +80,7 @@ describe('instrumentVueRouter()', () => {
7680
});
7781

7882
it('should return instrumentation that instruments VueRouter.onError', () => {
79-
const mockStartSpan = vi.fn();
83+
const mockStartSpan = vi.fn().mockReturnValue(MOCK_SPAN);
8084
instrumentVueRouter(
8185
mockVueRouter,
8286
{ routeLabel: 'name', instrumentPageLoad: true, instrumentNavigation: true },
@@ -103,7 +107,7 @@ describe('instrumentVueRouter()', () => {
103107
])(
104108
'should return instrumentation that instruments VueRouter.beforeEach(%s, %s) for navigations',
105109
(fromKey, toKey, transactionName, transactionSource) => {
106-
const mockStartSpan = vi.fn();
110+
const mockStartSpan = vi.fn().mockReturnValue(MOCK_SPAN);
107111
instrumentVueRouter(
108112
mockVueRouter,
109113
{ routeLabel: 'name', instrumentPageLoad: true, instrumentNavigation: true },
@@ -143,7 +147,8 @@ describe('instrumentVueRouter()', () => {
143147
'should return instrumentation that instruments VueRouter.beforeEach(%s, %s) for pageloads',
144148
(fromKey, toKey, transactionName, transactionSource) => {
145149
const mockRootSpan = {
146-
getSpanJSON: vi.fn().mockReturnValue({ op: 'pageload' }),
150+
...MOCK_SPAN,
151+
getSpanJSON: vi.fn().mockReturnValue({ op: 'pageload', data: {} }),
147152
updateName: vi.fn(),
148153
setAttribute: vi.fn(),
149154
setAttributes: vi.fn(),
@@ -183,7 +188,7 @@ describe('instrumentVueRouter()', () => {
183188
);
184189

185190
it('allows to configure routeLabel=path', () => {
186-
const mockStartSpan = vi.fn();
191+
const mockStartSpan = vi.fn().mockReturnValue(MOCK_SPAN);
187192
instrumentVueRouter(
188193
mockVueRouter,
189194
{ routeLabel: 'path', instrumentPageLoad: true, instrumentNavigation: true },
@@ -211,7 +216,7 @@ describe('instrumentVueRouter()', () => {
211216
});
212217

213218
it('allows to configure routeLabel=name', () => {
214-
const mockStartSpan = vi.fn();
219+
const mockStartSpan = vi.fn().mockReturnValue(MOCK_SPAN);
215220
instrumentVueRouter(
216221
mockVueRouter,
217222
{ routeLabel: 'name', instrumentPageLoad: true, instrumentNavigation: true },
@@ -240,6 +245,7 @@ describe('instrumentVueRouter()', () => {
240245

241246
it("doesn't overwrite a pageload transaction name it was set to custom before the router resolved the route", () => {
242247
const mockRootSpan = {
248+
...MOCK_SPAN,
243249
updateName: vi.fn(),
244250
setAttribute: vi.fn(),
245251
setAttributes: vi.fn(),
@@ -294,9 +300,7 @@ describe('instrumentVueRouter()', () => {
294300
});
295301

296302
it("updates the scope's `transactionName` when a route is resolved", () => {
297-
const mockStartSpan = vi.fn().mockImplementation(_ => {
298-
return {};
299-
});
303+
const mockStartSpan = vi.fn().mockReturnValue(MOCK_SPAN);
300304

301305
const scopeSetTransactionNameSpy = vi.fn();
302306

@@ -329,6 +333,7 @@ describe('instrumentVueRouter()', () => {
329333
'should return instrumentation that considers the instrumentPageLoad = %p',
330334
(instrumentPageLoad, expectedCallsAmount) => {
331335
const mockRootSpan = {
336+
...MOCK_SPAN,
332337
updateName: vi.fn(),
333338
setData: vi.fn(),
334339
setAttribute: vi.fn(),
@@ -367,7 +372,7 @@ describe('instrumentVueRouter()', () => {
367372
])(
368373
'should return instrumentation that considers the instrumentNavigation = %p',
369374
(instrumentNavigation, expectedCallsAmount) => {
370-
const mockStartSpan = vi.fn();
375+
const mockStartSpan = vi.fn().mockReturnValue(MOCK_SPAN);
371376
instrumentVueRouter(
372377
mockVueRouter,
373378
{ routeLabel: 'name', instrumentPageLoad: true, instrumentNavigation },
@@ -386,7 +391,7 @@ describe('instrumentVueRouter()', () => {
386391
);
387392

388393
it("doesn't throw when `next` is not available in the beforeEach callback (Vue Router 4)", () => {
389-
const mockStartSpan = vi.fn();
394+
const mockStartSpan = vi.fn().mockReturnValue(MOCK_SPAN);
390395
instrumentVueRouter(
391396
mockVueRouter,
392397
{ routeLabel: 'path', instrumentPageLoad: true, instrumentNavigation: true },

0 commit comments

Comments
 (0)