Include proper instrumentation entry in required files manifest (#68983)

### What When the `edge-instrumentation.js` is not built, we do not include it into the serve required manifest. We moved the write of required server files manifest to a late phase, where we used the edge routes counter to determine if we need to write the edge entry file of instrumentation into manifest x-ref: https://github.com/vercel/next.js/actions/runs/10416595866/job/28850695138
vercel · Aug 19, 2024 · 276fd52 · 276fd52
1 parent b8165af
commit 276fd52
Show file tree

Hide file tree

Showing 5 changed files with 131 additions and 97 deletions.
diff --git a/packages/next/src/build/index.ts b/packages/next/src/build/index.ts
@@ -1236,103 +1236,6 @@ export default async function build(
         PAGES_MANIFEST
       )
 
-      const { cacheHandler } = config
-
-      const requiredServerFilesManifest = nextBuildSpan
-        .traceChild('generate-required-server-files')
-        .traceFn(() => {
-          const serverFilesManifest: RequiredServerFilesManifest = {
-            version: 1,
-            config: {
-              ...config,
-              configFile: undefined,
-              ...(ciEnvironment.hasNextSupport
-                ? {
-                    compress: false,
-                  }
-                : {}),
-              cacheHandler: cacheHandler
-                ? path.relative(distDir, cacheHandler)
-                : config.cacheHandler,
-              experimental: {
-                ...config.experimental,
-                trustHostHeader: ciEnvironment.hasNextSupport,
-
-                // @ts-expect-error internal field TODO: fix this, should use a separate mechanism to pass the info.
-                isExperimentalCompile: isCompileMode,
-              },
-            },
-            appDir: dir,
-            relativeAppDir: path.relative(outputFileTracingRoot, dir),
-            files: [
-              ROUTES_MANIFEST,
-              path.relative(distDir, pagesManifestPath),
-              BUILD_MANIFEST,
-              PRERENDER_MANIFEST,
-              path.join(SERVER_DIRECTORY, MIDDLEWARE_MANIFEST),
-              path.join(SERVER_DIRECTORY, MIDDLEWARE_BUILD_MANIFEST + '.js'),
-              path.join(
-                SERVER_DIRECTORY,
-                MIDDLEWARE_REACT_LOADABLE_MANIFEST + '.js'
-              ),
-              ...(appDir
-                ? [
-                    ...(config.experimental.sri
-                      ? [
-                          path.join(
-                            SERVER_DIRECTORY,
-                            SUBRESOURCE_INTEGRITY_MANIFEST + '.js'
-                          ),
-                          path.join(
-                            SERVER_DIRECTORY,
-                            SUBRESOURCE_INTEGRITY_MANIFEST + '.json'
-                          ),
-                        ]
-                      : []),
-                    path.join(SERVER_DIRECTORY, APP_PATHS_MANIFEST),
-                    path.join(APP_PATH_ROUTES_MANIFEST),
-                    APP_BUILD_MANIFEST,
-                    path.join(
-                      SERVER_DIRECTORY,
-                      SERVER_REFERENCE_MANIFEST + '.js'
-                    ),
-                    path.join(
-                      SERVER_DIRECTORY,
-                      SERVER_REFERENCE_MANIFEST + '.json'
-                    ),
-                  ]
-                : []),
-              REACT_LOADABLE_MANIFEST,
-              config.optimizeFonts
-                ? path.join(
-                    SERVER_DIRECTORY,
-                    AUTOMATIC_FONT_OPTIMIZATION_MANIFEST
-                  )
-                : null,
-              BUILD_ID_FILE,
-              path.join(SERVER_DIRECTORY, NEXT_FONT_MANIFEST + '.js'),
-              path.join(SERVER_DIRECTORY, NEXT_FONT_MANIFEST + '.json'),
-              ...(hasInstrumentationHook
-                ? [
-                    path.join(
-                      SERVER_DIRECTORY,
-                      `${INSTRUMENTATION_HOOK_FILENAME}.js`
-                    ),
-                    path.join(
-                      SERVER_DIRECTORY,
-                      `edge-${INSTRUMENTATION_HOOK_FILENAME}.js`
-                    ),
-                  ]
-                : []),
-            ]
-              .filter(nonNullable)
-              .map((file) => path.join(config.distDir, file)),
-            ignore: [] as string[],
-          }
-
-          return serverFilesManifest
-        })
-
       async function turbopackBuild(): Promise<{
         duration: number
         buildTraceContext: undefined
@@ -2352,6 +2255,108 @@ export default async function build(
         )
       }
 
+      const { cacheHandler } = config
+
+      const instrumentationHookEntryFiles: string[] = []
+      if (hasInstrumentationHook) {
+        instrumentationHookEntryFiles.push(
+          path.join(SERVER_DIRECTORY, `${INSTRUMENTATION_HOOK_FILENAME}.js`)
+        )
+        // If there's edge routes, append the edge instrumentation hook
+        if (edgeRuntimeAppCount || edgeRuntimePagesCount) {
+          instrumentationHookEntryFiles.push(
+            path.join(
+              SERVER_DIRECTORY,
+              `edge-${INSTRUMENTATION_HOOK_FILENAME}.js`
+            )
+          )
+        }
+      }
+
+      const requiredServerFilesManifest = nextBuildSpan
+        .traceChild('generate-required-server-files')
+        .traceFn(() => {
+          const serverFilesManifest: RequiredServerFilesManifest = {
+            version: 1,
+            config: {
+              ...config,
+              configFile: undefined,
+              ...(ciEnvironment.hasNextSupport
+                ? {
+                    compress: false,
+                  }
+                : {}),
+              cacheHandler: cacheHandler
+                ? path.relative(distDir, cacheHandler)
+                : config.cacheHandler,
+              experimental: {
+                ...config.experimental,
+                trustHostHeader: ciEnvironment.hasNextSupport,
+
+                // @ts-expect-error internal field TODO: fix this, should use a separate mechanism to pass the info.
+                isExperimentalCompile: isCompileMode,
+              },
+            },
+            appDir: dir,
+            relativeAppDir: path.relative(outputFileTracingRoot, dir),
+            files: [
+              ROUTES_MANIFEST,
+              path.relative(distDir, pagesManifestPath),
+              BUILD_MANIFEST,
+              PRERENDER_MANIFEST,
+              path.join(SERVER_DIRECTORY, MIDDLEWARE_MANIFEST),
+              path.join(SERVER_DIRECTORY, MIDDLEWARE_BUILD_MANIFEST + '.js'),
+              path.join(
+                SERVER_DIRECTORY,
+                MIDDLEWARE_REACT_LOADABLE_MANIFEST + '.js'
+              ),
+              ...(appDir
+                ? [
+                    ...(config.experimental.sri
+                      ? [
+                          path.join(
+                            SERVER_DIRECTORY,
+                            SUBRESOURCE_INTEGRITY_MANIFEST + '.js'
+                          ),
+                          path.join(
+                            SERVER_DIRECTORY,
+                            SUBRESOURCE_INTEGRITY_MANIFEST + '.json'
+                          ),
+                        ]
+                      : []),
+                    path.join(SERVER_DIRECTORY, APP_PATHS_MANIFEST),
+                    path.join(APP_PATH_ROUTES_MANIFEST),
+                    APP_BUILD_MANIFEST,
+                    path.join(
+                      SERVER_DIRECTORY,
+                      SERVER_REFERENCE_MANIFEST + '.js'
+                    ),
+                    path.join(
+                      SERVER_DIRECTORY,
+                      SERVER_REFERENCE_MANIFEST + '.json'
+                    ),
+                  ]
+                : []),
+              REACT_LOADABLE_MANIFEST,
+              config.optimizeFonts
+                ? path.join(
+                    SERVER_DIRECTORY,
+                    AUTOMATIC_FONT_OPTIMIZATION_MANIFEST
+                  )
+                : null,
+              BUILD_ID_FILE,
+              path.join(SERVER_DIRECTORY, NEXT_FONT_MANIFEST + '.js'),
+              path.join(SERVER_DIRECTORY, NEXT_FONT_MANIFEST + '.json'),
+              ...instrumentationHookEntryFiles,
+            ]
+              .filter(nonNullable)
+              .map((file) => path.join(config.distDir, file)),
+            ignore: [] as string[],
+          }
+
+          return serverFilesManifest
+        })
+
       if (!hasSsrAmpPages) {
         requiredServerFilesManifest.ignore.push(
           path.relative(

diff --git a/test/production/instrumentation/required-files-instrumentation-entry/app/node/route.js b/test/production/instrumentation/required-files-instrumentation-entry/app/node/route.js
@@ -0,0 +1,3 @@
+export function GET() {
+  return new Response('node')
+}
diff --git a/test/production/instrumentation/required-files-instrumentation-entry/instrumentation.js b/test/production/instrumentation/required-files-instrumentation-entry/instrumentation.js
@@ -0,0 +1 @@
+export function register() {}
diff --git a/test/production/instrumentation/required-files-instrumentation-entry/next.config.js b/test/production/instrumentation/required-files-instrumentation-entry/next.config.js
@@ -0,0 +1,6 @@
+/** @type {import('next').NextConfig} */
+module.exports = {
+  experimental: {
+    instrumentationHook: true,
+  },
+}
diff --git a/...ntation/required-files-instrumentation-entry/required-files-instrumentation-entry.test.ts b/...ntation/required-files-instrumentation-entry/required-files-instrumentation-entry.test.ts
@@ -0,0 +1,19 @@
+import { nextTestSetup } from 'e2e-utils'
+
+async function readRequiredFilesManifest(next: any) {
+  const manifest = JSON.parse(
+    await next.readFile('.next/required-server-files.json')
+  )
+  return manifest.files
+}
+
+describe('instrumentation - required-files-instrumentation-entry', () => {
+  const { next } = nextTestSetup({
+    files: __dirname,
+  })
+
+  it('should not contain edge entry in required files manifest', async () => {
+    const requiredFiles = await readRequiredFilesManifest(next)
+    expect(requiredFiles).not.toContain('.next/server/edge-instrumentation.js')
+  })
+})