Should we provide jdk11 version of OTLP http exporters?

[jack-berg]

Related issues: #4777

@brunobat brings up in CNCF slack that OkHttp is problematic because older versions may have unpatched CVEs, while newer versions have a dependency on kotlin stdlib.

In #5341 I've prototyped an approach where we publish a multi-release jar where in which the java11+ version uses the jdk 11+ Http Client. After some initial concerns about performance, I've been able to iron out much of the problem by replacing some of the default implementation of BodyPublisher.

But additional work is needed to get the prototype in a place to merge.

Before I do that work, I want to answer a few key questions:

1. Should we provide an OTLP exporter option which doesn't rely on dependencies?
2. If yes, how should we deliver that solution? a. One option is to do the multi-release jar prototyped in #5341. Users would need to explicitly exclude the okhttp dependency using gradle exclude syntax. b. Another option is to provide dedicated "sender" or "tranasport" artifacts representing the different implementations. I.e. publish new opentelemetry-exporter-jdk-http-sender and opentelemetry-exporter-okhttp-sender. We could then include the okhttp sender as a default dependency of opentelemetry-exporter-otlp. Or we could force users to choose one, and document that when you add opentelemetry-exporter-otlp, you must also add a opentelemetry-exporter-*-sender.

Want to gather feedback before I go any further working on this. I think the #5341 prototype makes it clear that its possible and reasonable to have a zero dependency otlp http exporter. Now we need to decide whether we to deliver and maintain that.

[io7m]

A common way to handle this is to define an interface, have various implementations implement it, and then load an implementation using ServiceLoader. This is explicitly supported by the module system (uses and provides).

At a very high level, you'd have something like:

A module io.opentelemetry.http.spi defining the interface of "things that can provide HTTP clients" but no actual executable code:

interface HTTPClient {
  // Whatever methods are needed for OTLP
}

interface HTTPClientProvider {
  HTTPClient create();
}

Then a module io.opentelemetry.http.jdk that provides an implementation of HTTPClient using the JDK 11+ client. Additionally, a module io.opentelemetry.http.okhttp3 that provides an alternative implementation of the HTTP client using okhttp3. Both modules would include an entry in their module descriptors such as:

module io.opentelemetry.http.jdk {
  provides io.opentelemetry.http.spi.HttpClient
    with io.opentelemetry.http.jdk.JDKHttpClients; // ... Or whatever class name is used.
}

The OLTP implementation then declares a dependency solely on io.opentelemetry.http.spi, and uses ServiceLoader to ask for an implementation of HTTPClientProvider. An implementation will be automatically chosen (somewhat arbitrarily) from whatever modules are present on the module path that claim they provide an implementation. The person assembling the application is responsible for making sure that at least one implementation is available, by ensuring that either io.opentelemetry.http.jdk or io.opentelemetry.http.okhttp3 are on the module path at runtime. Obviously OLTP would be responsible for providing a useful error message if the users have failed to do this.

[io7m]

The nice thing about using services is that it makes the dependency explicit whilst also leaving users free to pick which dependency they want. The service declarations are also recognized by GraalVM and jlink and so on.

[io7m]

This is also assuming that you actually want to continue to have multiple http client implementations. I'm not sure if that was determined or not.

[brunobat]

@io7m there are retry requirements and the need to use a provided serializer

[io7m]

@io7m there are retry requirements and the need to use a provided serializer

I can't speak to the actual implementation of the client as I don't know the OLTP codebase well enough. I just wanted to explain that there's a fairly standard way to deliver something like this (where you have users picking between optional dependencies) that's supported by the module system. That was question 2 on the list above. :slightly_smiling_face:

[jack-berg]

After further consideration I think the best approach will be to create multiple http sender artifacts, one for okhttp and one for jdk http client, and require that one be present in order to use the OTLP http exporters. The implementation of this is more clear than the multi version jar approach, and means that users are including / excluding opentelemetry artifacts in order to achieve the desired behavior rather than okhttp dependencies.

I propose we:

• Add a new HttpSender interface to opentelemetry-exporter-common. This will contain the generic contract to send an http request and process the response, including retry logic.
• Add a new HttpSenderProvider SPI interface to opentelemetry-exporter-common. This SPI will be used to find / instantiate the HttpSender on the classpath.
• Start publishing a new opentelemetry-exporter-http-sender-okhttp artifact, containing the OkHttp based implementation of HttpSender.
• Start publishing a new opentelemetry-exporter-http-sender-jdk artifact, containing the jdk 11 http client based implementation of HttpSender.

We can decide whether opentelemetry-exporter-otlp should include one of the senders as a dependency by default, or force the user to choose one. I'm in favor of forcing the user to choose one so its very explicit. This would result in dependencies like:

implementation("io.opentelemetry:opentelemetry-exporter-otlp")
implementation("io.opentelemetry:opentelemetry-exporter-http-sender-jdk")
// or alternatively...
// implementation("io.opentelemetry:opentelemetry-exporter-http-sender-okhttp")

The otel java agent would add a dependency on the opentelemetry-exporter-http-sender-okhttp to maintain java 8 compatibility, and since the okhttp dependency isn't a problem in the agent.

As mentioned in the java sig, we suspect the majority of SDK users would add a dependency on opentelemetry-exporter-http-sender-jdk because:

• Using the SDK directly means that the application is actively maintained, since it requires modification of source code / dependencies
• Actively maintained projects are likely to be upgraded to java 11+ and have access to jdk http client. If jdk http client is available and has acceptable performance, it will likely be preferred since it avoids extra dependencies.
• The projects that are likely stuck on java 8 are those which not being actively developed, which will prefer the java agent approach.

I'm working on implementing this plan locally and will deliver it incrementally to make review more manageable. Please let me know if you have strong feelings against this approach.

[bogdandrutu]

What is the lifetime of java8? Since OkHTTP and other major libraries dropped support for java8 have you considered to do the same?

[jack-berg]

Removing java 8 support would mean removing java 8 support for the otel java agent, which would be extremely unpopular given that the java agent is popular among legacy apps.

[mateuszrzeszutek]

Since OkHTTP and other major libraries dropped support for java8 have you considered to do the same?

OkHttp still supports Java 8 though.

Android still uses Java 8 level APIs, and I'm not sure whether that's likely to change -- probably not, Kotlin's supposed to bridge that gap. If we drop Java 8 we might run into issues with Android support.

[brunobat]

I added a comment on the PR because I think we should consider only allowing one HttpSender, to avoid mistakes.

[geoand]

I think this is a great way to embrace the newer versions of the JDK.

It makes me wonder, is there a plan to do JDK version of GrpcExporter as well?

[jack-berg]

It makes me wonder, is there a plan to do JDK version of GrpcExporter as well?

We would love to but the JDK HttpClient doesn't support trailing headers, which are required for gRPC 😞

[geoand]

I did not know that, very interesting, thanks!