[Maven-Extension] Use Auto Configure Otel SDK Builder

[cyrille-leclerc]

Description:

Simplify Maven Extension configuration code using the new Otel Java SDK Auto Configuration Builder

Existing Issue(s):

• Add an auto-configuration builder. opentelemetry-java#3753

Testing:

Unfortunately not yet integration tests on the MAven Extension, I don't know how to do this yet.

Documentation:

No documentation needed

Outstanding items:

• TODO delayed by a bug of Intellij to load the opentelemetry-java-contrib on Cyrille's laptop

[anuraaga]

I mentioned it in another comment but how about just default to none regardless of endpoint variable? I think it's reasonable enough to export to localhost, and going from zero to two properties for a remote endpoint isn't that much harder than going from zero to one.

It seems simpler to document, just
The default value for exporter is none
The default value for endpoint is localhost:4317

Instead of

The default value for exporters is none, unless endpoint is explicitly passed in which case it is OTLP.
And endpoint effectively doesn't have a default value.

Former just seems simpler to me, and also fits better with non-otlp exporters which have their own endpoint variables.

[cyrille-leclerc]

💯 The change you propose would simplify integration of the Otel SDK in apps like Maven or Jenkins.

[anuraaga]

Ah I didn't mean changing the default of the OTel SDK itself (which is defined by the OTel spec) but just here, removing this if statement. We have a general stance of avoiding having default values of two variables associated with each other

open-telemetry/opentelemetry-specification#1895 (comment)

Which I think we have here. I don't see the usability drop to be significant enough if a user needs to set both OTEL_TRACES_EXPORTER and OTEL_EXPORTER_OTLP_ENDPOINT to send to a remote backend to not follow this general policy. Is it ok?

[kenfinnigan]

I've been trying to figure out from the history but can't seem to.

Is this comment still applicable @anuraaga or was it resolved @cyrille-leclerc?

Happy to review/approve when it's clear

[anuraaga]

Think this comment is still applicable. @cyrille-leclerc what do you think?

[cyrille-leclerc]

Sorry for the delay, I was not sure to understand.

Do you mean that we would change the default behaviour of the "OpenTelemetry SDK Autoconfigure" so that the default value for OTEL_TRACES_EXPORTER who no longer be otlp but be none (official doc here)?

This change would make OTEL_TRACES_EXPORTER behave the same way OTEL_METRICS_EXPORTER behave, correct?

I have few questions please:

• Is it correct that on the Otel Java agent, OTEL_TRACES_EXPORTER defaults to otlp when OTEL_METRICS_EXPORTER defaults to none making many users missing Java metrics not catching this subtle nuance of default value?
• Would this be a breaking change for Otel Java Agent with many existing deployments not defining any of OTEL_TRACES_EXPORTER and OTEL_EXPORTER_OTLP_ENDPOINT, these deployments would suddenly loose tracing
• The duality OTEL_TRACES_EXPORTER / OTEL_METRICS_EXPORTER is probably becoming verbose boilerplate with the rise of Otel metrics, including the rising support of metrics in observability backends (e.g. Jaeger is adding support for metrics via PromQL). This verbosity will even increase with the introduction of OTEL_LOGS_EXPORTER soon. Would it make sense to introduce a generic OTEL_EXPORTER so that the Java SDK exports all the signals it collects to the specified exporter?

[anuraaga]

Sorry if I wasn't clear - I don't mean to change the behavior of SDK autoconfigure (specifically, code in the opentelemetry-java repo) but just to change the defaults here. I'd like to remove handling of endpoint here so that

• No settings: No export
• OTEL_EXPORTER_OTLP_ENDPOINT=opentelemetry.io: No export
• OTEL_TRACES_EXPORTER=otlp: Exports to default localhost address
• OTEL_TRACES_EXPORTER=otlp OTEL_EXPORTER_OTLP_ENDPOINT=opentelemetry.io: Exports to opentelemetry.io

I know you are trying to make it simpler to use a remote endpoint, but I believe this provides a more consistent experience with our normal conventions, and don't believe the user experience difference for users of a remote endpoint to be negative enough in practice.

Notably, one issue I find with the current behavior is that
OTEL_TRACES_EXPORTER has a default value of none. Yet these two sets have different behavior

• OTEL_EXPORTER_OTLP_ENDPONT=opentelemetry.io: Exports
• OTEL_TRACES_EXPORTER=none OTEL_EXPORTER_OTLP_ENDPOINT=opentelemetry.io: Doesn't export

The behavior shouldn't be different since the default value of OTEL_TRACES_EXPORTER is none - we've just added a confusing caveat that the default is different based on the presence of another variable, something we would like to avoid.

This seems like an inconsistent experience. Does it make sense? I can add a commit directly to the PR if things are still unclear, let me know.

Would it make sense to introduce a generic OTEL_EXPORTER so that the Java SDK exports all the signals it collects to the specified exporter?

This is an interesting topic - it doesn't affect OTel "normally" since exporters all default to the value otlp. There is no value other than otlp that would satisfy OTEL_EXPORTER because zipkin doesn't do metrics, etc. It does affect this sort of use case where it is a good idea to change the normal default of otlp to none - we would need to discuss in spec if that means the OTEL_EXPORTER variable would be good to have for this use case.

[cyrille-leclerc]

Thanks for you patience @anuraaga , I hope I understood well your point, please check the updated code, it's greatly simplified.

[anuraaga]

Thanks! Just a couple small points - I think we can remove the getSysPropOrEnv methods too :)

[anuraaga]

It'll be cool if we can follow with populating this with XML properties from the POM file :)