Add support for async instrumentation register (#48575)

docs/advanced-features/instrumentation.md

@@ -23,30 +23,30 @@ export function register() {
 }
 ```
 
-However, we recommend importing files with side effects using `require` from within your `register` function instead. The following example demonstrates a basic usage of `require` in a `register` function:
+However, we recommend importing files with side effects using `import` from within your `register` function instead. The following example demonstrates a basic usage of `import` in a `register` function:
 
 ```ts
 // /instrumentation.ts
 
-export function register() {
-  require('package-with-side-effect')
+export async function register() {
+  await import('package-with-side-effect')
 }
 ```
 
 By doing this, you can colocate all of your side effects in one place in your code, and avoid any unintended consequences from importing files.
 
-We call `register` in all environments, so it's necessary to conditionally require any code that doesn't support both `edge` and `nodejs`. You can use the environment variable `NEXT_RUNTIME` to get the current environment. Importing an environment-specific code would look like this:
+We call `register` in all environments, so it's necessary to conditionally import any code that doesn't support both `edge` and `nodejs`. You can use the environment variable `NEXT_RUNTIME` to get the current environment. Importing an environment-specific code would look like this:
 
 ```ts
 // /instrumentation.ts
 
-export function register() {
+export async function register() {
   if (process.env.NEXT_RUNTIME === 'nodejs') {
-    require('./instrumentation-node')
+    await import('./instrumentation-node')
   }
 
   if (process.env.NEXT_RUNTIME === 'edge') {
-    require('./instrumentation-edge')
+    await import('./instrumentation-edge')
   }
 }
 ```

docs/advanced-features/open-telemetry.md

@@ -60,13 +60,13 @@ npm install @opentelemetry/sdk-node @opentelemetry/resources @opentelemetry/sema
 ```
 
 Now you can initialize `NodeSDK` in your `instrumentation.ts`.
-OpenTelemetry APIs are not compatible with edge runtime, so you need to make sure that you are importing them only when `process.env.NEXT_RUNTIME === 'nodejs'`. Conditionally importing with an `require` doesn't play well with typescript. We recommend using a conditionally `require`ing new file `instrumentation.node.ts` which can use normal `import`s:
+OpenTelemetry APIs are not compatible with edge runtime, so you need to make sure that you are importing them only when `process.env.NEXT_RUNTIME === 'nodejs'`. We recommend creating a new file `instrumentation.node.ts` which you conditionally import only when using node:
 
 ```ts
 // instrumentation.ts
-export function register() {
+export async function register() {
   if (process.env.NEXT_RUNTIME === 'nodejs') {
-    require('./instrumentation.node.ts')
+    await import('./instrumentation.node.ts')
   }
 }
 ```
@@ -90,7 +90,7 @@ sdk.start()
 ```
 
 Doing this is equivalent to using `@vercel/otel`, but it's possible to modify and extend.
-For example, you could use `@opentelemetry/exporter-trace-otlp-grpc` instead of `@opentelemetry/exporter-trace-otlp-http`.
+For example, you could use `@opentelemetry/exporter-trace-otlp-grpc` instead of `@opentelemetry/exporter-trace-otlp-http` or you can specify more resource attributes.
 
 ## Testing your instrumentation
 

packages/next/src/server/dev/next-dev-server.ts

@@ -21,7 +21,10 @@ import findUp from 'next/dist/compiled/find-up'
 import { join as pathJoin, relative, resolve as pathResolve, sep } from 'path'
 import Watchpack from 'next/dist/compiled/watchpack'
 import { ampValidation } from '../../build/output'
-import { PUBLIC_DIR_MIDDLEWARE_CONFLICT } from '../../lib/constants'
+import {
+  INSTRUMENTATION_HOOK_FILENAME,
+  PUBLIC_DIR_MIDDLEWARE_CONFLICT,
+} from '../../lib/constants'
 import { fileExists } from '../../lib/file-exists'
 import { findPagesDir } from '../../lib/find-pages-dir'
 import loadCustomRoutes from '../../lib/load-custom-routes'
@@ -1496,9 +1499,9 @@ export default class DevServer extends Server {
         const instrumentationHook = await require(pathJoin(
           this.distDir,
           'server',
-          'instrumentation'
+          INSTRUMENTATION_HOOK_FILENAME
         ))
-        instrumentationHook.register()
+        await instrumentationHook.register()
       } catch (err: any) {
         err.message = `An error occurred while loading instrumentation hook: ${err.message}`
         throw err

packages/next/src/server/next-server.ts

@@ -340,8 +340,7 @@ export default class NextNodeServer extends BaseServer {
           'server',
           INSTRUMENTATION_HOOK_FILENAME
         ))
-
-        instrumentationHook.register?.()
+        await instrumentationHook.register?.()
       } catch (err: any) {
         if (err.code !== 'MODULE_NOT_FOUND') {
           err.message = `An error occurred while loading instrumentation hook: ${err.message}`

packages/next/src/server/web/adapter.ts

@@ -59,9 +59,34 @@ export type AdapterOptions = {
   IncrementalCache?: typeof import('../lib/incremental-cache').IncrementalCache
 }
 
+async function registerInstrumentation() {
+  if (
+    '_ENTRIES' in globalThis &&
+    _ENTRIES.middleware_instrumentation &&
+    _ENTRIES.middleware_instrumentation.register
+  ) {
+    try {
+      await _ENTRIES.middleware_instrumentation.register()
+    } catch (err: any) {
+      err.message = `An error occurred while loading instrumentation hook: ${err.message}`
+      throw err
+    }
+  }
+}
+
+let registerInstrumentationPromise: Promise<void> | null = null
+function ensureInstrumentationRegistered() {
+  if (!registerInstrumentationPromise) {
+    registerInstrumentationPromise = registerInstrumentation()
+  }
+  return registerInstrumentationPromise
+}
+
 export async function adapter(
   params: AdapterOptions
 ): Promise<FetchEventResult> {
+  await ensureInstrumentationRegistered()
+
   // TODO-APP: use explicit marker for this
   const isEdgeRendering = typeof self.__BUILD_MANIFEST !== 'undefined'
 
@@ -334,16 +359,6 @@ export function enhanceGlobals() {
     configurable: false,
   })
 
-  if (
-    '_ENTRIES' in globalThis &&
-    _ENTRIES.middleware_instrumentation &&
-    _ENTRIES.middleware_instrumentation.register
-  ) {
-    try {
-      _ENTRIES.middleware_instrumentation.register()
-    } catch (err: any) {
-      err.message = `An error occurred while loading instrumentation hook: ${err.message}`
-      throw err
-    }
-  }
+  // Eagerly fire instrumentation hook to make the startup faster.
+  void ensureInstrumentationRegistered()
 }

test/e2e/instrumentation-hook/instrumentation-hook.test.ts

@@ -82,6 +82,20 @@ describe('Instrumentation Hook', () => {
     })
   })
 
+  describeCase('with-async-node-page', ({ next }) => {
+    it('with-async-node-page should run the instrumentation hook', async () => {
+      const page = await next.render('/')
+      expect(page).toContain('Node - finished: true')
+    })
+  })
+
+  describeCase('with-async-edge-page', ({ next }) => {
+    it('with-async-edge-page should run the instrumentation hook', async () => {
+      const page = await next.render('/')
+      expect(page).toContain('Edge - finished: true')
+    })
+  })
+
   describeCase('general', ({ next, isNextDev }) => {
     it('should not overlap with a instrumentation page', async () => {
       const page = await next.render('/instrumentation')

test/e2e/instrumentation-hook/with-async-edge-page/instrumentation.edge.js

@@ -0,0 +1,8 @@
+/*global globalThis*/
+
+const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms))
+
+export async function register() {
+  await sleep(1000)
+  globalThis.instrumentationFinished = true
+}

test/e2e/instrumentation-hook/instrumentation.js → test/e2e/instrumentation-hook/with-async-edge-page/instrumentation.js

@@ -1,6 +1,8 @@
-export function register() {
+export async function register() {
   if (process.env.NEXT_RUNTIME === 'edge') {
     console.log('instrumentation hook on the edge')
+    const { register } = await import('./instrumentation.edge')
+    await register()
   } else if (process.env.NEXT_RUNTIME === 'nodejs') {
     console.log('instrumentation hook on nodejs')
   } else {

test/e2e/instrumentation-hook/with-async-edge-page/pages/index.tsx

@@ -0,0 +1,15 @@
+export default function Page({ finished }) {
+  return <p>{`Edge - finished: ${finished}`}</p>
+}
+
+export function getServerSideProps() {
+  return {
+    props: {
+      finished: Boolean(globalThis.instrumentationFinished),
+    },
+  }
+}
+
+export const config = {
+  runtime: 'experimental-edge',
+}

test/e2e/instrumentation-hook/with-async-node-page/instrumentation.js

@@ -0,0 +1,11 @@
+export async function register() {
+  if (process.env.NEXT_RUNTIME === 'edge') {
+    console.log('instrumentation hook on the edge')
+  } else if (process.env.NEXT_RUNTIME === 'nodejs') {
+    console.log('instrumentation hook on nodejs')
+    const { register } = await import('./instrumentation.node')
+    await register()
+  } else {
+    await require('this should fail')
+  }
+}

test/e2e/instrumentation-hook/with-async-node-page/instrumentation.node.js

@@ -0,0 +1,8 @@
+/*global globalThis*/
+
+const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms))
+
+export async function register() {
+  await sleep(1000)
+  globalThis.instrumentationFinished = true
+}

test/e2e/instrumentation-hook/with-async-node-page/pages/index.tsx

@@ -0,0 +1,11 @@
+export default function Page({ finished }) {
+  return <p>{`Node - finished: ${finished}`}</p>
+}
+
+export async function getServerSideProps() {
+  return {
+    props: {
+      finished: Boolean(globalThis.instrumentationFinished),
+    },
+  }
+}