feat(deps): Bump OpenTelemetry instrumentations#18934
Conversation
635ce99 to
6c98baf
Compare
size-limit report 📦
|
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
…cess.arg…" This reverts commit fae3a77.
336c4f0 to
77d761a
Compare
| replace({ | ||
| preventAssignment: true, | ||
| values: { | ||
| 'process.argv0': JSON.stringify(''), // needed because otel relies on process.argv0 for the default service name, but that api is not available in the edge runtime. | ||
| }, | ||
| }), |
There was a problem hiding this comment.
Bug: The removal of custom bundling logic for OpenTelemetry dependencies will cause a runtime error in the Vercel Edge environment because process.argv0 references will no longer be replaced.
Severity: HIGH
Suggested Fix
Restore the custom external function in the Rollup configuration to ensure @opentelemetry/resources and @opentelemetry/sdk-trace-base are explicitly bundled, allowing the replace plugin to correctly substitute Node.js-specific code.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/vercel-edge/rollup.npm.config.mjs#L16-L21
Potential issue: The pull request removes custom Rollup configuration that was
responsible for bundling specific OpenTelemetry packages (`@opentelemetry/resources`,
`@opentelemetry/sdk-trace-base`) into the Vercel Edge build. The new default
configuration marks these packages as external. As a result, the `replace` plugin, which
substitutes Node.js-specific references like `process.argv0` with an empty string, will
no longer be applied to their code. When this code executes in the Vercel Edge runtime,
which lacks the `process` global, it will attempt to access `process.argv0` and trigger
a runtime error, likely during initialization.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
We stopped bundling in the package so this isn't applying here. The issue was fixed upstream in Otel.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| ], | ||
| }, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Removing bundling workaround may break Edge runtime
Medium Severity
The custom external handling that forced @opentelemetry/resources to be bundled was removed. Previously, this ensured the process.argv0 replacement applied to OTEL packages (since process.argv0 doesn't exist in Edge runtime). Now @opentelemetry/resources (a dependency) will be external and not bundled, so any process.argv0 usage within it won't be replaced. The test file build-artifacts.test.ts that verified process.argv0 doesn't appear in the build output was also deleted, removing the safety net for this Edge compatibility requirement.
|
Note: This PR also reverts #18759 because the issue was fixed upstream. |
Closes #18958 (added automatically)