[Maven Extension] Add Tracer instrumentationVersion (ie otel.library.version)
#191
Conversation
| if (otelMavenExtensionVersion != null) { | ||
| tracerBuilder.setInstrumentationVersion(otelMavenExtensionVersion); | ||
| } | ||
| this.tracer = tracerBuilder.build(); |
There was a problem hiding this comment.
Do we need the null check? From looking at the implementation it appears we can go with:
this.tracer = this.openTelemetry.getTracer("io.opentelemetry.contrib.maven", this.getClass().getPackage().getImplementationVersion());
There was a problem hiding this comment.
I initially started like this as implementationVersion will only be null in unit tests.
However, I hardened the code because null is a violation of the contractOpenTelemetry#getTracer(@Nonnull instrumentationName, @Nonnull instrumentationVersion).
It's probably an acceptable violation of the contract as the implementation supports null and we would not be afraid of regressions if the implementation starts rejecting null instrumentationVersion because it would fail in unit tests.
So if you are ok with this light violation, I'm happy to simplify the code and adopt your proposal.
There was a problem hiding this comment.
@kenfinnigan please review the proposed code change. Slightly different from your suggestion in order to also output the otel maven extension version in the logs.
| @@ -25,6 +25,9 @@ | |||
| @Component(role = OpenTelemetrySdkService.class, hint = "opentelemetry-service") | |||
| public final class OpenTelemetrySdkService implements Initializable, Disposable { | |||
|
|
|||
| public static final String VERSION = | |||
| OpenTelemetrySdkService.class.getPackage().getImplementationVersion(); | |||
Description:
Add Tracer instrumentationVersion (ie
otel.library.versionattribute).Existing Issue(s):
otel.library.version#152Testing:
Tested manually.
No unit test because algorithm relies on the jar manifest implementationVersion that is not available during the unit tests.
Documentation:
None
Outstanding items:
None