-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Guide towards v9 for ESM OTEL setups #11974
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
Bundle ReportChanges will decrease total bundle size by 15 bytes (-0.0%) ⬇️. This is within the configured threshold ✅ Detailed changes
|
|
||
The Sentry SDK will automatically register ESM loader hooks by default. | ||
However, if you have your own OpenTelemetry setup, it is recommended to configure the Sentry SDK to not register these hooks and instead register them yourself. | ||
You can do so by setting `registerEsmLoaderHooks` to `false` and [set up ESM loader hooks](https://github.com/open-telemetry/opentelemetry-js/blob/main/doc/esm-support.md#instrumentation-hook-required-for-esm): |
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.
I think it might be preferable to recommend the Sentry hook here that wraps the otel hook. That would leave us some options in the future if we ever need to add any extra logic.
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.
Very valid, however, I don't think it buys us anything. We already cannot be sure that they use the Sentry hooks, therefore we can't make any decisions based on that.
Given that it doesn't really matter I'd rather refer to the OTEL docs - also because the target audience we assume in this paragraph is people that already have a complete OTEL setup.
docs/platforms/javascript/common/opentelemetry/custom-setup.mdx
Outdated
Show resolved
Hide resolved
docs/platforms/javascript/common/opentelemetry/custom-setup.mdx
Outdated
Show resolved
Hide resolved
docs/platforms/javascript/common/opentelemetry/using-opentelemetry-apis.mdx
Outdated
Show resolved
Hide resolved
docs/platforms/javascript/common/opentelemetry/using-opentelemetry-apis.mdx
Outdated
Show resolved
Hide resolved
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.
Added some comments, looks good! Thanks for updating! 🕺🦃
Co-authored-by: Alex Krawiec <[email protected]>
Co-authored-by: Alex Krawiec <[email protected]>
DESCRIBE YOUR PR
Ref: getsentry/sentry-javascript#14483
addOpenTelemetryInstrumentation()
.IS YOUR CHANGE URGENT?
Help us prioritize incoming PRs by letting us know when the change needs to go live.