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

Stabilize client-side OTEL export #312

Open
myieye opened this issue Oct 9, 2023 · 2 comments · Fixed by #341
Open

Stabilize client-side OTEL export #312

myieye opened this issue Oct 9, 2023 · 2 comments · Fixed by #341
Labels
OpenTelemetry owner: Tim, Kevin
Milestone

Comments

@myieye
Copy link
Contributor

myieye commented Oct 9, 2023

Client-side OTEL uses the Beacon API for exporting traces. That's pretty much what the beacon API is for:

Unlike requests made using XMLHttpRequest or the Fetch API, the browser guarantees to initiate beacon requests before the page is unloaded and to run them to completion.

The main use case for the Beacon API is to send analytics such as client-side events or session data to the server.

but it has limitations that make trace collection flaky (#307, open-telemetry/opentelemetry-js#3489).

Using the fetch API with the keepalive parameter would be more reliable (although it's not implemented yet in FireFox).

Despite the lack of support in FF it still might be worth changing to the fetch API, but the last time this was discussed in OTEL it didn't lead anywhere. There doesn't seem to be an easy way to do it currently. There is a way to use XMLHttpRequest, but that wouldn't be as big of a win.

I was directed to what looks like the most active relevant OTEL issue:

There is some other relevant work, but most of it is somewhat stale:

@myieye myieye added this to the v1 milestone Oct 9, 2023
@hahn-kev
Copy link
Collaborator

Thanks for doing that research Tim, that looks like a lot of work. If you are interested feel free to contribute to OTEL in some way if you think that could help.

On the other hand, one option while we're waiting for them to fix this is that we could just provide our own implementation and override the send method.
https://github.com/open-telemetry/opentelemetry-js/blob/c320c981c5b8cd9c42d65183c2c2c5b737a0b2a1/experimental/packages/otlp-exporter-base/src/platform/browser/OTLPExporterBrowserBase.ts#L63

@myieye
Copy link
Contributor Author

myieye commented Nov 24, 2023

As expected, after merging #341 (implementing a rudimentary, but seemingls solid fallback), we don't get any more sendBeacon error traces, because we always retry with XHR.
We also don't get any XHR failures in HoneyComb, because the only case where we expect it to fail is if it's too late, in which case we can't get notified that it was too late, because....it's too late.

So, this is already pretty much as good as it gets and our implementation is pretty simple.
The only other optimisation I can think of is putting failed exports into local-storage or something and then trying them later, but that seems like overkill.

So, I'm not sure if we really need to keep this open. At least, I'm not really interested in working on it for a while (e.g. until there are valuable changes we can use in the OTEL SDK).

@hahn-kev Can we put this in vNext?

@myieye myieye linked a pull request Nov 24, 2023 that will close this issue
@hahn-kev hahn-kev modified the milestones: v1, vNext Nov 24, 2023
@myieye myieye added the OpenTelemetry owner: Tim, Kevin label Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OpenTelemetry owner: Tim, Kevin
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants