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): Add performance monitoring to server components #7242

Merged
merged 38 commits into from
Feb 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
948251f
feat(nextjs): Add performance monitoring to server components
lforst Feb 21, 2023
9b96913
Use original comp
lforst Feb 21, 2023
eb9b72b
Fix tests
lforst Feb 22, 2023
3b4496f
Merge branch 'develop' into lforst-perf-instrument-server-components
lforst Feb 22, 2023
041cb5e
Lint
lforst Feb 22, 2023
351f6f4
lil test
lforst Feb 22, 2023
4822861
test: Add test utitilty to intercept sentry requests
lforst Feb 23, 2023
2e832ca
Add some comments
lforst Feb 23, 2023
313093e
Merge branch 'lforst-event-proxy-server' into lforst-perf-instrument-…
lforst Feb 23, 2023
d379fc8
Add tests
lforst Feb 23, 2023
b41ce3b
cjs
lforst Feb 23, 2023
61cad4d
Merge branch 'develop' into lforst-event-proxy-server
lforst Feb 23, 2023
ffcc206
.
lforst Feb 23, 2023
94daab4
move
lforst Feb 23, 2023
6fcff66
Merge branch 'lforst-event-proxy-server' into lforst-perf-instrument-…
lforst Feb 23, 2023
24249cf
.
lforst Feb 23, 2023
b505bf2
.
lforst Feb 23, 2023
a70e3fa
.
lforst Feb 23, 2023
a3eb731
.
lforst Feb 23, 2023
cc3905a
.
lforst Feb 23, 2023
4170568
.
lforst Feb 23, 2023
d9a8153
.
lforst Feb 23, 2023
130084c
pls finally pass
lforst Feb 24, 2023
41f266a
sigh
lforst Feb 24, 2023
bb16d7d
remove console.log
lforst Feb 24, 2023
f1b5be4
.
lforst Feb 24, 2023
493098b
work pls?
lforst Feb 24, 2023
aadeb29
Update package.json
lforst Feb 24, 2023
6f4aa7b
debug
lforst Feb 24, 2023
2493a52
pls finally work
lforst Feb 24, 2023
b5feedc
Friggin finally
lforst Feb 24, 2023
ae7ca3d
spare me
lforst Feb 24, 2023
9090f79
Extract server port registration into helper function
lforst Feb 24, 2023
f2bc116
remove redundant stuff
lforst Feb 24, 2023
92b093d
Merge branch 'lforst-event-proxy-server' into lforst-perf-instrument-…
lforst Feb 24, 2023
e02698e
Merge remote-tracking branch 'origin/develop' into lforst-perf-instru…
lforst Feb 24, 2023
3e328bc
Remove console.logs from tests
lforst Feb 24, 2023
98499ec
extract var
lforst Feb 24, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 43 additions & 0 deletions packages/e2e-tests/test-applications/nextjs-13-app-dir/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# See https://help.github.com/articles/ignoring-files/ for more about ignoring files.

# dependencies
/node_modules
/.pnp
.pnp.js

# testing
/coverage

# next.js
/.next/
/out/

# production
/build

# misc
.DS_Store
*.pem

# debug
npm-debug.log*
yarn-debug.log*
yarn-error.log*
.pnpm-debug.log*

# local env files
.env*.local

# vercel
.vercel

# typescript
*.tsbuildinfo
next-env.d.ts

!*.d.ts

# Sentry
.sentryclirc

