opentelemetry-go icon indicating copy to clipboard operation
opentelemetry-go copied to clipboard

OTLP*HTTP Exporter: Custom HTTP client

Open roidelapluie opened this issue 3 years ago • 9 comments

Problem Statement

It would be useful to be able to pass a custom HTTP client to this instrumentation library.

Proposed Solution

Add a WithClient() option.

Alternatives

Configuring everything with options, but it's quite complex and it costs a lot when you already have a fully featured http client.

Additional Context

I would use this for Prometheus, which has a fully featured http client.

roidelapluie avatar Feb 22 '22 15:02 roidelapluie

Is this a request related to the otelhttp instrumentation?

MrAlias avatar Feb 22 '22 17:02 MrAlias

The otelhttp instrumentation provides a RoundTripper that should be integrated into your client construction logic. This seems like a logical place to introduce a new option to configure an otelhttp.Transport.

Aneurysm9 avatar Feb 22 '22 18:02 Aneurysm9

This is about the HTTP client that sends the traces to the collector, not the HTTP client that we want to trace. E.g. our roundtripper can read passwords from files, and I'd like to be able to use the same abilities to speak to the OTLP receivers.

roidelapluie avatar Feb 22 '22 19:02 roidelapluie

This issue has been raised a number of times. I'm not in favor of it because of the ease with which you can create loops.

Let's say somewhere you use the contribs net/http to do:

// Instrument all of our library HTTP calls
stdhttp.DefaultClient = &stdhttp.Client{
    Transport: contribhttp.NewTransport(...)
}

And later someone comes along and does

exp, err := otlptracehttp.New(context.Background(),
	otlptracehttp.WithHTTPClient(http.DefaultClient),
)

Now, every time an HTTP request is made, it will use the instrumented client to make a request, which will generate more traces. Forever.

Any solution to this should also include some protection that we don't create these loops, or that the instrumentation has some escape hatch to not add tracing to the exports.

MadVikingGod avatar Feb 23 '22 13:02 MadVikingGod

@lmolkova was working on specifying a mechanism for suppressing instrumentation from multiple layers of clients. I'm not sure if it's perfectly applicable here, but it would probably be worth looking at that OTEP to see if it aligns with the need to prevent instrumentation loops in exporters.

Aneurysm9 avatar Feb 23 '22 16:02 Aneurysm9

looks like without this issue the oidc extension is mostly useless at it is not possible to update the used token

https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/extension/oidcauthextension

maybe another possible solution could be to use a function as value for headers.. so it can be implemented more dynamically

eloo avatar Feb 14 '23 10:02 eloo

We're looking into adding OTLP endpoint with AAD (oauth2, etc) auth on Azure Monitor and would be intersted in passing custom client that would allow this.

I.e. we'd want to enable the following flow:

  • exporter sends batch to OTLP HTTP endpoint
  • endpoint responds with 401
  • custom client in exporter (or auth extension in collector) then goes and requests an auth token in any way it wants
  • and retries original export batch request now with valid token. The endpoint from now on (and until token expires) responds with success
  • at some point token expires and is refreshed using the same flow

This should be a common concern for any HTTP-based OTLP endpoint with auth enabled.

If it helps, OTLP exporter in .NET provides a similar option: https://github.com/open-telemetry/opentelemetry-dotnet/tree/main/src/OpenTelemetry.Exporter.OpenTelemetryProtocol#configure-httpclient

lmolkova avatar Feb 14 '23 22:02 lmolkova

@MadVikingGod can we add the feature to atleast provide a specifications (config) of the underlying custom http.Transport and we can build that client internally. This will be really helpful in implementing a trivial task of adding the oauth2 client auth support.

This will eliminate the usage an already instrumented client as the passed config/spec will not be the exact http.Client or http.Transport. An alternate way would be to a clone of provided http.Client and overriding certain fields to avoid the instrumentation to kick in.

lprakashv avatar Sep 23 '23 15:09 lprakashv

We've hit this issue when trying to export traces from an environment without external connectivity (a secure enclave.) We need to create a custom http.Transport with the Proxy and DialContext methods overwritten, which allows us to send requests through a non-TCP socket.

Currently, the only workaround that works for us is to not use the otlptracehttp package entirely, relying on a custom exporter instead. However, it'd be much simpler if we could just pass a custom client or at least an http.Transport.

patrislav avatar Apr 24 '24 14:04 patrislav