Skip to content

fix(instrumentation): re-trigger require hooks for builtins after reg…#96047

Open
tabrejansary wants to merge 1 commit into
vercel:canaryfrom
tabrejansary:fix/otel-builtin-instrumentation-95894
Open

fix(instrumentation): re-trigger require hooks for builtins after reg…#96047
tabrejansary wants to merge 1 commit into
vercel:canaryfrom
tabrejansary:fix/otel-builtin-instrumentation-95894

Conversation

@tabrejansary

Copy link
Copy Markdown

…ister (#95894)

Comment thread .husky/pre-commit
@@ -1 +1,2 @@
export PATH="$PATH:/c/Users/Tabrej ansari/AppData/Roaming/npm"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
export PATH="$PATH:/c/Users/Tabrej ansari/AppData/Roaming/npm"

A machine-specific export PATH line hardcoding the author's Windows user directory was committed to the shared .husky/pre-commit hook.

Fix on Vercel

// Touch the built-in modules most commonly instrumented by OTel and similar
// libraries. getBuiltinModule() is synchronous and has no side effects beyond
// triggering pending require hooks.
const builtins = ['http', 'https', 'net', 'dns'] as const

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

touchBuiltinModulesForInstrumentation() uses process.getBuiltinModule(), which bypasses Module._load and therefore never triggers require-in-the-middle hooks, making the OpenTelemetry re-patching a no-op.

Fix on Vercel

Comment thread .husky/pre-commit
@@ -1 +1,2 @@
export PATH="$PATH:/c/Users/Tabrej ansari/AppData/Roaming/npm"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this looks like it got committed by accident — a hardcoded local npm path with your username. drop it from the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants