Skip to content

feat(core): Deprecate StartSpanOptions.origin in favour of passing attribute #10274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions packages/astro/src/server/middleware.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
import {
captureException,
continueTrace,
Expand Down Expand Up @@ -119,9 +119,11 @@ async function instrumentRequest(
const res = await startSpan(
{
...traceCtx,
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.astro',
},
name: `${method} ${interpolatedRoute || ctx.url.pathname}`,
op: 'http.server',
origin: 'auto.http.astro',
status: 'ok',
metadata: {
// eslint-disable-next-line deprecation/deprecation
Expand Down
8 changes: 6 additions & 2 deletions packages/astro/test/server/middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ describe('sentryMiddleware', () => {

expect(startSpanSpy).toHaveBeenCalledWith(
{
attributes: {
'sentry.origin': 'auto.http.astro',
},
data: {
method: 'GET',
url: 'https://mydomain.io/users/123/details',
Expand All @@ -66,7 +69,6 @@ describe('sentryMiddleware', () => {
metadata: {},
name: 'GET /users/[id]/details',
op: 'http.server',
origin: 'auto.http.astro',
status: 'ok',
},
expect.any(Function), // the `next` function
Expand Down Expand Up @@ -94,6 +96,9 @@ describe('sentryMiddleware', () => {

expect(startSpanSpy).toHaveBeenCalledWith(
{
attributes: {
'sentry.origin': 'auto.http.astro',
},
data: {
method: 'GET',
url: 'http://localhost:1234/a%xx',
Expand All @@ -102,7 +107,6 @@ describe('sentryMiddleware', () => {
metadata: {},
name: 'GET a%xx',
op: 'http.server',
origin: 'auto.http.astro',
status: 'ok',
},
expect.any(Function), // the `next` function
Expand Down
5 changes: 4 additions & 1 deletion packages/bun/src/integrations/bunserver.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
Transaction,
captureException,
Expand Down Expand Up @@ -69,9 +70,11 @@ function instrumentBunServeOptions(serveOptions: Parameters<typeof Bun.serve>[0]
ctx => {
return startSpan(
{
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.bun.serve',
},
op: 'http.server',
name: `${request.method} ${parsedUrl.path || '/'}`,
origin: 'auto.http.bun.serve',
...ctx,
data,
metadata: {
Expand Down
11 changes: 6 additions & 5 deletions packages/core/src/tracing/span.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,15 @@ export class Span implements SpanInterface {
this.tags = spanContext.tags ? { ...spanContext.tags } : {};
// eslint-disable-next-line deprecation/deprecation
this.data = spanContext.data ? { ...spanContext.data } : {};
this._attributes = spanContext.attributes ? { ...spanContext.attributes } : {};
// eslint-disable-next-line deprecation/deprecation
this.instrumenter = spanContext.instrumenter || 'sentry';

this.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, spanContext.origin || 'manual');
this._attributes = {};
this.setAttributes({
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: spanContext.origin || 'manual',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: spanContext.op,
...spanContext.attributes,
});

// eslint-disable-next-line deprecation/deprecation
this._name = spanContext.name || spanContext.description;
Expand All @@ -146,9 +150,6 @@ export class Span implements SpanInterface {
if ('sampled' in spanContext) {
this._sampled = spanContext.sampled;
}
if (spanContext.op) {
this.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, spanContext.op);
}
if (spanContext.status) {
this._status = spanContext.status;
}
Expand Down
8 changes: 6 additions & 2 deletions packages/core/src/tracing/trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { handleCallbackErrors } from '../utils/handleCallbackErrors';
import { hasTracingEnabled } from '../utils/hasTracingEnabled';
import { spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils';

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

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

/** The origin of the span - if it comes from auto instrumenation or manual instrumentation. */
/**
* The origin of the span - if it comes from auto instrumentation or manual instrumentation.
*
* @deprecated Set `attributes[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]` instead.
*/
origin?: SpanOrigin;

/** Attributes for the span. */
Expand Down
34 changes: 34 additions & 0 deletions packages/core/test/lib/tracing/trace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,40 @@ describe('startSpan', () => {
expect(ref.spanRecorder.spans).toHaveLength(2);
expect(spanToJSON(ref.spanRecorder.spans[1]).op).toEqual('db.query');
});

it.each([
{ origin: 'auto.http.browser' },
{ attributes: { 'sentry.origin': 'auto.http.browser' } },
// attribute should take precedence over top level origin
{ origin: 'manual', attributes: { 'sentry.origin': 'auto.http.browser' } },
])('correctly sets the span origin', async () => {
let ref: any = undefined;
client.on('finishTransaction', transaction => {
ref = transaction;
});
try {
await startSpan({ name: 'GET users/[id]', origin: 'auto.http.browser' }, () => {
return callback();
});
} catch (e) {
//
}

const jsonSpan = spanToJSON(ref);
expect(jsonSpan).toEqual({
data: {
'sentry.origin': 'auto.http.browser',
'sentry.sample_rate': 0,
},
origin: 'auto.http.browser',
description: 'GET users/[id]',
span_id: expect.any(String),
start_timestamp: expect.any(Number),
status: isError ? 'internal_error' : undefined,
timestamp: expect.any(Number),
trace_id: expect.any(String),
});
});
});

it('creates & finishes span', async () => {
Expand Down
6 changes: 4 additions & 2 deletions packages/ember/addon/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { getOwnConfig, isDevelopingApp, macroCondition } from '@embroider/macros
import { startSpan } from '@sentry/browser';
import type { BrowserOptions } from '@sentry/browser';
import * as Sentry from '@sentry/browser';
import { applySdkMetadata } from '@sentry/core';
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, applySdkMetadata } from '@sentry/core';
import { GLOBAL_OBJ } from '@sentry/utils';
import Ember from 'ember';

Expand Down Expand Up @@ -82,9 +82,11 @@ export const instrumentRoutePerformance = <T extends RouteConstructor>(BaseRoute
): Promise<ReturnType<X>> => {
return startSpan(
{
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'ember',
},
op,
name: description,
origin: 'auto.ui.ember',
},
() => {
return fn(...args);
Expand Down
13 changes: 10 additions & 3 deletions packages/ember/addon/instance-initializers/sentry-performance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { ExtendedBackburner } from '@sentry/ember/runloop';
import type { Span, Transaction } from '@sentry/types';
import { GLOBAL_OBJ, browserPerformanceTimeOrigin, timestampInSeconds } from '@sentry/utils';

import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
import type { BrowserClient } from '..';
import { getActiveSpan, startInactiveSpan } from '..';
import type { EmberRouterMain, EmberSentryConfig, GlobalConfig, OwnConfig, StartTransactionFunction } from '../types';
Expand Down Expand Up @@ -150,9 +151,11 @@ export function _instrumentEmberRouter(
},
});
transitionSpan = startInactiveSpan({
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.ember',
},
op: 'ui.ember.transition',
name: `route:${fromRoute} -> route:${toRoute}`,
origin: 'auto.ui.ember',
});
});

Expand Down Expand Up @@ -212,9 +215,11 @@ function _instrumentEmberRunloop(config: EmberSentryConfig): void {

if ((now - currentQueueStart) * 1000 >= minQueueDuration) {
startInactiveSpan({
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.ember',
},
name: 'runloop',
op: `ui.ember.runloop.${queue}`,
origin: 'auto.ui.ember',
startTimestamp: currentQueueStart,
})?.end(now);
}
Expand Down Expand Up @@ -370,7 +375,9 @@ function _instrumentInitialLoad(config: EmberSentryConfig): void {
startInactiveSpan({
op: 'ui.ember.init',
name: 'init',
origin: 'auto.ui.ember',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.ember',
},
startTimestamp,
})?.end(endTimestamp);
performance.clearMarks(startName);
Expand Down
7 changes: 5 additions & 2 deletions packages/nextjs/src/common/utils/edgeWrapperUtils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
addTracingExtensions,
captureException,
Expand Down Expand Up @@ -40,8 +41,10 @@ export function withEdgeWrapping<H extends EdgeRouteHandler>(
...transactionContext,
name: options.spanDescription,
op: options.spanOp,
origin: 'auto.function.nextjs.withEdgeWrapping',
attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route' },
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.withEdgeWrapping',
},
metadata: {
// eslint-disable-next-line deprecation/deprecation
...transactionContext.metadata,
Expand Down
3 changes: 2 additions & 1 deletion packages/nextjs/src/common/wrapApiHandlerWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
} from '@sentry/core';
import { consoleSandbox, isString, logger, objectify, stripUrlQueryAndFragment } from '@sentry/utils';

import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
import type { AugmentedNextApiRequest, AugmentedNextApiResponse, NextApiHandler } from './types';
import { platformSupportsStreaming } from './utils/platformSupportsStreaming';
import { flushQueue } from './utils/responseEnd';
Expand Down Expand Up @@ -108,9 +109,9 @@ export function withSentry(apiHandler: NextApiHandler, parameterizedRoute?: stri
...transactionContext,
name: `${reqMethod}${reqPath}`,
op: 'http.server',
origin: 'auto.http.nextjs',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.nextjs',
},
metadata: {
// eslint-disable-next-line deprecation/deprecation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
import type { WebFetchHeaders } from '@sentry/types';
import { winterCGHeadersToDict } from '@sentry/utils';

import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
import type { GenerationFunctionContext } from '../common/types';
import { isNotFoundNavigationError, isRedirectNavigationError } from './nextNavigationErrorUtils';
import { commonObjectToPropagationContext } from './utils/commonObjectTracing';
Expand Down Expand Up @@ -67,11 +68,11 @@ export function wrapGenerationFunctionWithSentry<F extends (...args: any[]) => a
{
op: 'function.nextjs',
name: `${componentType}.${generationFunctionIdentifier} (${componentRoute})`,
origin: 'auto.function.nextjs',
...transactionContext,
data,
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs',
},
metadata: {
// eslint-disable-next-line deprecation/deprecation
Expand Down
3 changes: 2 additions & 1 deletion packages/nextjs/src/common/wrapServerComponentWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
} from '@sentry/core';
import { winterCGHeadersToDict } from '@sentry/utils';

import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
import { isNotFoundNavigationError, isRedirectNavigationError } from '../common/nextNavigationErrorUtils';
import type { ServerComponentContext } from '../common/types';
import { commonObjectToPropagationContext } from './utils/commonObjectTracing';
Expand Down Expand Up @@ -61,9 +62,9 @@ export function wrapServerComponentWithSentry<F extends (...args: any[]) => any>
op: 'function.nextjs',
name: `${componentType} Server Component (${componentRoute})`,
status: 'ok',
origin: 'auto.function.nextjs',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'component',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs',
},
metadata: {
// eslint-disable-next-line deprecation/deprecation
Expand Down
4 changes: 2 additions & 2 deletions packages/nextjs/test/config/withSentry.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as SentryCore from '@sentry/core';
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, addTracingExtensions } from '@sentry/core';
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, addTracingExtensions } from '@sentry/core';
import type { NextApiRequest, NextApiResponse } from 'next';

import type { AugmentedNextApiResponse, NextApiHandler } from '../../src/common/types';
Expand Down Expand Up @@ -44,9 +44,9 @@ describe('withSentry', () => {
{
name: 'GET http://dogs.are.great',
op: 'http.server',
origin: 'auto.http.nextjs',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.nextjs',
},
metadata: {
request: expect.objectContaining({ url: 'http://dogs.are.great' }),
Expand Down
2 changes: 1 addition & 1 deletion packages/nextjs/test/edge/edgeWrapperUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,10 @@ describe('withEdgeWrapping', () => {
},
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[coreSdk.SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.withEdgeWrapping',
},
name: 'some label',
op: 'some op',
origin: 'auto.function.nextjs.withEdgeWrapping',
}),
expect.any(Function),
);
Expand Down
6 changes: 3 additions & 3 deletions packages/nextjs/test/edge/withSentryAPI.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as coreSdk from '@sentry/core';
import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';

import { wrapApiHandlerWithSentry } from '../../src/edge';

Expand Down Expand Up @@ -58,10 +58,10 @@ describe('wrapApiHandlerWithSentry', () => {
},
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.withEdgeWrapping',
},
name: 'POST /user/[userId]/post/[postId]',
op: 'http.server',
origin: 'auto.function.nextjs.withEdgeWrapping',
}),
expect.any(Function),
);
Expand All @@ -80,10 +80,10 @@ describe('wrapApiHandlerWithSentry', () => {
metadata: {},
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[coreSdk.SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.nextjs.withEdgeWrapping',
},
name: 'handler (/user/[userId]/post/[postId])',
op: 'http.server',
origin: 'auto.function.nextjs.withEdgeWrapping',
}),
expect.any(Function),
);
Expand Down
5 changes: 4 additions & 1 deletion packages/remix/src/utils/instrumentServer.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint-disable max-lines */
import {
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
getActiveSpan,
getActiveTransaction,
getClient,
Expand Down Expand Up @@ -411,7 +412,9 @@ export function startRequestHandlerTransaction(
const transaction = hub.startTransaction({
name,
op: 'http.server',
origin: 'auto.http.remix',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.remix',
},
tags: {
method: request.method,
},
Expand Down
Loading