Skip to content
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

OTLP exporters shouldn't use window as it won't work in web workers #3168

Closed
schickling opened this issue Aug 15, 2022 · 5 comments · May be fixed by #3542
Closed

OTLP exporters shouldn't use window as it won't work in web workers #3168

schickling opened this issue Aug 15, 2022 · 5 comments · May be fixed by #3542
Labels
bug Something isn't working priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization up-for-grabs Good for taking. Extra help will be provided by maintainers

Comments

@schickling
Copy link

What happened?

Steps to Reproduce

Try to use @opentelemetry/exporter-trace-otlp-http in a web worker.

Expected Result

Otel should work in a web worker

Actual Result

An error is thrown with window is not defined

image

Additional Details

OpenTelemetry Setup Code

No response

package.json

No response

Relevant log output

No response

@schickling schickling added bug Something isn't working triage labels Aug 15, 2022
@legendecas
Copy link
Member

We should run OTLP exporter tests in worker environments just like we do in packages like sdk-trace-base

@legendecas legendecas added up-for-grabs Good for taking. Extra help will be provided by maintainers and removed triage labels Aug 15, 2022
@dyladan dyladan added the priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization label Aug 17, 2022
@MSNev
Copy link
Contributor

MSNev commented Aug 17, 2022

Looking at the code for how the exporter base "uses" the window to listen for the unload event (it should be listening for more and it probably should not be just shutting down as there may be additional events (spans) that are created as part of shutdown that still need to be sent).

Having said that though, as mentioned in the SIG, because of the way this is currently coded this would be a feature request change and at the very least the "base" should be changed to "check" for the existence of window and the add/remove EventListener

@SaschaBrechmannVHV
Copy link

There is a JS lib, to move JS at the Browser to WebWorker.
partytown

Partytown is a lazy-loaded library to help relocate resource intensive scripts into a web worker, and off of the main thread. Its goal is to help speed up sites by dedicating the main thread to your code, and offloading third-party scripts to a web worker.

This could a way around.
Regards, Sascha

@justin0mcateer
Copy link

I was able to get this working in WebWorkers by simply changing the two instances of 'window' in OTLPExporterBrowserBase to 'globalThis'.

I haven't tested this elsewhere, but this should work in pretty much any environment since early 2019.

@pichlermarc pichlermarc added this to the OTLP Exporter GA milestone Apr 13, 2023
@pichlermarc
Copy link
Member

This was fixed by #4157 and then the handlers were removed completely in #4438.
Please let me know if there's any issues still and I'll re-open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority:p4 Bugs and spec inconsistencies which do not fall into a higher prioritization up-for-grabs Good for taking. Extra help will be provided by maintainers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants