Skip to content

Commit

Permalink
Fix instrumentation with only pages (#62622)
Browse files Browse the repository at this point in the history
This ensures we properly transpile `instrumentation.ts` when only
`pages` is being used as previously we were relying on the `app`
specific loaders which aren't configured when an `app` directory isn't
present. We regressed on this in
#60984 as it was working as
expected prior to this commit

x-ref: [slack
thread](https://vercel.slack.com/archives/C011GK1JUBA/p1709075846401649?thread_ts=1706643408.233909&cid=C011GK1JUBA)

Closes NEXT-2632
  • Loading branch information
ijjk authored Feb 28, 2024
1 parent 9408f35 commit eaa7606
Show file tree
Hide file tree
Showing 10 changed files with 117 additions and 20 deletions.
11 changes: 10 additions & 1 deletion packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,15 @@ export default async function getBaseWebpackConfig(
].filter(Boolean)
: []

const instrumentLayerLoaders = [
// When using Babel, we will have to add the SWC loader
// as an additional pass to handle RSC correctly.
// This will cause some performance overhead but
// acceptable as Babel will not be recommended.
swcServerLayerLoader,
babelLoader,
].filter(Boolean)

const middlewareLayerLoaders = [
// When using Babel, we will have to use SWC to do the optimization
// for middleware to tree shake the unused default optimized imports like "next/server".
Expand Down Expand Up @@ -1478,7 +1487,7 @@ export default async function getBaseWebpackConfig(
{
test: codeCondition.test,
issuerLayer: WEBPACK_LAYERS.instrument,
use: appServerLayerLoaders,
use: instrumentLayerLoaders,
},
...(hasAppDir
? [
Expand Down
5 changes: 0 additions & 5 deletions test/e2e/multi-zone/app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,6 @@
"name": "with-zones",
"version": "1.0.0",
"private": true,
"scripts": {
"dev": "node server.js",
"build": "yarn next build apps/host && yarn next build apps/guest",
"start": "NODE_ENV=production node server.js"
},
"dependencies": {
"@types/react": "18.0.28",
"next": "canary",
Expand Down
4 changes: 4 additions & 0 deletions test/e2e/multi-zone/multi-zone.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,13 @@ createNextDescribe(
startCommand: (global as any).isNextDev ? 'pnpm dev' : 'pnpm start',
packageJson: {
scripts: {
dev: 'node server.js',
build: 'yarn next build apps/host && yarn next build apps/guest',
start: 'NODE_ENV=production node server.js',
'post-build': 'echo done',
},
},
dependencies: require('./app/package.json').dependencies,
},
({ next, isNextDev }) => {
it.each([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ export async function generateMetadata() {
}

export default async function Page() {
const data = await fetch('https://vercel.com')
const data = await fetch('https://example.vercel.sh')
return <pre>RESONSE: {data.status}</pre>
}
9 changes: 7 additions & 2 deletions test/e2e/opentelemetry/app/app/[param]/rsc-fetch/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ export async function generateMetadata() {
}

export default async function Page() {
const data = await fetch('https://vercel.com')
return <pre>{await data.text()}</pre>
const data = await fetch('https://example.vercel.sh')
return (
<>
<p>Page</p>
<pre>{await data.text()}</pre>
</>
)
}
3 changes: 3 additions & 0 deletions test/e2e/opentelemetry/instrumentation-minimal.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export async function register() {
console.log('instrumentation log')
}
67 changes: 67 additions & 0 deletions test/e2e/opentelemetry/instrumentation-pages-app-only.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { nextTestSetup } from 'e2e-utils'
import { retry } from 'next-test-utils'

for (const { app, src, pathname, text } of [
{
pages: true,
src: false,
pathname: '/pages/param/getServerSideProps',
text: 'Page',
},
{
pages: true,
src: true,
pathname: '/pages/param/getServerSideProps',
text: 'Page',
},
{
app: true,
src: false,
pathname: '/app/param/rsc-fetch',
text: 'Page',
},
{
app: true,
src: true,
pathname: '/app/param/rsc-fetch',
text: 'Page',
},
]) {
describe(`instrumentation ${app ? 'app' : 'pages'}${
src ? ' src/' : ''
}`, () => {
const curDir = app ? 'app' : 'pages'
const oppositeDir = app ? 'pages' : 'app'

const { next } = nextTestSetup({
files: __dirname,
env: {
NEXT_PUBLIC_SIMPLE_INSTRUMENT: '1',
},
skipDeployment: true,
packageJson: {
scripts: {
'setup-dir': `mv instrumentation-minimal.ts instrumentation.ts; rm -rf ${oppositeDir}${
src
? `; mkdir src; mv ${curDir} src/; mv instrumentation.ts src/`
: ''
}`,
dev: 'pnpm setup-dir && next dev',
build: 'pnpm setup-dir && next build',
start: 'next start',
},
},
startCommand: `pnpm ${(global as any).isNextDev ? 'dev' : 'start'}`,
buildCommand: `pnpm build`,
dependencies: require('./package.json').dependencies,
})

it('should start and serve correctly', async () => {
const html = await next.render(pathname)
expect(html).toContain(text)
retry(() => {
expect(next.cliOutput).toContain('instrumentation log')
})
})
})
}
4 changes: 2 additions & 2 deletions test/e2e/opentelemetry/instrumentation.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { register as registerForTest } from './instrumentation-test'

export async function register() {
const { register: registerForTest } = await import('./instrumentation-test')

if (process.env.__NEXT_TEST_MODE) {
registerForTest()
} else if (process.env.NEXT_RUNTIME === 'nodejs') {
Expand Down
18 changes: 10 additions & 8 deletions test/e2e/opentelemetry/opentelemetry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,13 @@ createNextDescribe(
],
},
{
name: 'fetch GET https://vercel.com/',
name: 'fetch GET https://example.vercel.sh/',
attributes: {
'http.method': 'GET',
'http.url': 'https://vercel.com/',
'net.peer.name': 'vercel.com',
'next.span_name': 'fetch GET https://vercel.com/',
'http.url': 'https://example.vercel.sh/',
'net.peer.name': 'example.vercel.sh',
'next.span_name':
'fetch GET https://example.vercel.sh/',
'next.span_type': 'AppRender.fetch',
},
kind: 2,
Expand Down Expand Up @@ -265,14 +266,15 @@ createNextDescribe(
],
},
{
name: 'fetch GET https://vercel.com/',
name: 'fetch GET https://example.vercel.sh/',
kind: 2,
attributes: {
'next.span_name': 'fetch GET https://vercel.com/',
'next.span_name':
'fetch GET https://example.vercel.sh/',
'next.span_type': 'AppRender.fetch',
'http.url': 'https://vercel.com/',
'http.url': 'https://example.vercel.sh/',
'http.method': 'GET',
'net.peer.name': 'vercel.com',
'net.peer.name': 'example.vercel.sh',
},
status: { code: 0 },
},
Expand Down
14 changes: 13 additions & 1 deletion test/lib/next-modes/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ export class NextInstance {
this.env = {}
Object.assign(this, opts)

require('console').log('packageJson??', this.packageJson)

if (!(global as any).isNextDeploy) {
this.env = {
...this.env,
Expand Down Expand Up @@ -102,7 +104,17 @@ export class NextInstance {
)
}

await fs.cp(files.fsPath, this.testDir, { recursive: true })
await fs.cp(files.fsPath, this.testDir, {
recursive: true,
filter(source) {
// we don't copy a package.json as it's manually written
// via the createNextInstall process
if (path.relative(files.fsPath, source) === 'package.json') {
return false
}
return true
},
})
} else {
for (const filename of Object.keys(files)) {
const item = files[filename]
Expand Down

0 comments on commit eaa7606

Please sign in to comment.