Skip to content
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
1 change: 1 addition & 0 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -1027,6 +1027,7 @@ The following span names were adjusted:
| `function` (Angular `TraceMethod`) | The decorator's `name` option in angle brackets | `<getUser>`, `<unnamed>` | The decorator's `name` option, or `Function execution` if it has none | `Login.ngOnInit`, `getUsers`, `Function execution` |
| `function` (SvelteKit) | The route the wrapped function ran for, or the raw URL path if the SDK couldn't resolve one | `/users/[id]`, `/users/123`, `GET /api/users/[id]` | The name of the wrapped function | `load`, `GET` |
| `function` (Ember route hooks) | The full route name | `slow-loading-route.index` | The hook the span wraps, matching its `code.function.name`. The route moves to `sentry.description` | `beforeModel`, `model`, `setupController` |
| `function` (React Router route hooks) | The route the hook ran for, the raw URL path if React Router matched no pattern, or the fetcher key | `/users/:id`, `/users/123`, `Fetcher fetcher-1` | The hook the span wraps, matching its `code.function.name`. The previous name moves to `sentry.description` | `loader`, `action`, `clientLoader`, `fetcher` |
| `function.gcp` | The request method and path for HTTP functions, otherwise the trigger's event or trigger type | `POST /users`, `google.pubsub.topic.publish`, `firebase.function.http.request` | The function name, or `Serverless function execution` if the SDK cannot resolve one | `myFunction`, `Serverless function execution` |
| `function.aws` | The Lambda function name | `my-function` | Unchanged, except that the SDK now falls back to `Serverless function execution` if it cannot resolve the function name | `my-function`, `Serverless function execution` |
| `graphql` | The graphql phase and, for operations, the operation name | `query GetUser`, `graphql.parse`, `graphql.resolve user.0.name` | The operation type, or the processing type where there is none | `GraphQL query`, `GraphQL parse`, `GraphQL resolve` |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,14 @@ test.describe('server - instrumentation API lazy loading', () => {
expect(loaderSpan).toMatchObject({
span_id: expect.any(String),
trace_id: expect.any(String),
name: '/performance/lazy-route',
name: 'loader',
});

expect(loaderSpan!.attributes).toMatchObject({
'sentry.origin': { value: 'auto.function.react_router.instrumentation_api', type: 'string' },
'sentry.op': { value: 'function', type: 'string' },
'code.function.name': { value: 'loader', type: 'string' },
'sentry.description': { value: '/performance/lazy-route', type: 'string' },
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ test.describe('server - instrumentation API performance', () => {
expect(loaderSpan).toMatchObject({
span_id: expect.any(String),
trace_id: expect.any(String),
name: '/performance/server-loader',
name: 'loader',
parent_span_id: expect.any(String),
start_timestamp: expect.any(Number),
end_timestamp: expect.any(Number),
Expand All @@ -97,6 +97,7 @@ test.describe('server - instrumentation API performance', () => {
'sentry.origin': { value: 'auto.function.react_router.instrumentation_api', type: 'string' },
'sentry.op': { value: 'function', type: 'string' },
'code.function.name': { value: 'loader', type: 'string' },
'sentry.description': { value: '/performance/server-loader', type: 'string' },
});
});

Expand All @@ -113,7 +114,7 @@ test.describe('server - instrumentation API performance', () => {
expect(actionSpan).toMatchObject({
span_id: expect.any(String),
trace_id: expect.any(String),
name: '/performance/server-action',
name: 'action',
parent_span_id: expect.any(String),
start_timestamp: expect.any(Number),
end_timestamp: expect.any(Number),
Expand All @@ -124,6 +125,7 @@ test.describe('server - instrumentation API performance', () => {
'sentry.origin': { value: 'auto.function.react_router.instrumentation_api', type: 'string' },
'sentry.op': { value: 'function', type: 'string' },
'code.function.name': { value: 'action', type: 'string' },
'sentry.description': { value: '/performance/server-action', type: 'string' },
});
});

Expand Down
29 changes: 26 additions & 3 deletions packages/react-router/src/client/createClientInstrumentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
import {
SENTRY_SEGMENT_NAME_SOURCE,
CODE_FUNCTION_NAME,
SENTRY_DESCRIPTION,
SENTRY_OP,
URL_FULL,
URL_TEMPLATE,
Expand Down Expand Up @@ -231,13 +232,21 @@ export function createSentryClientInstrumentation(
},

async fetch(callFetch, info) {
const client = getClient();
const hasSpanStreaming = !!client && hasSpanStreamingEnabled(client);
const description = `Fetcher ${info.fetcherKey}`;

await startSpan(
{
name: `Fetcher ${info.fetcherKey}`,
// With span streaming, a `function` span is named after the function it wraps. The
// fetcher key identifies a single fetcher instance and would be high cardinality.
name: hasSpanStreaming ? 'fetcher' : description,
attributes: {
[SENTRY_OP]: FUNCTION,
[CODE_FUNCTION_NAME]: 'fetcher',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api',
// Relay infers a `function` span's description from `code.function.name` alone, which drops the key.
...(hasSpanStreaming && { [SENTRY_DESCRIPTION]: description }),
},
},
async span => {
Expand Down Expand Up @@ -266,13 +275,20 @@ export function createSentryClientInstrumentation(
// pageload, so this only affects navigations.)
updateRootSpanRoute(routePattern, !!pattern);

const client = getClient();
const hasSpanStreaming = !!client && hasSpanStreamingEnabled(client);

await startSpan(
{
name: routePattern,
// With span streaming, a `function` span is named after the function it wraps, because
// `routePattern` falls back to the raw request path for routes without a pattern.
name: hasSpanStreaming ? 'clientLoader' : routePattern,
attributes: {
[SENTRY_OP]: FUNCTION,
[CODE_FUNCTION_NAME]: 'clientLoader',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api',
// Relay infers a `function` span's description from `code.function.name` alone, which drops the route.
...(hasSpanStreaming && { [SENTRY_DESCRIPTION]: routePattern }),
},
},
async span => {
Expand All @@ -293,13 +309,20 @@ export function createSentryClientInstrumentation(
const routePattern = pattern || urlPath;
updateRootSpanRoute(routePattern, !!pattern);

const client = getClient();
const hasSpanStreaming = !!client && hasSpanStreamingEnabled(client);

await startSpan(
{
name: routePattern,
// With span streaming, a `function` span is named after the function it wraps, because
// `routePattern` falls back to the raw request path for routes without a pattern.
name: hasSpanStreaming ? 'clientAction' : routePattern,
attributes: {
[SENTRY_OP]: FUNCTION,
[CODE_FUNCTION_NAME]: 'clientAction',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api',
// Relay infers a `function` span's description from `code.function.name` alone, which drops the route.
...(hasSpanStreaming && { [SENTRY_DESCRIPTION]: routePattern }),
},
},
async span => {
Expand Down
19 changes: 17 additions & 2 deletions packages/react-router/src/server/createServerInstrumentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
CODE_FUNCTION_NAME,
HTTP_REQUEST_METHOD,
HTTP_ROUTE,
SENTRY_DESCRIPTION,
SENTRY_OP,
URL_FULL,
URL_PATH,
Expand Down Expand Up @@ -144,13 +145,20 @@ export function createSentryServerInstrumentation(
const routePattern = normalizeRoutePath(pattern) || urlPath;
updateRootSpanWithRoute(info.request.method, pattern, urlPath);

const client = getClient();
const hasSpanStreaming = !!client && hasSpanStreamingEnabled(client);

await startSpan(
{
name: routePattern,
// With span streaming, a `function` span is named after the function it wraps, because
// `routePattern` falls back to the raw request path for routes without a pattern.
name: hasSpanStreaming ? 'loader' : routePattern,
attributes: {
[SENTRY_OP]: FUNCTION,
[CODE_FUNCTION_NAME]: 'loader',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api',
// Relay infers a `function` span's description from `code.function.name` alone, which drops the route.
...(hasSpanStreaming && { [SENTRY_DESCRIPTION]: routePattern }),
},
},
async span => {
Expand All @@ -172,13 +180,20 @@ export function createSentryServerInstrumentation(
const routePattern = normalizeRoutePath(pattern) || urlPath;
updateRootSpanWithRoute(info.request.method, pattern, urlPath);

const client = getClient();
const hasSpanStreaming = !!client && hasSpanStreamingEnabled(client);

await startSpan(
{
name: routePattern,
// With span streaming, a `function` span is named after the function it wraps, because
// `routePattern` falls back to the raw request path for routes without a pattern.
name: hasSpanStreaming ? 'action' : routePattern,
attributes: {
[SENTRY_OP]: FUNCTION,
[CODE_FUNCTION_NAME]: 'action',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.function.react_router.instrumentation_api',
// Relay infers a `function` span's description from `code.function.name` alone, which drops the route.
...(hasSpanStreaming && { [SENTRY_DESCRIPTION]: routePattern }),
},
},
async span => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ vi.mock('@sentry/browser', () => ({

// Span streaming is the default trace lifecycle, and it's what makes span names low cardinality.
const mockStreamingClient = { getOptions: () => ({ traceLifecycle: 'stream' }) };
const mockStaticClient = { getOptions: () => ({ traceLifecycle: 'static' }) };

describe('createSentryClientInstrumentation', () => {
beforeEach(() => {
Expand Down Expand Up @@ -206,6 +207,7 @@ describe('createSentryClientInstrumentation', () => {
const mockInstrument = vi.fn();

(coreBrowser.startSpan as any).mockImplementation((_opts: any, fn: any) => fn());
(core.getClient as any).mockReturnValue(mockStreamingClient);

const instrumentation = createSentryClientInstrumentation();
instrumentation.router?.({ instrument: mockInstrument });
Expand All @@ -221,10 +223,11 @@ describe('createSentryClientInstrumentation', () => {

expect(coreBrowser.startSpan).toHaveBeenCalledWith(
expect.objectContaining({
name: 'Fetcher fetcher-1',
name: 'fetcher',
attributes: expect.objectContaining({
'sentry.op': 'function',
'code.function.name': 'fetcher',
'sentry.description': 'Fetcher fetcher-1',
'sentry.origin': 'auto.function.react_router.instrumentation_api',
}),
}),
Expand All @@ -233,11 +236,35 @@ describe('createSentryClientInstrumentation', () => {
expect(mockCallFetch).toHaveBeenCalled();
});

it('keeps the fetcher key in the span name without span streaming', async () => {
const mockCallFetch = vi.fn().mockResolvedValue({ status: 'success', error: undefined });
const mockInstrument = vi.fn();

(coreBrowser.startSpan as any).mockImplementation((_opts: any, fn: any) => fn());
(core.getClient as any).mockReturnValue(mockStaticClient);

const instrumentation = createSentryClientInstrumentation();
instrumentation.router?.({ instrument: mockInstrument });

const hooks = mockInstrument.mock.calls[0]![0];

await hooks.fetch(mockCallFetch, { href: '/api/data', currentUrl: '/home', fetcherKey: 'fetcher-1' });

expect(coreBrowser.startSpan).toHaveBeenCalledWith(
expect.objectContaining({
name: 'Fetcher fetcher-1',
attributes: expect.not.objectContaining({ 'sentry.description': expect.anything() }),
}),
expect.any(Function),
);
});

it('should instrument route loader with spans', async () => {
const mockCallLoader = vi.fn().mockResolvedValue({ status: 'success', error: undefined });
const mockInstrument = vi.fn();

(coreBrowser.startSpan as any).mockImplementation((_opts: any, fn: any) => fn());
(core.getClient as any).mockReturnValue(mockStreamingClient);

const instrumentation = createSentryClientInstrumentation();
// Route has id, index, path as required properties
Expand All @@ -261,10 +288,11 @@ describe('createSentryClientInstrumentation', () => {

expect(coreBrowser.startSpan).toHaveBeenCalledWith(
expect.objectContaining({
name: '/users/:id',
name: 'clientLoader',
attributes: expect.objectContaining({
'sentry.op': 'function',
'code.function.name': 'clientLoader',
'sentry.description': '/users/:id',
'sentry.origin': 'auto.function.react_router.instrumentation_api',
}),
}),
Expand All @@ -278,6 +306,7 @@ describe('createSentryClientInstrumentation', () => {
const mockInstrument = vi.fn();

(coreBrowser.startSpan as any).mockImplementation((_opts: any, fn: any) => fn());
(core.getClient as any).mockReturnValue(mockStreamingClient);

const instrumentation = createSentryClientInstrumentation();
instrumentation.route?.({
Expand All @@ -299,10 +328,11 @@ describe('createSentryClientInstrumentation', () => {

expect(coreBrowser.startSpan).toHaveBeenCalledWith(
expect.objectContaining({
name: '/users/:id',
name: 'clientAction',
attributes: expect.objectContaining({
'sentry.op': 'function',
'code.function.name': 'clientAction',
'sentry.description': '/users/:id',
'sentry.origin': 'auto.function.react_router.instrumentation_api',
}),
}),
Expand Down Expand Up @@ -593,6 +623,7 @@ describe('createSentryClientInstrumentation', () => {
const mockInstrument = vi.fn();

(coreBrowser.startSpan as any).mockImplementation((_opts: any, fn: any) => fn());
(core.getClient as any).mockReturnValue(mockStaticClient);

const instrumentation = createSentryClientInstrumentation();
instrumentation.route?.({
Expand Down Expand Up @@ -620,6 +651,39 @@ describe('createSentryClientInstrumentation', () => {
);
});

it('keeps the raw pathname out of the span name with span streaming', async () => {
const mockCallLoader = vi.fn().mockResolvedValue({ status: 'success', error: undefined });
const mockInstrument = vi.fn();

(coreBrowser.startSpan as any).mockImplementation((_opts: any, fn: any) => fn());
(core.getClient as any).mockReturnValue(mockStreamingClient);

const instrumentation = createSentryClientInstrumentation();
instrumentation.route?.({
id: 'test-route',
index: false,
path: '/test',
instrument: mockInstrument,
});

const hooks = mockInstrument.mock.calls[0]![0];

await hooks.loader(mockCallLoader, {
request: { method: 'GET', url: 'http://example.com/users/123', headers: { get: () => null } },
params: { id: '123' },
unstable_pattern: undefined,
context: undefined,
});

expect(coreBrowser.startSpan).toHaveBeenCalledWith(
expect.objectContaining({
name: 'clientLoader',
attributes: expect.objectContaining({ 'sentry.description': '/users/123' }),
}),
expect.any(Function),
);
});

it('should instrument route middleware with spans', async () => {
const mockCallMiddleware = vi.fn().mockResolvedValue({ status: 'success', error: undefined });
const mockInstrument = vi.fn();
Expand Down
Loading
Loading