elastic / apm-agent-java

https://www.elastic.co/guide/en/apm/agent/java/current/index.html
Apache License 2.0
567 stars 320 forks source link

Agent jar includes the wrong (old) version of opentelemetry API #3400

Closed jackshirazi closed 1 year ago

jackshirazi commented 1 year ago

Describe the bug

The agent jar includes shaded classes (.esclazz) which are used by the agent classloader. The opentelemetry-api included should be version 1.31.0 but it seems to have included a cached 1.14.0 version (which should only be used for a specific old-version compilation support)

Steps to reproduce

Any otel metric with a name longer than 63 characters should be okay with 1.31.0 but if used with the agent it is rejected (which was the case in the 1.14.0 version) Specifically, changing the name of the metric in the example plugin from page_views to page_viewsaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa and then doing mvn clean install at the project root to run the tests, will produce the stack trace below. Note that ValidationUtil is no longer present in opentelemetry-api 1.31.0. (note also upgrade the versions of otel and elastic in the pom, to 1.31.0 and 1.43.0 respectively)

Expected behavior

The metric name longer than 63 characters should now be accepted (the limit is higher)

Debug logs

2023-11-01 10:56:02,427 [Thread-14] WARN  io.opentelemetry.ApiUsageLogging - Instrument name "page_viewsaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" is invalid, returning noop instrument. Instrument names must consist of 63 or fewer characters including alphanumeric, _, ., -, and start with a letter. Returning noop instrument.
java.lang.AssertionError: null
    at io.opentelemetry.api.internal.ValidationUtil.log(ValidationUtil.java:55) ~[elastic-apm-agent-3c88fddf9494a420bcb10cb2c091b884-e6256b9ad98d9992211305097d439d61.jar:1.39.0]
    at io.opentelemetry.api.internal.ValidationUtil.checkValidInstrumentName(ValidationUtil.java:72) ~[elastic-apm-agent-3c88fddf9494a420bcb10cb2c091b884-e6256b9ad98d9992211305097d439d61.jar:1.39.0]
    at io.opentelemetry.sdk.metrics.SdkMeter.counterBuilder(SdkMeter.java:77) ~[elastic-apm-agent-3c88fddf9494a420bcb10cb2c091b884-e6256b9ad98d9992211305097d439d61.jar:1.39.0]
    at co.elastic.apm.agent.embeddedotel.proxy.ProxyMeter.counterBuilder(ProxyMeter.java:37) ~[elastic-apm-agent-3c88fddf9494a420bcb10cb2c091b884-e6256b9ad98d9992211305097d439d61.jar:1.39.0]
    at co.elastic.apm.agent.opentelemetry.metrics.bridge.v1_14.BridgeMeter.counterBuilder(BridgeMeter.java:38) ~[elastic-apm-agent-3c88fddf9494a420bcb10cb2c091b884-e6256b9ad98d9992211305097d439d61.jar:1.39.0]
    at co.elastic.apm.example.webserver.plugin.ExampleMetricsInstrumentation$AdviceClass.onEnterHandle(ExampleMetricsInstrumentation.java:75) ~[classes/:?]
    at co.elastic.apm.example.webserver.ExampleBasicHttpServer.handleRequest(ExampleBasicHttpServer.java:124) ~[classes/:?]
    at co.elastic.apm.example.webserver.ExampleBasicHttpServer.processClient(ExampleBasicHttpServer.java:113) ~[classes/:?]
    at co.elastic.apm.example.webserver.ExampleBasicHttpServer.start(ExampleBasicHttpServer.java:80) ~[classes/:?]
    at co.elastic.apm.example.webserver.ExampleHttpServerInstrumentationIT.lambda$startServer$0(ExampleHttpServerInstrumentationIT.java:37) ~[test-classes/:?]
    at java.lang.Thread.run(Thread.java:833) [?:?]
jackshirazi commented 1 year ago

The latest snapshot doesn't show this, so I suspect the version included may be non-deterministic (or it's been fixed?)

jackshirazi commented 1 year ago

Okay, it was just that we were including an old API version (1.22.0) up until recently, it was fixed in #3359 so will be correct in the next release