-
Notifications
You must be signed in to change notification settings - Fork 0
feat: update otel setup #94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the OpenTelemetry setup to avoid critical dependency warnings in Next.js builds by replacing @opentelemetry/sdk-node
with a direct NodeTracerProvider
implementation.
- Removes
@opentelemetry/sdk-node
dependency from all packages and examples - Refactors instrumentation setup to use
NodeTracerProvider
directly instead ofNodeSDK
- Updates documentation to reflect the new OpenTelemetry setup approach
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
packages/ai/package.json | Removes sdk-node dependency |
examples/example-instrumentation-nextjs-v5/src/instrumentation.node.ts | Refactors from NodeSDK to NodeTracerProvider setup |
examples/example-instrumentation-nextjs-v5/package.json | Removes sdk-node dependency |
examples/example-instrumentation-nextjs-v5/README.md | Updates documentation reference from NodeSDK to OpenTelemetry tracer |
examples/example-instrumentation-nextjs-v4/src/instrumentation.node.ts | Refactors from NodeSDK to NodeTracerProvider setup |
examples/example-instrumentation-nextjs-v4/package.json | Removes sdk-node and require-in-the-middle dependencies |
examples/example-instrumentation-nextjs-v4/README.md | Updates documentation reference from NodeSDK to OpenTelemetry tracer |
examples/example-instrumentation-express/README.md | Updates documentation reference from NodeSDK to OpenTelemetry tracer |
examples/example-evals-nextjs/src/instrumentation.node.ts | Refactors from NodeSDK to NodeTracerProvider setup |
examples/example-evals-nextjs/package.json | Removes sdk-node dependency |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
* remove NodeSDK from instrumentation setups * uninstall NodeSDK * update docs * better tools
Background
We got feedback from some users that in some frameworks such as Next.js they get scary-looking messages during build:
Critical dependency: require function is used in a way in which dependencies cannot be statically extracted
This is caused by
@opentelemetry/sdk-node
.The mitigation suggested here does not work unfortunately as
sdk-node
is built different from libraries like sentryThis leaves two possible mitigations:
// see the changes made in this PR
Changes in this PR
Move everything to version 2
Trace (before)
Trace (after)