-
Notifications
You must be signed in to change notification settings - Fork 810
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
TraceExporter for fetch
-only environment
#4631
Comments
Hi @lacolaco I would like to understand better your use case. Do you want to instrument a WebWorker/ServiceWorker that runs on the Browser? If so, do you also want to instrument the UI thread and correlation between traces form the UI and the worker? |
@david-luna I have two use cases; ServiceWorker on the browser and Cloudflare Workers. ServiceWorker needs to be able to propagate traces from UI threads to and from the backend server. This is equivalent to a proxy server from an overall perspective. The Cloudflare Worker is simply instrumentation as a back-end server. |
I'm not familiar with Cloudfare Workers but regarding Service Workers I think sending traces with At least instrumentation of FetchEvent needs to be implemented so context is properly propagated when creating spans within the SW. Also I think there might be necesary to review core and web-sdk packages to make sure they are not using APIs not available in workers. |
To clarify in case there is some misunderstanding, I am referring to the implementation of the exporter using the fetch api, not instrumentation for the fetch api. |
okay, so I guess you will do manual instrumentation of the incoming requests to the Service Worker to keep the trace context from the UI. I'll share today with the JavaScript SIG or maybe you can join and discuss this topic. |
Thank you for the escalating! |
We did discuss adding this feature and definitely is a feature we want in OTEL. PR #3542 it's quite complex but it will get simpler once #4542 lands into the main branch. So the plan would be:
|
@david-luna Sounds nice! Thank you all for the great job! |
Thank you @david-luna for summarizing this and bringing it up in the SIG. To expand on what David said, there are a few more steps to be able to merge #3542 - we'll need to re-structure how we handle transports in the OTLP exporters (see #4116 for a more in-depth reasoning on why this is necessary). I have a draft for this at #4415, but applying that'll be a multi-step process as we try to avoid breaking users (the exporters are quite heavily used, so we try to tread lightly with changes there).
After all these changes adding |
hi this is very much needed! |
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. |
Looking forward to when this is supported here in the upstream, much love to all the awesome contributors spending time and effort to prepare the foundations for this change! In the meantime, I've forked @sugi working change in #3542 to https://github.com/jgibo/opentelemetry-js/tree/service-worker-support. And from my fork I've published the changes to npm as package https://www.npmjs.com/package/@jgibo/exporter-trace-otlp-http |
NB: Before opening a feature request against this repo, consider whether the feature should/could be implemented in the other OpenTelemetry client libraries. If so, please open an issue on opentelemetry-specification first.
Is your feature request related to a problem? Please describe.
Currently, built-in
TraceExporter
implementation for browser depends onXMLHttpRequest
orsendBeacon
. These can't be adopted forfetch
-only environments like WebWorker, ServiceWorker, or some edge workers (e.g. Cloudflare workers).https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/otlp-exporter-base/src/platform/browser/OTLPExporterBrowserBase.ts#L59
Once we accept
fetch
as an HTTP request sender, it can also be used in Node.js environments.Describe the solution you'd like
OTLPExporterBrowserBase
OTLPExporterFetch
Describe alternatives you've considered
Additional context
fetch
but only about instrumentation.The text was updated successfully, but these errors were encountered: