Skip to content

Commit

Permalink
Ensure instrumentation register only call once (#67805)
Browse files Browse the repository at this point in the history
### What

Ensure instrumentation.js register only called once

### Why

In #67539 we accidentally make it call twice in both next-dev-ser and
next-server. Adding a condition to ensure the one in next-server is only
called in production runtime.
  • Loading branch information
huozhi authored Jul 16, 2024
1 parent 58166a8 commit f9f5be0
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 11 deletions.
11 changes: 4 additions & 7 deletions packages/next/src/server/dev/next-dev-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,6 @@ export default class DevServer extends Server {
const telemetry = new Telemetry({ distDir: this.distDir })

await super.prepareImpl()
await this.startServerSpan
.traceChild('run-instrumentation-hook')
.traceAsyncFn(() => this.runInstrumentationHookIfAvailable())
await this.matchers.reload()

// Store globals again to preserve changes made by the instrumentation hook.
Expand Down Expand Up @@ -623,10 +620,10 @@ export default class DevServer extends Server {
return instrumentationModule
}

private async runInstrumentationHookIfAvailable() {
if (this.instrumentation) {
await this.instrumentation.register?.()
}
protected async runInstrumentationHookIfAvailable() {
await this.startServerSpan
.traceChild('run-instrumentation-hook')
.traceAsyncFn(() => this.instrumentation?.register?.())
}

protected async ensureEdgeFunction({
Expand Down
9 changes: 5 additions & 4 deletions packages/next/src/server/next-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ export default class NextNodeServer extends BaseServer<
protected async loadInstrumentationModule() {
if (
!this.serverOptions.dev &&
this.nextConfig.experimental.instrumentationHook
!!this.nextConfig.experimental.instrumentationHook
) {
try {
this.instrumentation = await dynamicRequire(
Expand All @@ -332,10 +332,11 @@ export default class NextNodeServer extends BaseServer<

protected async prepareImpl() {
await super.prepareImpl()
await this.runInstrumentationHookIfAvailable()
}

if (this.instrumentation) {
await this.instrumentation.register?.()
}
protected async runInstrumentationHookIfAvailable() {
await this.instrumentation?.register?.()
}

protected loadEnvConfig({
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export function register() {
if (process.env.NEXT_RUNTIME === 'nodejs') {
console.log('register-log')
}
}
5 changes: 5 additions & 0 deletions test/e2e/instrumentation-hook/register-once/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module.exports = {
experimental: {
instrumentationHook: true,
},
}
3 changes: 3 additions & 0 deletions test/e2e/instrumentation-hook/register-once/pages/foo.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return 'foo'
}
17 changes: 17 additions & 0 deletions test/e2e/instrumentation-hook/register-once/register-once.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { nextTestSetup } from 'e2e-utils'

describe('instrumentation-hook - register-once', () => {
const { next, skipped } = nextTestSetup({
files: __dirname,
skipDeployment: true,
})

if (skipped) {
return
}

it('should only register once', async () => {
await next.fetch('/foo')
expect(next.cliOutput).toIncludeRepeated('register-log', 1)
})
})

0 comments on commit f9f5be0

Please sign in to comment.