Provide OpenTelemetry OTLP/HTTP exporter

[knutwannheden]

Description

The current quarkus-opentelemetry-exporter-otlp extension exports OpenTelemetry traces using the OTLP/gRPC protocol (using the OtlpGrpcSpanExporter from the io.opentelemetry:opentelemetry-exporter-otlp-trace module). The OpenTelemetry SDK however also includes a OtlpHttpSpanExporter (in module io.opentelemetry:opentelemetry-exporter-otlp-http-trace).

We would like to be able to export our OpenTelemetry traces using the OTLP/HTTP transport (see also https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/otlp.md#otlphttp).

Implementation ideas

I think the most straight-forward solution would be to provide a new extension with a config namespace of quarkus.opentelemetry.tracer.exporter.otlp-http (in contrast to the current quarkus.opentelemetry.tracer.exporter.otlp which would pertain to the OTLP/gRPC exporter only). The config keys would basically be exactly the same, as the OTLP/HTTP exporter builder exposes the same configuration options.

The user would then have choice of either adding the quarkus-opentelemetry-exporter-otlp or the quarkus-opentelemetry-exporter-otlp-http extension.

[quarkus-bot]

/cc @radcortez

[radcortez]

Sounds good. Ideally we should align as much as possible with the OTel SDK and libraries.

[knutwannheden]

We will look into submitting a PR with our Quarkus extension and adapting the documentation.

[radcortez]

Thanks :)

[fage88]

any update on this one? I am very interested to see this

[radcortez]

We want to implement #26444 first, which is underway. This will allow us to support the expected configuration for the OTLP http exporter.

[fehmathais]

Guys, any updates on this? Are you still working on it? I've got realized that is such important to the library.
The @knutwannheden idea seems very interesting...

[radcortez]

The OTel Auto Config works took a bit longer than expected. It is mainly done, and it should be easier to support OTLP/HTTP with that, so yes, we are still working on this. Sorry for the delay.

[geoand]

I would very much like us to do this right after #34647 and #34545 get merged in order to avoid conflicts.

Also, what do we think about providing a Vert.x client based implementation (similar to what #34647 does for gRPC)?

[radcortez]

Also, what do we think about providing a Vert.x client based implementation (similar to what #34647 does for gRPC)?

I think we have to. Until now, we always relied on the exporters provided by OTel, and we started the support with gRPC. We never added support for the HTTP one (hence the issue). The idea was to add it at some point. I haven't looked into the OTel HTTP exported, but most likely, the implementation also relies on OKHttp.

[geoand]

I haven't looked into the OTel HTTP exported, but most likely, the implementation also relies on OKHttp

Currently, yes. But the next release will also have a JDK HTTP Client implementation.

[radcortez]

Hum, in that case, we should use their implementation to save us some work.

[geoand]

Yeah, that's certainly the easiest thing to do, but it probably does result in a loading a bunch of extra classes that otherwise would not be.

But I do agree we should start with it - although it will require waiting for the next OTel release (which I doubt is a problem).

[radcortez]

Ok, my recommendation is to wait for the OTel implementation, but I won't stop you to provide our own implementation if you are bored :P

[geoand]

😂.

I'll have a look

[geoand]

I see that we are on 1.25 while upstream has released 1.28 that includes the change I mentioned above.

So my plan is that when the rest of my PRs get merged, I will first open a PR updating to the latest version (with any changes necessary) and then start working on this.

[radcortez]

Sure.

[geoand]

I want to get #34545 in before working on this because is can cause more conflicts

[geoand]

So my plan is that when the rest of my PRs get merged, I will first open a PR updating to the latest version (with any changes necessary) and then start working on this.

@brunobat it seems like the bump to 1.28 breaks a few things.
It's probably best if you handle the upgrade since you have a lot more context than I do.

[brunobat]

Sure, don't worry.

[geoand]

I'll look into this once #34851 is in

[geoand]

I assume this ticket is about providing http/json support for quarkus.otel.exporter.otlp.traces.protocol, correct?

[geoand]

I am also a little confused by this. Isn't our exporter a gRPC exporter?

[brunobat]

There is not yet JSON serialisation for OTel's OTLP. PR still pending.
HTTP payload must be protobuf.

[geoand]

Gotcha, however my confusion remains, as it seems to me that what we currently provide in our code is GRPC and this ticket is about adding HTTP_PROTOBUF. Is this understanding flawed?

[radcortez]

Yes, we only support gRPC, but OTel also supports HTTP (in OTLP format):

• https://opentelemetry.io/docs/specs/otel/protocol/exporter/
• https://github.com/open-telemetry/opentelemetry-collector/blob/main/exporter/otlphttpexporter/README.md

[geoand]

Thanks.

So just to be sure we are all on the same page:

• Our current default of http/protobuf is incorrect (only as a name, since we only supported this exporter anyway) and should be changed to grpc
• We want to add support http/protobuf in this PR thus allowing the user to see set quarkus.otel.exporter.otlp.traces.protocol to either grpc or http/protobuf.

Correct?

[brunobat]

Yes @geoand. It seems correct to me.

[radcortez]

Yes, not sure why the name is incorrect.

I think we may want to keep the HTTP protobuf implementation with http name only and the json one as http+json to follow OTel conventions:

https://github.com/open-telemetry/oteps/blob/main/text/0099-otlp-http.md
https://github.com/open-telemetry/oteps/blob/main/text/0122-otlp-http-json.md

[geoand]

Thanks for the clarification folks

[geoand]

I have an implementation that seems to work, but now I need to add an integration test module.

[geoand]

Change is ready. I'll open a PR once #34851 is in.

[geoand]

I opened #35156