Skip to content
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

feat(nextjs/vercel-edge/cloudflare): Switch to OTEL for performance monitoring #13889

Merged
merged 30 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
a9a22e8
feat(vercel-edge): Use opentelemetry for `@sentry/vercel-edge` (#13742)
lforst Oct 7, 2024
af2f5cb
ref(nextjs): Move pages router instrumentation into separate folder (…
lforst Oct 7, 2024
121bcee
feat(nextjs): Use OTEL instrumentation for route handlers (#13887)
chargome Oct 8, 2024
fbb17f9
ref(nextjs): Make Pages Router API Routes instrumentation OTEL ready …
lforst Oct 8, 2024
5c9566e
Merge remote-tracking branch 'origin/develop' into lforst-nextjs-otel
lforst Oct 8, 2024
c00594a
ref: Disable incoming HTTP request instrumentation (#13918)
lforst Oct 9, 2024
1392e1b
Merge branch 'develop' into lforst-nextjs-otel
lforst Oct 10, 2024
496d98c
feat(nextjs): Fork isolation scope for root spans on Next.js edge run…
lforst Oct 10, 2024
92c0117
feat(nextjs): Fork isolation scope for root spans on Next.js Node.js …
lforst Oct 10, 2024
f24fc68
ref(nextjs): Make individual features opt into OTEL tracing (#13910)
lforst Oct 10, 2024
88656c2
feat(nextjs): Drop `BaseServer.handleRequest` without route (#13957)
lforst Oct 14, 2024
e979c75
fix(nextjs): Don't drop transactions for server components on navigat…
lforst Oct 15, 2024
b8162ae
Merge branch 'develop' into lforst-nextjs-otel
lforst Oct 15, 2024
83d90f7
test(nextjs): Adjust test for origin (#13985)
lforst Oct 15, 2024
b78f8da
Merge branch 'develop' into lforst-nextjs-otel
lforst Oct 16, 2024
c8d34da
fix dependency
lforst Oct 16, 2024
3577b6b
Nudge CI
lforst Oct 16, 2024
a90d899
Nudge CI
lforst Oct 16, 2024
74b68c7
feat(nextjs): Enable nextjs otel spans pages router (#13960)
chargome Oct 17, 2024
492c578
ref(nextjs): Switch edge wrapping to OTEL (#13937)
lforst Oct 17, 2024
a4bb383
Bump size limit
lforst Oct 17, 2024
eb1c1cd
fix(nextjs): Use `preprocessEvent` hook to improve span data (#14007)
lforst Oct 17, 2024
01bc887
Merge remote-tracking branch 'origin/develop' into lforst-nextjs-otel
lforst Oct 21, 2024
9e25f02
Merge remote-tracking branch 'origin/develop' into lforst-nextjs-otel
lforst Oct 22, 2024
e77cb72
fix(nextjs): Disable incoming http spans (#14052)
lforst Oct 22, 2024
dd57025
feat(nextjs): Include server action transaction in next tracing (#14048)
chargome Oct 23, 2024
9e2bd51
ref(nextjs): Fix tests (#14058)
lforst Oct 23, 2024
c533d21
Merge remote-tracking branch 'origin/develop' into lforst-nextjs-otel
lforst Oct 23, 2024
192c3a5
Always pre process dumps
lforst Oct 24, 2024
5bda357
fix(nextjs): Set span source to `url` for middlewares (#14074)
lforst Oct 24, 2024
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
2 changes: 2 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1037,6 +1037,7 @@ jobs:
retention-days: 7

- name: Pre-process E2E Test Dumps
if: always()
run: |
node ./scripts/normalize-e2e-test-dump-transaction-events.js

Expand Down Expand Up @@ -1193,6 +1194,7 @@ jobs:
run: pnpm ${{ matrix.assert-command || 'test:assert' }}

- name: Pre-process E2E Test Dumps
if: always()
run: |
node ./scripts/normalize-e2e-test-dump-transaction-events.js

Expand Down
2 changes: 1 addition & 1 deletion .size-limit.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ module.exports = [
import: createImport('init'),
ignore: ['next/router', 'next/constants'],
gzip: true,
limit: '39 KB',
limit: '39.1 KB',
},
// SvelteKit SDK (ESM)
{
Expand Down
10 changes: 4 additions & 6 deletions dev-packages/e2e-tests/publish-packages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,19 @@ const packageTarballPaths = glob.sync('packages/*/sentry-*.tgz', {

// Publish built packages to the fake registry
packageTarballPaths.forEach(tarballPath => {
// eslint-disable-next-line no-console
console.log(`Publishing tarball ${tarballPath} ...`);
// `--userconfig` flag needs to be before `publish`
childProcess.exec(
`npm --userconfig ${__dirname}/test-registry.npmrc publish ${tarballPath}`,
{
cwd: repositoryRoot, // Can't use __dirname here because npm would try to publish `@sentry-internal/e2e-tests`
encoding: 'utf8',
},
(err, stdout, stderr) => {
// eslint-disable-next-line no-console
console.log(stdout);
// eslint-disable-next-line no-console
console.log(stderr);
err => {
if (err) {
// eslint-disable-next-line no-console
console.error(err);
console.error(`Error publishing tarball ${tarballPath}`, err);
process.exit(1);
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"@types/node": "18.11.17",
"@types/react": "18.0.26",
"@types/react-dom": "18.0.9",
"next": "13.2.0",
"next": "13.5.7",
"react": "18.2.0",
"react-dom": "18.2.0",
"typescript": "4.9.5"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,38 @@ test('should create a pageload transaction when the `pages` directory is used',
type: 'transaction',
});
});

test('should create a pageload transaction with correct name when an error occurs in getServerSideProps', async ({
page,
}) => {
const transactionPromise = waitForTransaction('nextjs-13', async transactionEvent => {
return (
transactionEvent.transaction === '/[param]/error-getServerSideProps' &&
transactionEvent.contexts?.trace?.op === 'pageload'
);
});

await page.goto(`/something/error-getServerSideProps`, { waitUntil: 'networkidle' });

const transaction = await transactionPromise;

expect(transaction).toMatchObject({
contexts: {
trace: {
data: {
'sentry.op': 'pageload',
'sentry.origin': 'auto.pageload.nextjs.pages_router_instrumentation',
'sentry.source': 'route',
},
op: 'pageload',
origin: 'auto.pageload.nextjs.pages_router_instrumentation',
},
},
transaction: '/[param]/error-getServerSideProps',
transaction_info: { source: 'route' },
type: 'transaction',
});

// Ensure the transaction name is not '/_error'
expect(transaction.transaction).not.toBe('/_error');
});
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ test('should propagate serverside `getInitialProps` trace to client', async ({ p

const serverTransactionPromise = waitForTransaction('nextjs-13', async transactionEvent => {
return (
transactionEvent.transaction === '/[param]/withInitialProps' &&
transactionEvent.transaction === 'GET /[param]/withInitialProps' &&
transactionEvent.contexts?.trace?.op === 'http.server'
);
});
Expand Down Expand Up @@ -47,7 +47,7 @@ test('should propagate serverside `getInitialProps` trace to client', async ({ p
status: 'ok',
},
},
transaction: '/[param]/withInitialProps',
transaction: 'GET /[param]/withInitialProps',
transaction_info: {
source: 'route',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ test('Should record performance for getServerSideProps', async ({ page }) => {

const serverTransactionPromise = waitForTransaction('nextjs-13', async transactionEvent => {
return (
transactionEvent.transaction === '/[param]/withServerSideProps' &&
transactionEvent.transaction === 'GET /[param]/withServerSideProps' &&
transactionEvent.contexts?.trace?.op === 'http.server'
);
});
Expand Down Expand Up @@ -47,7 +47,7 @@ test('Should record performance for getServerSideProps', async ({ page }) => {
status: 'ok',
},
},
transaction: '/[param]/withServerSideProps',
transaction: 'GET /[param]/withServerSideProps',
transaction_info: {
source: 'route',
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { expect, test } from '@playwright/test';
import { waitForTransaction } from '@sentry-internal/test-utils';

test('should not automatically create transactions for routes that were excluded from auto wrapping (string)', async ({
test('should not apply build-time instrumentation for routes that were excluded from auto wrapping (string)', async ({
request,
}) => {
const transactionPromise = waitForTransaction('nextjs-13', async transactionEvent => {
Expand All @@ -13,17 +13,13 @@ test('should not automatically create transactions for routes that were excluded

expect(await (await request.get(`/api/endpoint-excluded-with-string`)).text()).toBe('{"success":true}');

let transactionPromiseReceived = false;
transactionPromise.then(() => {
transactionPromiseReceived = true;
});

await new Promise(resolve => setTimeout(resolve, 5_000));
const transaction = await transactionPromise;

expect(transactionPromiseReceived).toBe(false);
expect(transaction.contexts?.trace?.data?.['sentry.origin']).toBeDefined();
expect(transaction.contexts?.trace?.data?.['sentry.origin']).not.toBe('auto.http.nextjs'); // This is the origin set by the build time instrumentation
});

test('should not automatically create transactions for routes that were excluded from auto wrapping (regex)', async ({
test('should not apply build-time instrumentation for routes that were excluded from auto wrapping (regex)', async ({
request,
}) => {
const transactionPromise = waitForTransaction('nextjs-13', async transactionEvent => {
Expand All @@ -35,12 +31,8 @@ test('should not automatically create transactions for routes that were excluded

expect(await (await request.get(`/api/endpoint-excluded-with-regex`)).text()).toBe('{"success":true}');

let transactionPromiseReceived = false;
transactionPromise.then(() => {
transactionPromiseReceived = true;
});

await new Promise(resolve => setTimeout(resolve, 5_000));
const transaction = await transactionPromise;

expect(transactionPromiseReceived).toBe(false);
expect(transaction.contexts?.trace?.data?.['sentry.origin']).toBeDefined();
expect(transaction.contexts?.trace?.data?.['sentry.origin']).not.toBe('auto.http.nextjs'); // This is the origin set by the build time instrumentation
});
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ test('Should report an error event for errors thrown in getServerSideProps', asy

const transactionEventPromise = waitForTransaction('nextjs-13', transactionEvent => {
return (
transactionEvent.transaction === '/error-getServerSideProps' &&
transactionEvent.transaction === 'GET /[param]/error-getServerSideProps' &&
transactionEvent.contexts?.trace?.op === 'http.server'
);
});

await page.goto('/error-getServerSideProps');
await page.goto('/dogsaregreat/error-getServerSideProps');

expect(await errorEventPromise).toMatchObject({
contexts: {
Expand All @@ -40,7 +40,7 @@ test('Should report an error event for errors thrown in getServerSideProps', asy
url: expect.stringMatching(/^http.*\/error-getServerSideProps/),
},
timestamp: expect.any(Number),
transaction: 'getServerSideProps (/error-getServerSideProps)',
transaction: 'getServerSideProps (/[param]/error-getServerSideProps)',
});

expect(await transactionEventPromise).toMatchObject({
Expand All @@ -60,11 +60,11 @@ test('Should report an error event for errors thrown in getServerSideProps', asy
data: {
'http.response.status_code': 500,
'sentry.op': 'http.server',
'sentry.origin': 'auto.function.nextjs',
'sentry.origin': 'auto',
'sentry.source': 'route',
},
op: 'http.server',
origin: 'auto.function.nextjs',
origin: 'auto',
span_id: expect.any(String),
status: 'internal_error',
trace_id: expect.any(String),
Expand All @@ -80,7 +80,7 @@ test('Should report an error event for errors thrown in getServerSideProps', asy
},
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
transaction: '/error-getServerSideProps',
transaction: 'GET /[param]/error-getServerSideProps',
transaction_info: { source: 'route' },
type: 'transaction',
});
Expand All @@ -95,11 +95,12 @@ test('Should report an error event for errors thrown in getServerSideProps in pa

const transactionEventPromise = waitForTransaction('nextjs-13', transactionEvent => {
return (
transactionEvent.transaction === '/customPageExtension' && transactionEvent.contexts?.trace?.op === 'http.server'
transactionEvent.transaction === 'GET /[param]/customPageExtension' &&
transactionEvent.contexts?.trace?.op === 'http.server'
);
});

await page.goto('/customPageExtension');
await page.goto('/123/customPageExtension');

expect(await errorEventPromise).toMatchObject({
contexts: {
Expand All @@ -126,7 +127,7 @@ test('Should report an error event for errors thrown in getServerSideProps in pa
url: expect.stringMatching(/^http.*\/customPageExtension/),
},
timestamp: expect.any(Number),
transaction: 'getServerSideProps (/customPageExtension)',
transaction: 'getServerSideProps (/[param]/customPageExtension)',
});

expect(await transactionEventPromise).toMatchObject({
Expand All @@ -146,11 +147,11 @@ test('Should report an error event for errors thrown in getServerSideProps in pa
data: {
'http.response.status_code': 500,
'sentry.op': 'http.server',
'sentry.origin': 'auto.function.nextjs',
'sentry.origin': 'auto',
'sentry.source': 'route',
},
op: 'http.server',
origin: 'auto.function.nextjs',
origin: 'auto',
span_id: expect.any(String),
status: 'internal_error',
trace_id: expect.any(String),
Expand All @@ -166,7 +167,7 @@ test('Should report an error event for errors thrown in getServerSideProps in pa
},
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
transaction: '/customPageExtension',
transaction: 'GET /[param]/customPageExtension',
transaction_info: { source: 'route' },
type: 'transaction',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,31 +9,49 @@ export default function Layout({ children }: { children: React.ReactNode }) {
<h1>Layout (/)</h1>
<ul>
<li>
<Link href="/">/</Link>
<Link href="/" prefetch={false}>
/
</Link>
</li>
<li>
<Link href="/client-component">/client-component</Link>
<Link href="/client-component" prefetch={false}>
/client-component
</Link>
</li>
<li>
<Link href="/client-component/parameter/42">/client-component/parameter/42</Link>
<Link href="/client-component/parameter/42" prefetch={false}>
/client-component/parameter/42
</Link>
</li>
<li>
<Link href="/client-component/parameter/foo/bar/baz">/client-component/parameter/foo/bar/baz</Link>
<Link href="/client-component/parameter/foo/bar/baz" prefetch={false}>
/client-component/parameter/foo/bar/baz
</Link>
</li>
<li>
<Link href="/server-component">/server-component</Link>
<Link href="/server-component" prefetch={false}>
/server-component
</Link>
</li>
<li>
<Link href="/server-component/parameter/42">/server-component/parameter/42</Link>
<Link href="/server-component/parameter/42" prefetch={false}>
/server-component/parameter/42
</Link>
</li>
<li>
<Link href="/server-component/parameter/foo/bar/baz">/server-component/parameter/foo/bar/baz</Link>
<Link href="/server-component/parameter/foo/bar/baz" prefetch={false}>
/server-component/parameter/foo/bar/baz
</Link>
</li>
<li>
<Link href="/not-found">/not-found</Link>
<Link href="/not-found" prefetch={false}>
/not-found
</Link>
</li>
<li>
<Link href="/redirect">/redirect</Link>
<Link href="/redirect" prefetch={false}>
/redirect
</Link>
</li>
</ul>
<SpanContextProvider>{children}</SpanContextProvider>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,5 @@ export async function middleware(request: NextRequest) {

// See "Matching Paths" below to learn more
export const config = {
matcher: ['/api/endpoint-behind-middleware'],
matcher: ['/api/endpoint-behind-middleware', '/api/endpoint-behind-faulty-middleware'],
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,22 @@ export const config = {
export default async function handler() {
// Without a working async context strategy the two spans created by `Sentry.startSpan()` would be nested.

const outerSpanPromise = Sentry.withIsolationScope(() => {
return Sentry.startSpan({ name: 'outer-span' }, () => {
return new Promise<void>(resolve => setTimeout(resolve, 300));
});
const outerSpanPromise = Sentry.startSpan({ name: 'outer-span' }, () => {
return new Promise<void>(resolve => setTimeout(resolve, 300));
});

setTimeout(() => {
Sentry.withIsolationScope(() => {
return Sentry.startSpan({ name: 'inner-span' }, () => {
const innerSpanPromise = new Promise<void>(resolve => {
setTimeout(() => {
Sentry.startSpan({ name: 'inner-span' }, () => {
return new Promise<void>(resolve => setTimeout(resolve, 100));
}).then(() => {
resolve();
});
});
}, 100);
}, 100);
});

await outerSpanPromise;
await innerSpanPromise;

return new Response('ok', { status: 200 });
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import type { NextApiRequest, NextApiResponse } from 'next';

type Data = {
name: string;
};

export default function handler(req: NextApiRequest, res: NextApiResponse<Data>) {
res.status(200).json({ name: 'John Doe' });
}
Loading
Loading