-
Notifications
You must be signed in to change notification settings - Fork 825
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
Should use 'globalThis' instead of 'window' for the browser usage #3991
Comments
I think it would make sense to update the use sites of |
These days, all browsers support 'globalThis' nativelly. I believe that the 'globalThis' defined in https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-core/src/platform/browser/globalThis.ts was added in order to offer some backwards compatibility with legacy browsers. |
This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days. |
Hi guys, any update on this? I am currently being forced to keep a patched version in order to be able to use it from a web worker. Thank you |
@cristianmadularu, no movement right now; marking this up-for-grabs in case someone wants to pick it up. |
@pichlermarc I can fix it. Could you please give me access so I can push a fix? (2 lines of code) |
Thanks! The usual flow for this is to open a PR from a fork, which should be possible for every GitHub user. |
OK I created a PR :) Thanks! |
Thanks for taking care of this. It's not workng automatically but I took the liberty to add the entry, I'll merge the PR momentarily. 🙂 |
Oh great, thank you! :) |
Hi! 👋
Firstly, thanks for your work on this project! 🙂
I noticed that when running in browser, I was getting errors when trying to log from a web worker. Upon debugging, I realized that the OTLPExporterBrowserBase is referencing the 'window' instead of using 'globalThis'. As expected, the 'window' is not available in the web worker. I have created a local patch for our project (replacing the usage of 'window' with 'globalThis') but I think that this change should be applied to the published library as well.
I used patch-package to patch
@opentelemetry/[email protected]
for the project I'm working on.Here is the diff that solved my problem:
This issue body was partially generated by patch-package.
The text was updated successfully, but these errors were encountered: