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

Proxy support in OtlpHttpClient? #563

Closed
hygt opened this issue Mar 25, 2024 · 7 comments · Fixed by #578
Closed

Proxy support in OtlpHttpClient? #563

hygt opened this issue Mar 25, 2024 · 7 comments · Fixed by #578
Labels
enhancement New feature or request module:sdk:exporter Features and improvements to the sdk exporter module

Comments

@hygt
Copy link

hygt commented Mar 25, 2024

Hello,

I see that the current OtlpHttpClient implementation is using a hardcoded Ember client builder.
This is an issue if you need proxy support.

Would it be possible to make the builder accept a given http4s Client?
I can probably open the PR.

@iRevive
Copy link
Contributor

iRevive commented Mar 25, 2024

Hi. OpenTelemetry Java allows configuring proxy: https://github.com/open-telemetry/opentelemetry-java/blob/main/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/trace/OtlpHttpSpanExporterBuilder.java#L181-L185.

It would be nice to follow a similar approach in SDK, but it's impossible.

AFAIK Ember does not support proxy, right? So passing a preconfigured client is the only available option.


The following options are configured within the ember client builder while creating an OtlpHttpClient:

  1. timeout
  2. TLS context

We configure two middlewares (retry and gzip compression), depending on the configuration.

If we allow passing an external client, the timeout and TLS context settings will be ignored.
However, should the middleware be applied to the external client? I believe it should.

@iRevive iRevive added enhancement New feature or request tracing Improvements to tracing module module:sdk:exporter Features and improvements to the sdk exporter module and removed tracing Improvements to tracing module labels Mar 31, 2024
@hygt
Copy link
Author

hygt commented Mar 31, 2024

Hi again, yes my issue comes from the Ember client not supporting any kind of proxying.

After delving into the SDK codebase I've realized I was a bit too optimistic.
Given the auto-configuration driven API, I don't see an easy way to pass a specific http4s Client, or even an abstracted builder that would take the timeout and TLS context as parameter.

An obvious fix would be to use http4s' JDK11+ client instead.
There's no http4s-grpc backend yet anyway?

As a side note I wish there was a way to bypass the auto-configuration entirely, essentially just pass my custom Defaults case class.

@iRevive
Copy link
Contributor

iRevive commented Mar 31, 2024

There's no http4s-grpc backend yet anyway?

not yet

An obvious fix would be to use http4s' JDK11+ client instead.

We cannot because the implementation must be cross-platform. jdk11 client isn't available for Scala Native and Scala.js platforms.

Given the auto-configuration driven API, I don't see an easy way to pass a specific http4s Client

From what I see, the easiest option is to allow passing a custom client right to the OtlpSpanExporterAutoConfigure.

OpenTelemetrySdk
  .autoConfigured[IO](
    _.addSpanExporterConfigurer(
      OtlpSpanExporterAutoConfigure[IO](jdk11ClientWithProxy)
    )
  )

However, the timeout and TLS context configurations (from env variables/props) will be ignored, so you must manually preconfigure the client.

@iRevive
Copy link
Contributor

iRevive commented Mar 31, 2024

We should also allow passing an instance of the OtlpSpanExporter (add withSpanExporter to the SDK builder):

OtlpHttpSpanExporter.builder[IO]./*configurations*/.build.flatMap { spanExporter =>
  OpenTelemetrySdk.autoConfigured[IO](_.withSpanExporter(spanExporter))
}

@iRevive
Copy link
Contributor

iRevive commented Mar 31, 2024

And the last idea: we can add proxy configuration to the span exporter builder and dynamically choose the client.

If the proxy is configured, the exporter will use the JDK11 client. On Scala Native, Scala.js, and JVM pre 11 the SDK will fail to load in this case.

However, it could be tricky from the dependency management standpoint.

@iRevive
Copy link
Contributor

iRevive commented Apr 1, 2024

And the last idea: we can add proxy configuration to the span exporter builder and dynamically choose the client.

If the proxy is configured, the exporter will use the JDK11 client. On Scala Native, Scala.js, and JVM pre 11 the SDK will fail to load in this case.

However, it could be tricky from the dependency management standpoint.

I gave it a try. It may work, but there are plenty of complications, unfortunately.

@iRevive
Copy link
Contributor

iRevive commented Apr 10, 2024

We will release these changes soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request module:sdk:exporter Features and improvements to the sdk exporter module
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants