Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ test('Sends two linked transactions (server & client) to Sentry', async ({ page
const httpServerTraceId = httpServerTransaction.contexts?.trace?.trace_id;
const httpServerSpanId = httpServerTransaction.contexts?.trace?.span_id;
const loaderSpanId = httpServerTransaction?.spans?.find(
span => span.data && span.data['code.function'] === 'loader',
span => span.data && span.data['code.function.name'] === 'loader',
)?.span_id;

const pageLoadTraceId = pageloadTransaction.contexts?.trace?.trace_id;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ test('Sends parameterized transaction name to Sentry', async ({ page }) => {

test('Sends form data with action span', async ({ page }) => {
const formdataActionTransaction = waitForTransaction('create-remix-app-express', transactionEvent => {
return transactionEvent?.spans?.some(span => span.data && span.data['code.function'] === 'action') || false;
return transactionEvent?.spans?.some(span => span.data && span.data['code.function.name'] === 'action') || false;
});

await page.goto('/action-formdata');
Expand All @@ -34,11 +34,12 @@ test('Sends form data with action span', async ({ page }) => {
await page.locator('button[type=submit]').click();

const actionSpan = (await formdataActionTransaction)?.spans?.find(
span => span.data && span.data['code.function'] === 'action',
span => span.data && span.data['code.function.name'] === 'action',
);

expect(actionSpan).toBeDefined();
expect(actionSpan?.op).toBe('action.remix');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: do we (need to) preserve this information somehow that this is action or loader? do we set code.function.name here? if we set it we should probably also assert on it

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes we set it and implicitly asserted it by using it in the .find() predicate above, but added an explicit assertion and changed to code.function.name since code.function will be deprecated 👍

expect(actionSpan?.op).toBe('function');
expect(actionSpan?.data?.['code.function.name']).toBe('action');
expect(actionSpan?.data).toMatchObject({
'formData.text': 'test',
'formData.file': 'file.txt',
Expand All @@ -47,17 +48,18 @@ test('Sends form data with action span', async ({ page }) => {

test('Sends a loader span to Sentry', async ({ page }) => {
const loaderTransactionPromise = waitForTransaction('create-remix-app-express', transactionEvent => {
return transactionEvent?.spans?.some(span => span.data && span.data['code.function'] === 'loader') || false;
return transactionEvent?.spans?.some(span => span.data && span.data['code.function.name'] === 'loader') || false;
});

await page.goto('/');

const loaderSpan = (await loaderTransactionPromise)?.spans?.find(
span => span.data && span.data['code.function'] === 'loader',
span => span.data && span.data['code.function.name'] === 'loader',
);

expect(loaderSpan).toBeDefined();
expect(loaderSpan?.op).toBe('loader.remix');
expect(loaderSpan?.op).toBe('function');
expect(loaderSpan?.data?.['code.function.name']).toBe('loader');
});

test('Propagates trace when ErrorBoundary is triggered', async ({ page }) => {
Expand All @@ -83,7 +85,7 @@ test('Propagates trace when ErrorBoundary is triggered', async ({ page }) => {
const httpServerTraceId = httpServerTransaction.contexts?.trace?.trace_id;
const httpServerSpanId = httpServerTransaction.contexts?.trace?.span_id;
const loaderSpanId = httpServerTransaction?.spans?.find(
span => span.data && span.data['code.function'] === 'loader',
span => span.data && span.data['code.function.name'] === 'loader',
)?.span_id;

const pageLoadTraceId = pageloadTransaction.contexts?.trace?.trace_id;
Expand Down Expand Up @@ -111,7 +113,7 @@ test('Parameterizes a 2-level nested route on the server', async ({ page }) => {
const transaction = await transactionPromise;

expect(transaction.contexts?.trace?.data?.['sentry.source']).toBe('route');
expect(transaction.spans?.some(s => s.data?.['code.function'] === 'loader' && s.op === 'loader.remix')).toBe(true);
expect(transaction.spans?.some(s => s.data?.['code.function.name'] === 'loader' && s.op === 'function')).toBe(true);
});

test('Parameterizes a 3-level nested API route on the server', async ({ page }) => {
Expand Down Expand Up @@ -160,19 +162,21 @@ test('Records action and loader spans on a parameterized action route', async ({
const transaction = await transactionPromise;

const actionSpan = transaction.spans?.find(
s => s.data?.['code.function'] === 'action' && s.data?.['match.route.id'] === 'routes/action-json-response.$id',
s =>
s.data?.['code.function.name'] === 'action' && s.data?.['match.route.id'] === 'routes/action-json-response.$id',
);
expect(actionSpan).toBeDefined();
expect(actionSpan?.op).toBe('action.remix');
expect(actionSpan?.op).toBe('function');
expect(actionSpan?.data?.['match.params.id']).toBe('123123');

const rootLoaderSpan = transaction.spans?.find(
s => s.data?.['code.function'] === 'loader' && s.data?.['match.route.id'] === 'root',
s => s.data?.['code.function.name'] === 'loader' && s.data?.['match.route.id'] === 'root',
);
expect(rootLoaderSpan).toBeDefined();

const routeLoaderSpan = transaction.spans?.find(
s => s.data?.['code.function'] === 'loader' && s.data?.['match.route.id'] === 'routes/action-json-response.$id',
s =>
s.data?.['code.function.name'] === 'loader' && s.data?.['match.route.id'] === 'routes/action-json-response.$id',
);
expect(routeLoaderSpan).toBeDefined();

Expand All @@ -191,7 +195,9 @@ test('Records loader spans on a deferred loader response', async ({ page }) => {
expect(transaction.contexts?.trace?.data?.['sentry.source']).toBe('route');
expect(
transaction.spans?.some(
s => s.data?.['code.function'] === 'loader' && s.data?.['match.route.id'] === 'routes/loader-defer-response.$id',
s =>
s.data?.['code.function.name'] === 'loader' &&
s.data?.['match.route.id'] === 'routes/loader-defer-response.$id',
),
).toBe(true);
});
Expand Down Expand Up @@ -263,7 +269,9 @@ test('Sends two linked transactions (server & client) to Sentry', async ({ page
const httpServerTraceId = httpServerTransaction.contexts?.trace?.trace_id;
const httpServerSpanId = httpServerTransaction.contexts?.trace?.span_id;

const loaderSpan = httpServerTransaction?.spans?.find(span => span.data && span.data['code.function'] === 'loader');
const loaderSpan = httpServerTransaction?.spans?.find(
span => span.data && span.data['code.function.name'] === 'loader',
);
const loaderSpanId = loaderSpan?.span_id;
const loaderParentSpanId = loaderSpan?.parent_span_id;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ test('Sends two linked transactions (server & client) to Sentry', async ({ page
const httpServerTraceId = httpServerTransaction.contexts?.trace?.trace_id;
const httpServerSpanId = httpServerTransaction.contexts?.trace?.span_id;
const loaderSpanId = httpServerTransaction?.spans?.find(
span => span.data && span.data['code.function'] === 'loader',
span => span.data && span.data['code.function.name'] === 'loader',
)?.span_id;

const pageLoadTraceId = pageloadTransaction.contexts?.trace?.trace_id;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ test('Sends two linked transactions (server & client) to Sentry', async ({ page
const httpServerTraceId = httpServerTransaction.contexts?.trace?.trace_id;
const httpServerSpanId = httpServerTransaction.contexts?.trace?.span_id;
const loaderSpanId = httpServerTransaction?.spans?.find(
span => span.data && span.data['code.function'] === 'loader',
span => span.data && span.data['code.function.name'] === 'loader',
)?.span_id;

const pageLoadTraceId = pageloadTransaction.contexts?.trace?.trace_id;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ test('Should set a "not_found" status on a server component span when notFound()
expect(transactionEvent.spans).toContainEqual(
expect.objectContaining({
description: 'resolve page server component "/server-component/not-found"',
op: 'function.nextjs',
op: 'function',
data: expect.objectContaining({
'sentry.nextjs.ssr.function.type': 'Page',
'sentry.nextjs.ssr.function.route': '/server-component/not-found',
Expand Down Expand Up @@ -122,7 +122,7 @@ test('Should capture an error and transaction for a app router page', async ({ p
expect(transactionEvent.spans).toContainEqual(
expect.objectContaining({
description: 'resolve page server component "/server-component/faulty"',
op: 'function.nextjs',
op: 'function',
data: expect.objectContaining({
'sentry.nextjs.ssr.function.type': 'Page',
'sentry.nextjs.ssr.function.route': '/server-component/faulty',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,15 @@ test.describe('server - instrumentation API error capture', () => {
const transaction = await txPromise;

// Find the loader span
const loaderSpan = transaction?.spans?.find(
(span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react_router.loader',
);
const loaderSpan = transaction?.spans?.find(span => span.data?.['code.function.name'] === 'loader');

expect(loaderSpan).toMatchObject({
data: {
'sentry.origin': 'auto.function.react_router.instrumentation_api',
'sentry.op': 'function.react_router.loader',
'sentry.op': 'function',
'code.function.name': 'loader',
},
op: 'function.react_router.loader',
op: 'function',
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ import { waitForTransaction } from '@sentry-internal/test-utils';
import { APP_NAME } from '../constants';

// As of React Router 7.15+, HydratedRouter invokes the client `fetch` hook in Framework Mode.
// A fetcher submission produces a `function.react_router.fetcher` transaction
// (origin `auto.function.react_router.instrumentation_api`) that nests the client action/loader
// spans and the `http.client` spans for the underlying `.data` requests.
// A fetcher submission produces a `function` transaction (origin
// `auto.function.react_router.instrumentation_api`, `code.function.name` `fetcher`) that nests the
// client action/loader spans and the `http.client` spans for the underlying `.data` requests.
// See: https://github.com/remix-run/react-router/discussions/13749

test.describe('client - instrumentation API fetcher', () => {
Expand All @@ -20,7 +20,7 @@ test.describe('client - instrumentation API fetcher', () => {
});

const fetcherTxPromise = waitForTransaction(APP_NAME, async transactionEvent => {
return transactionEvent.contexts?.trace?.op === 'function.react_router.fetcher';
return transactionEvent.contexts?.trace?.data?.['code.function.name'] === 'fetcher';
});

await page.goto(`/performance/fetcher-test`);
Expand All @@ -35,9 +35,9 @@ test.describe('client - instrumentation API fetcher', () => {
// The fetcher transaction nests the client action span and the http.client span(s) for the
// underlying `.data` request(s) - i.e. the OTel/browser fetch span is parented by the fetcher
// span, not emitted standalone.
const spanOps = (fetcherTx.spans ?? []).map(span => span.op);
expect(spanOps).toContain('function.react_router.client_action');
expect(spanOps).toContain('http.client');
const spans = fetcherTx.spans ?? [];
expect(spans.some(span => span.data?.['code.function.name'] === 'clientAction')).toBe(true);
expect(spans.map(span => span.op)).toContain('http.client');
});

test('should still send server action transaction when fetcher submits', async ({ page }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,21 @@ test.describe('server - instrumentation API lazy loading', () => {
});

// Find the lazy span
const lazySpan = transaction?.spans?.find(
(span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react_router.lazy',
);
const lazySpan = transaction?.spans?.find(span => span.data?.['code.function.name'] === 'lazy');

expect(lazySpan).toMatchObject({
span_id: expect.any(String),
trace_id: expect.any(String),
data: {
'sentry.origin': 'auto.function.react_router.instrumentation_api',
'sentry.op': 'function.react_router.lazy',
'sentry.op': 'function',
'code.function.name': 'lazy',
},
description: 'Lazy Route Load',
parent_span_id: expect.any(String),
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
op: 'function.react_router.lazy',
op: 'function',
origin: 'auto.function.react_router.instrumentation_api',
});
});
Expand All @@ -71,19 +70,18 @@ test.describe('server - instrumentation API lazy loading', () => {
const transaction = await txPromise;

// Find the loader span that runs after lazy loading
const loaderSpan = transaction?.spans?.find(
(span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react_router.loader',
);
const loaderSpan = transaction?.spans?.find(span => span.data?.['code.function.name'] === 'loader');

expect(loaderSpan).toMatchObject({
span_id: expect.any(String),
trace_id: expect.any(String),
data: {
'sentry.origin': 'auto.function.react_router.instrumentation_api',
'sentry.op': 'function.react_router.loader',
'sentry.op': 'function',
'code.function.name': 'loader',
},
description: '/performance/lazy-route',
op: 'function.react_router.loader',
op: 'function',
origin: 'auto.function.react_router.instrumentation_api',
});
});
Expand All @@ -97,13 +95,9 @@ test.describe('server - instrumentation API lazy loading', () => {

const transaction = await txPromise;

const lazySpan = transaction?.spans?.find(
(span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react_router.lazy',
);
const lazySpan = transaction?.spans?.find(span => span.data?.['code.function.name'] === 'lazy');

const loaderSpan = transaction?.spans?.find(
(span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react_router.loader',
);
const loaderSpan = transaction?.spans?.find(span => span.data?.['code.function.name'] === 'loader');

expect(lazySpan).toBeDefined();
expect(loaderSpan).toBeDefined();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,25 +37,24 @@ test.describe('server - instrumentation API middleware', () => {
});

// Find the middleware span
const middlewareSpan = transaction?.spans?.find(
(span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react_router.middleware',
);
const middlewareSpan = transaction?.spans?.find(span => span.data?.['code.function.name'] === 'middleware');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: are these actually middlewares or just functions that are called middleware? if the former shouldn't the op be middleware?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch! updated to middleware


expect(middlewareSpan).toBeDefined();
expect(middlewareSpan).toMatchObject({
span_id: expect.any(String),
trace_id: expect.any(String),
data: expect.objectContaining({
'sentry.origin': 'auto.function.react_router.instrumentation_api',
'sentry.op': 'function.react_router.middleware',
'sentry.op': 'middleware',
'code.function.name': 'middleware',
'react_router.route.id': 'routes/performance/with-middleware',
'http.route': '/performance/with-middleware',
'react_router.middleware.index': 0,
}),
parent_span_id: expect.any(String),
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
op: 'function.react_router.middleware',
op: 'middleware',
origin: 'auto.function.react_router.instrumentation_api',
});

Expand All @@ -73,13 +72,9 @@ test.describe('server - instrumentation API middleware', () => {

const transaction = await txPromise;

const middlewareSpan = transaction?.spans?.find(
(span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react_router.middleware',
);
const middlewareSpan = transaction?.spans?.find(span => span.data?.['code.function.name'] === 'middleware');

const loaderSpan = transaction?.spans?.find(
(span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react_router.loader',
);
const loaderSpan = transaction?.spans?.find(span => span.data?.['code.function.name'] === 'loader');

expect(middlewareSpan).toBeDefined();
expect(loaderSpan).toBeDefined();
Expand All @@ -100,9 +95,7 @@ test.describe('server - instrumentation API middleware', () => {
await expect(page.locator('#multi-middleware-title')).toBeVisible();
await expect(page.locator('#multi-middleware-content')).toHaveText('This route has 3 middlewares');

const middlewareSpans = transaction?.spans?.filter(
(span: { data?: { 'sentry.op'?: string } }) => span.data?.['sentry.op'] === 'function.react_router.middleware',
);
const middlewareSpans = transaction?.spans?.filter(span => span.data?.['code.function.name'] === 'middleware');

expect(middlewareSpans).toHaveLength(3);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,21 +103,22 @@ test.describe('server - instrumentation API performance', () => {
const transaction = await txPromise;

// Find the loader span
const loaderSpan = transaction?.spans?.find(span => span.data?.['sentry.op'] === 'function.react_router.loader');
const loaderSpan = transaction?.spans?.find(span => span.data?.['code.function.name'] === 'loader');

expect(loaderSpan).toMatchObject({
span_id: expect.any(String),
trace_id: expect.any(String),
data: {
'sentry.origin': 'auto.function.react_router.instrumentation_api',
'sentry.op': 'function.react_router.loader',
'sentry.op': 'function',
'code.function.name': 'loader',
},
description: '/performance/server-loader',
parent_span_id: expect.any(String),
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
status: 'ok',
op: 'function.react_router.loader',
op: 'function',
origin: 'auto.function.react_router.instrumentation_api',
});
});
Expand All @@ -133,21 +134,22 @@ test.describe('server - instrumentation API performance', () => {
const transaction = await txPromise;

// Find the action span
const actionSpan = transaction?.spans?.find(span => span.data?.['sentry.op'] === 'function.react_router.action');
const actionSpan = transaction?.spans?.find(span => span.data?.['code.function.name'] === 'action');

expect(actionSpan).toMatchObject({
span_id: expect.any(String),
trace_id: expect.any(String),
data: {
'sentry.origin': 'auto.function.react_router.instrumentation_api',
'sentry.op': 'function.react_router.action',
'sentry.op': 'function',
'code.function.name': 'action',
},
description: '/performance/server-action',
parent_span_id: expect.any(String),
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
status: 'ok',
op: 'function.react_router.action',
op: 'function',
origin: 'auto.function.react_router.instrumentation_api',
});
});
Expand Down
Loading
Loading