Apply outputFileTracingIncludes to the instrumentation entry#92616
Draft
yogeshwaran-c wants to merge 1 commit intovercel:canaryfrom
Draft
Apply outputFileTracingIncludes to the instrumentation entry#92616yogeshwaran-c wants to merge 1 commit intovercel:canaryfrom
yogeshwaran-c wants to merge 1 commit intovercel:canaryfrom
Conversation
The instrumentation hook is registered as a webpack entry whose name is
the bare string `instrumentation` (no `app/` or `pages/` prefix). The
post-trace include/exclude pass in `collect-build-traces.ts` only
normalized `app/*` and `pages/*` entries into a leading-slash route key,
so a user-supplied `outputFileTracingIncludes: { '/instrumentation': [...] }`
silently never matched the instrumentation entry. The result was that
extra files (e.g. native `.node` binaries pulled in by an OpenTelemetry
or similar instrumentation package) were missing from
`.next/server/instrumentation.js.nft.json` and therefore from the
standalone output, even though the user had configured them.
Treat `entryName === 'instrumentation'` as a special case and normalize
its route to `/instrumentation`, so the same `/`-prefixed convention
users already use for app and pages routes works for instrumentation
too.
The behavior is webpack-only today; Turbopack does not populate
`entryNameFilesMap` and therefore skips this post-trace pass entirely.
The new test is gated on `IS_TURBOPACK_TEST`, mirroring the existing
`build-trace-extra-entries` suite.
Fixes vercel#68740
Collaborator
|
Allow CI Workflow Run
Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
Make
outputFileTracingIncludes: { '/instrumentation': [...] }actually take effect for the instrumentation hook entry. Today the include globs are silently dropped, so extra files (native.nodeprebuilds, support assets, etc.) pulled in viainstrumentation.tsgo missing from.next/server/instrumentation.js.nft.jsonand from the standalone output, even when the user has configured them.Why?
The post-trace include/exclude pass in
collect-build-traces.tsderives aroutekey from the webpack entry name. Forapp/*andpages/*entries it normalizes to a leading-slash route (/foo), and the user'soutputFileTracingIncludesglob keys are matched against that. But the instrumentation hook is registered as a top-level webpack entry whose name is the bare stringinstrumentation— noapp/orpages/prefix — so neither branch ran,routestayed as'instrumentation', and a key like'/instrumentation'(which is what users naturally write to match the convention) never matched.The result is exactly the user's report in #68740: an instrumentation package such as
@splunk/otelships a native.nodebinary that nft can't statically resolve, and there's no working knob to add it back.outputFileTracingIncludeslooks like the right tool but is a no-op for this entry.How?
Treat
entryName === 'instrumentation'as a special case in the post-trace pass and normalize its route to/instrumentation, mirroring howapp/*andpages/*entries are normalized. After this,outputFileTracingIncludes: { '/instrumentation': ['./node_modules/@splunk/otel/prebuilds/**/*'] }works the same way'/app/route1': [...]does for an app router route, and the matching files land ininstrumentation.js.nft.jsonand the standalone output.The change is intentionally minimal — one new branch in one function — and does not touch the trace generation, the standalone copy step, or the existing
app//pages/paths.Scope / known limitation
This post-trace pass is webpack-only. Turbopack does not populate
chunksTrace.entryNameFilesMap, so the entireoutputFileTracingIncludes/outputFileTracingExcludespost-pass is already a no-op under Turbopack today (the existingbuild-trace-extra-entriesintegration test isdescribe.skip'd for Turbopack for the same reason). The fix here brings instrumentation to feature parity with howapp/*andpages/*entries already behave under webpack. Turbopack parity foroutputFileTracingIncludesis a larger, separate piece of work and is not in scope.Test plan
Added
test/integration/build-trace-extra-entries-instrumentation/:instrumentation.js, aninclude-me/native-binary.nodestub, andnext.config.jssettingoutputFileTracingIncludes: { '/instrumentation': ['./include-me/**/*'] }.nextBuildand asserts thatinclude-me/native-binary.nodeis present in.next/server/instrumentation.js.nft.json.IS_TURBOPACK_TEST, matching the existingbuild-trace-extra-entriessuite.Verified locally on Windows / webpack:
Expected: true / Received: false) and passes after the fix.build-trace-extra-entriestest was checked and is unaffected by this change (it fails identically on canary and on this branch — unrelated to the fix; it assertsapp/route1includes that aren't landing on this Windows env).Fixes #68740