open-telemetry / opentelemetry-js

OpenTelemetry JavaScript Client
https://opentelemetry.io
Apache License 2.0
2.76k stars 811 forks source link

[Feature Request] Provide a way to customize http client of exporter-otlp-http #2501

Closed meteorlxy closed 2 years ago

meteorlxy commented 3 years ago

Is your feature request related to a problem? Please describe.

OTEL provides different exporters for web and node in @opentelemetry/exporter-otlp-http, because we need to use XHR in web, and use http module in node.

However, users may need to use OTEL in other environment, which does not support XHR nor http module.

For example, mini programs are very popular in China (I think @legendecas could understand the popularity of mini program), but most of those platforms only allow using their own API to send http request:

In such cases, currently we have to fork @opentelemetry/exporter-otlp-http and modify the the source code to compat with those platform.

So it could be better to provide a more flexible way to customize http client.

Describe the solution you'd like

Maybe we could split @opentelemetry/exporter-otlp-http into:

Then users could extend @opentelemetry/exporter-otlp-http-base to develop their own compat packages.

echoontheway commented 3 years ago

Sounds good! If @opentelemetry/exporter-otlp-http-base is provided, I think it's also very convenient for Hippy(https://github.com/Tencent/Hippy) which is popular used in my company to extend the exporter.

legendecas commented 3 years ago

I'm very excited to see wider support of OpenTelemetry on various JavaScript environments.

I believe the condition also applies to Deno and Cloudflare Worker. They don't provide native XHR support but only fetch API support. Obviously they don't support opentelemtry/node packages as they're Web API compatible platforms. Also, as XMLHttpRequest and Navigator.sendBeacon are not supported on Web ServiceWorkers, the "browser" ServiceWorker is not able to support the current otlp HTTP exporter too. So I'd say upgrading otlp http exporter to optionally depends on Fetch API and prevent from using window specific API (usages in otlp http exporter) would be a first step to support more Web environments.

However, I'd hardly believe that OpenTelemetry JS SDK can be run natively on mini programs as they didn't implement Web Standard APIs like globalThis.performance and more importantly they didn't provide any guarantee that their APIs are Web compatible. AFAICT there is no clear plan that OpenTelemetry JS SDK is going to provide official support on platform other than Node.js and Web compatible platforms.

vmarchaud commented 3 years ago

AFAICT there is no clear plan that OpenTelemetry JS SDK is going to provide official support on platform other than Node.js and Web compatible platforms.

I'm not personally against introducing support for additional platforms (there is one issue for deno already #2293) but that would require someone have time dedicated to maintain them.

dyladan commented 3 years ago

AFAICT there is no clear plan that OpenTelemetry JS SDK is going to provide official support on platform other than Node.js and Web compatible platforms.

I'm not personally against introducing support for additional platforms (there is one issue for deno already #2293) but that would require someone have time dedicated to maintain them.

In this case, I think having exporter-otlp-{protocol}-base was already in the plan anyway in order to more easily support node and web in exporter-otlp-{protocol}-{platform}. If someone wants to make and support a new platform from there, i think it would be acceptable for it to live in the contrib repo.

github-actions[bot] commented 2 years ago

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.

github-actions[bot] commented 2 years ago

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.

github-actions[bot] commented 2 years ago

This issue was closed because it has been stale for 14 days with no activity.