Skip to content

Commit 34becef

Browse files
Lms24Lms24
committed
feat(core): Deprecate StartSpanOptions.origin in favour of passing attribute
partial fix more tests simplify ctor logic dropUndefinedKeys remove stronger typing of `SpanAttributes` review suggestions more replacements fix import
1 parent 8e21feb commit 34becef

File tree

20 files changed

+138
-47
lines changed

20 files changed

+138
-47
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
1+
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
22
import {
33
captureException,
44
continueTrace,
@@ -119,9 +119,11 @@ async function instrumentRequest(
119119
const res = await startSpan(
120120
{
121121
...traceCtx,
122+
attributes: {
123+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.astro',
124+
},
122125
name: `${method} ${interpolatedRoute || ctx.url.pathname}`,
123126
op: 'http.server',
124-
origin: 'auto.http.astro',
125127
status: 'ok',
126128
metadata: {
127129
// eslint-disable-next-line deprecation/deprecation

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ describe('sentryMiddleware', () => {
5858

5959
expect(startSpanSpy).toHaveBeenCalledWith(
6060
{
61+
attributes: {
62+
'sentry.origin': 'auto.http.astro',
63+
},
6164
data: {
6265
method: 'GET',
6366
url: 'https://mydomain.io/users/123/details',
@@ -66,7 +69,6 @@ describe('sentryMiddleware', () => {
6669
metadata: {},
6770
name: 'GET /users/[id]/details',
6871
op: 'http.server',
69-
origin: 'auto.http.astro',
7072
status: 'ok',
7173
},
7274
expect.any(Function), // the `next` function
@@ -94,6 +96,9 @@ describe('sentryMiddleware', () => {
9496

9597
expect(startSpanSpy).toHaveBeenCalledWith(
9698
{
99+
attributes: {
100+
'sentry.origin': 'auto.http.astro',
101+
},
97102
data: {
98103
method: 'GET',
99104
url: 'http://localhost:1234/a%xx',
@@ -102,7 +107,6 @@ describe('sentryMiddleware', () => {
102107
metadata: {},
103108
name: 'GET a%xx',
104109
op: 'http.server',
105-
origin: 'auto.http.astro',
106110
status: 'ok',
107111
},
108112
expect.any(Function), // the `next` function

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {
2+
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
23
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
34
Transaction,
45
captureException,
@@ -69,9 +70,11 @@ function instrumentBunServeOptions(serveOptions: Parameters<typeof Bun.serve>[0]
6970
ctx => {
7071
return startSpan(
7172
{
73+
attributes: {
74+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.bun.serve',
75+
},
7276
op: 'http.server',
7377
name: `${request.method} ${parsedUrl.path || '/'}`,
74-
origin: 'auto.http.bun.serve',
7578
...ctx,
7679
data,
7780
metadata: {

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,11 +130,15 @@ export class Span implements SpanInterface {
130130
this.tags = spanContext.tags ? { ...spanContext.tags } : {};
131131
// eslint-disable-next-line deprecation/deprecation
132132
this.data = spanContext.data ? { ...spanContext.data } : {};
133-
this._attributes = spanContext.attributes ? { ...spanContext.attributes } : {};
134133
// eslint-disable-next-line deprecation/deprecation
135134
this.instrumenter = spanContext.instrumenter || 'sentry';
136135

137-
this.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, spanContext.origin || 'manual');
136+
this._attributes = {};
137+
this.setAttributes({
138+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: spanContext.origin || 'manual',
139+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: spanContext.op,
140+
...spanContext.attributes,
141+
});
138142

139143
// eslint-disable-next-line deprecation/deprecation
140144
this._name = spanContext.name || spanContext.description;
@@ -146,9 +150,6 @@ export class Span implements SpanInterface {
146150
if ('sampled' in spanContext) {
147151
this._sampled = spanContext.sampled;
148152
}
149-
if (spanContext.op) {
150-
this.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, spanContext.op);
151-
}
152153
if (spanContext.status) {
153154
this._status = spanContext.status;
154155
}

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { handleCallbackErrors } from '../utils/handleCallbackErrors';
2020
import { hasTracingEnabled } from '../utils/hasTracingEnabled';
2121
import { spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils';
2222

23-
interface StartSpanOptions extends TransactionContext {
23+
interface StartSpanOptions extends Omit<TransactionContext, 'origin'> {
2424
/** A manually specified start time for the created `Span` object. */
2525
startTime?: SpanTimeInput;
2626

@@ -33,7 +33,11 @@ interface StartSpanOptions extends TransactionContext {
3333
/** An op for the span. This is a categorization for spans. */
3434
op?: string;
3535

36-
/** The origin of the span - if it comes from auto instrumenation or manual instrumentation. */
36+
/**
37+
* The origin of the span - if it comes from auto instrumentation or manual instrumentation.
38+
*
39+
* @deprecated Set `attributes[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]` instead.
40+
*/
3741
origin?: SpanOrigin;
3842

3943
/** Attributes for the span. */

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,40 @@ describe('startSpan', () => {
229229
expect(ref.spanRecorder.spans).toHaveLength(2);
230230
expect(spanToJSON(ref.spanRecorder.spans[1]).op).toEqual('db.query');
231231
});
232+
233+
it.each([
234+
{ origin: 'auto.http.browser' },
235+
{ attributes: { 'sentry.origin': 'auto.http.browser' } },
236+
// attribute should take precedence over top level origin
237+
{ origin: 'manual', attributes: { 'sentry.origin': 'auto.http.browser' } },
238+
])('correctly sets the span origin', async () => {
239+
let ref: any = undefined;
240+
client.on('finishTransaction', transaction => {
241+
ref = transaction;
242+
});
243+
try {
244+
await startSpan({ name: 'GET users/[id]', origin: 'auto.http.browser' }, () => {
245+
return callback();
246+
});
247+
} catch (e) {
248+
//
249+
}
250+
251+
const jsonSpan = spanToJSON(ref);
252+
expect(jsonSpan).toEqual({
253+
data: {
254+
'sentry.origin': 'auto.http.browser',
255+
'sentry.sample_rate': 0,
256+
},
257+
origin: 'auto.http.browser',
258+
description: 'GET users/[id]',
259+
span_id: expect.any(String),
260+
start_timestamp: expect.any(Number),
261+
status: isError ? 'internal_error' : undefined,
262+
timestamp: expect.any(Number),
263+
trace_id: expect.any(String),
264+
});
265+
});
232266
});
233267

234268
it('creates & finishes span', async () => {

‎packages/ember/addon/index.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { getOwnConfig, isDevelopingApp, macroCondition } from '@embroider/macros
55
import { startSpan } from '@sentry/browser';
66
import type { BrowserOptions } from '@sentry/browser';
77
import * as Sentry from '@sentry/browser';
8-
import { applySdkMetadata } from '@sentry/core';
8+
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, applySdkMetadata } from '@sentry/core';
99
import { GLOBAL_OBJ } from '@sentry/utils';
1010
import Ember from 'ember';
1111

@@ -82,9 +82,11 @@ export const instrumentRoutePerformance = <T extends RouteConstructor>(BaseRoute
8282
): Promise<ReturnType<X>> => {
8383
return startSpan(
8484
{
85+
attributes: {
86+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'ember',
87+
},
8588
op,
8689
name: description,
87-
origin: 'auto.ui.ember',
8890
},
8991
() => {
9092
return fn(...args);

‎packages/nextjs/src/common/utils/edgeWrapperUtils.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {
2+
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
23
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
34
addTracingExtensions,
45
captureException,
@@ -40,8 +41,10 @@ export function withEdgeWrapping<H extends EdgeRouteHandler>(
4041
...transactionContext,
4142
name: options.spanDescription,
4243
op: options.spanOp,
43-
origin: 'auto.function.nextjs.withEdgeWrapping',
44-
attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route' },
44+
attributes: {
45+
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
46+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.withEdgeWrapping',
47+
},
4548
metadata: {
4649
// eslint-disable-next-line deprecation/deprecation
4750
...transactionContext.metadata,

‎packages/nextjs/test/config/withSentry.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as SentryCore from '@sentry/core';
2-
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, addTracingExtensions } from '@sentry/core';
2+
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, addTracingExtensions } from '@sentry/core';
33
import type { NextApiRequest, NextApiResponse } from 'next';
44

55
import type { AugmentedNextApiResponse, NextApiHandler } from '../../src/common/types';
@@ -44,9 +44,9 @@ describe('withSentry', () => {
4444
{
4545
name: 'GET http://dogs.are.great',
4646
op: 'http.server',
47-
origin: 'auto.http.nextjs',
4847
attributes: {
4948
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
49+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.nextjs',
5050
},
5151
metadata: {
5252
request: expect.objectContaining({ url: 'http://dogs.are.great' }),

‎packages/nextjs/test/edge/edgeWrapperUtils.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,10 @@ describe('withEdgeWrapping', () => {
8787
},
8888
attributes: {
8989
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
90+
[coreSdk.SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.withEdgeWrapping',
9091
},
9192
name: 'some label',
9293
op: 'some op',
93-
origin: 'auto.function.nextjs.withEdgeWrapping',
9494
}),
9595
expect.any(Function),
9696
);

0 commit comments

Comments
 (0)