.vscode
2 changes: 2 additions & 0 deletions packages/e2e-tests/test-applications/nextjs-13-app-dir/.npmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@sentry:registry=http://localhost:4873
@sentry-internal:registry=http://localhost:4873
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export default function Head() {
return (
<>
<title>Create Next App</title>
<meta content="width=device-width, initial-scale=1" name="viewport" />
<meta name="description" content="Generated by create next app" />
<link rel="icon" href="/favicon.ico" />
</>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function RootLayout({ children }: { children: React.ReactNode }) {
return (
<html lang="en">
<body>{children}</body>
</html>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
'use client';

import * as Sentry from '@sentry/nextjs';
import Link from 'next/link';

export default function Home() {
return (
<main>
<input
type="button"
value="Capture Exception"
id="exception-button"
onClick={() => {
Sentry.captureException(new Error('I am a click error!'));
}}
/>
<Link href="/user/5" id="navigation">
navigate
</Link>
</main>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export default async function Home() {
const dynamid = await (await fetch('http://example.com', { cache: 'no-store' })).text(); // do a fetch request so that this server component is always rendered when requested
return <p>I am a blank page :) {dynamid}</p>;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
interface Window {
recordedTransactions?: string[];
capturedExceptionId?: string;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/// <reference types="next" />
/// <reference types="next/image-types/global" />
/// <reference types="next/navigation-types/navigation" />

// NOTE: This file should not be edited
// see https://nextjs.org/docs/basic-features/typescript for more information.
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// This file sets a custom webpack configuration to use your Next.js app
// with Sentry.
// https://nextjs.org/docs/api-reference/next.config.js/introduction
// https://docs.sentry.io/platforms/javascript/guides/nextjs/

const { withSentryConfig } = require('@sentry/nextjs');

const moduleExports = {
experimental: {
appDir: true,
},
};

const sentryWebpackPluginOptions = {
// Additional config options for the Sentry Webpack plugin. Keep in mind that
// the following options are set automatically, and overriding them is not
// recommended:
// release, url, org, project, authToken, configFile, stripPrefix,
// urlPrefix, include, ignore

silent: true, // Suppresses all logs
// For all available options, see:
// https://github.com/getsentry/sentry-webpack-plugin#options.

// We're not testing source map uploads at the moment.
dryRun: true,
};

// Make sure adding Sentry options is the last code to run before exporting, to
// ensure that your source maps include changes from all other Webpack plugins
module.exports = withSentryConfig(moduleExports, sentryWebpackPluginOptions, {
hideSourceMaps: true,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"name": "create-next-app",
"version": "0.1.0",
"private": true,
"scripts": {
"dev": "next dev",
"build": "next build",
"start": "next start",
"lint": "next lint",
"test": "playwright test"
},
"dependencies": {
"@next/font": "13.0.7",
"@sentry/nextjs": "*",
"@types/node": "18.11.17",
"@types/react": "18.0.26",
"@types/react-dom": "18.0.9",
"next": "13.2.1",
"react": "18.2.0",
"react-dom": "18.2.0",
"typescript": "4.9.4"
},
"devDependencies": {
"ts-node": "10.9.1",
"@playwright/test": "^1.27.1"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import type { PlaywrightTestConfig } from '@playwright/test';
import { devices } from '@playwright/test';

/**
* See https://playwright.dev/docs/test-configuration.
*/
const config: PlaywrightTestConfig = {
testDir: './tests',
/* Maximum time one test can run for. */
timeout: 60 * 1000,
expect: {
/**
* Maximum time expect() should wait for the condition to be met.
* For example in `await expect(locator).toHaveText();`
*/
timeout: 5000,
},
/* Run tests in files in parallel */
fullyParallel: true,
/* Fail the build on CI if you accidentally left test.only in the source code. */
forbidOnly: !!process.env.CI,
/* Retry on CI only */
retries: 0,
/* Opt out of parallel tests on CI. */
workers: 1,
/* Reporter to use. See https://playwright.dev/docs/test-reporters */
reporter: 'dot',
/* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */
use: {
/* Maximum time each action such as `click()` can take. Defaults to 0 (no limit). */
actionTimeout: 0,
/* Base URL to use in actions like `await page.goto('/')`. */
baseURL: 'http://localhost:3000',

/* Collect trace when retrying the failed test. See https://playwright.dev/docs/trace-viewer */
trace: 'on-first-retry',
},

/* Configure projects for major browsers */
projects: [
{
name: 'chromium',
use: {
...devices['Desktop Chrome'],
},
},
],

/* Run your local dev server before starting the tests */
webServer: [
{
command: 'yarn start',
port: 3000,
},
{
command: 'yarn ts-node-script start-event-proxy.ts',
port: 27496,
},
],
};

export default config;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import * as Sentry from '@sentry/nextjs';

Sentry.init({
dsn: process.env.NEXT_PUBLIC_E2E_TEST_DSN,
tunnel: 'http://localhost:27496/', // proxy server
tracesSampleRate: 1.0,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import * as Sentry from '@sentry/nextjs';

Sentry.init({
dsn: process.env.NEXT_PUBLIC_E2E_TEST_DSN,
tunnel: 'http://localhost:27496/', // proxy server
tracesSampleRate: 1.0,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import * as Sentry from '@sentry/nextjs';

Sentry.init({
dsn: process.env.NEXT_PUBLIC_E2E_TEST_DSN,
tunnel: 'http://localhost:27496/', // proxy server
Copy link
Member

Choose a reason for hiding this comment

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

l: maybe we can inject the tunnel in via a environmental variable?

tracesSampleRate: 1.0,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { startEventProxyServer, waitForTransaction } from '../../test-utils/event-proxy-server';

startEventProxyServer({
port: 27496,
Copy link
Member

Choose a reason for hiding this comment

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

are these ports random? Should they not be?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's kinda tricky. For Next.js we need to know the port of the proxy server at runtime AND at build time. Playwright (and implicitly the proxy server) are started after the project is built (obviously).

Instead of writing some convoluted logic that picks a port and then passes it everywhere, for now, I just opted to pick some port statically. Of course, this is somewhat error-prone but usually, it is just a "write once and forget" kinda thing.

Copy link
Member

Choose a reason for hiding this comment

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

good enough for me!

proxyServerName: 'nextjs-13-app-dir',
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"$schema": "../../test-recipe-schema.json",
"testApplicationName": "nextjs-13-app-dir",
"buildCommand": "yarn install --pure-lockfile && npx playwright install && yarn build",
"tests": [
{
"testName": "Playwright tests",
"testCommand": "yarn test"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { test, expect } from '@playwright/test';
import { waitForError } from '../../../test-utils/event-proxy-server';
import axios, { AxiosError } from 'axios';

const authToken = process.env.E2E_TEST_AUTH_TOKEN;
const sentryTestOrgSlug = process.env.E2E_TEST_SENTRY_ORG_SLUG;
const sentryTestProject = process.env.E2E_TEST_SENTRY_TEST_PROJECT;
const EVENT_POLLING_TIMEOUT = 30_000;

test('Sends a client-side exception to Sentry', async ({ page }) => {
await page.goto('/');

const errorEventPromise = waitForError('nextjs-13-app-dir', errorEvent => {
return errorEvent?.exception?.values?.[0]?.value === 'I am a click error!';
});

const exceptionButton = page.locator('id=exception-button');
await exceptionButton.click();

const errorEvent = await errorEventPromise;
const exceptionEventId = errorEvent.event_id;

await expect
.poll(
async () => {
try {
const response = await axios.get(
`https://sentry.io/api/0/projects/${sentryTestOrgSlug}/${sentryTestProject}/events/${exceptionEventId}/`,
{ headers: { Authorization: `Bearer ${authToken}` } },
);

return response.status;
} catch (e) {
if (e instanceof AxiosError && e.response) {
if (e.response.status !== 404) {
throw e;
} else {
return e.response.status;
}
} else {
throw e;
}
}
},
{
timeout: EVENT_POLLING_TIMEOUT,
},
)
.toBe(200);
});