rh-messaging / artemis-prometheus-metrics-plugin

Apache License 2.0
22 stars 20 forks source link

After adding the Open Telemetry agent the plugin crashes #12

Closed EdwardKuenen closed 1 year ago

EdwardKuenen commented 1 year ago

Describe the bug We use Apache Artemis with the artemis-prometheus-metrics-plugin. We want to use the java agent for collecting logging. After adding the java agent the metrics are not collected anymore and the /metrics endpoint could not be found anymore.

Steps to reproduce Configure Artemis to use the prometheus plugin Add the java agent to the startup command with following configuration.

JAVA_ARGS="$JAVA_ARGS -javaagent:/data/opentelemetry/opentelemetry-javaagent.jar"
JAVA_ARGS="$JAVA_ARGS -Dotel.logs.exporter=otlp"
JAVA_ARGS="$JAVA_ARGS -Dotel.metrics.exporter=none"
JAVA_ARGS="$JAVA_ARGS -Dotel.traces.exporter=none"
JAVA_ARGS="$JAVA_ARGS -Dotel.service.name=artemis-broker"

What did you expect to see? The plugin keeps working and the logs are scraped.

What did you see instead? The logs are scraped, but the /metrics endpoints is not available anymore. In the log I see the stacktrace below.

What version are you using? Java agent 1.25.1

Environment OS: Ubuntu 20.04.6 LTS Runtime (if different from JDK above): OpenJdk Corretto-17.0.6.10.1

Additional context Issue at Open Telemetry Java instrumentation https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/8404

Stacktrace

    at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
    at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
    at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
    at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
    at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
    at org.eclipse.jetty.server.handler.ContextHandler$StaticContext.createInstance(ContextHandler.java:2891)
    at org.eclipse.jetty.servlet.ServletContextHandler$Context.createInstance(ServletContextHandler.java:1292)
    at org.eclipse.jetty.servlet.ServletContextHandler$Context.createInstance(ServletContextHandler.java:1301)
    at org.eclipse.jetty.servlet.BaseHolder.createInstance(BaseHolder.java:204)
    at org.eclipse.jetty.servlet.ServletHolder.createInstance(ServletHolder.java:1169)
    at org.eclipse.jetty.servlet.ServletHolder.newInstance(ServletHolder.java:1161)
    at org.eclipse.jetty.servlet.ServletHolder.initServlet(ServletHolder.java:594)
    at org.eclipse.jetty.servlet.ServletHolder.getServlet(ServletHolder.java:486)
    at org.eclipse.jetty.servlet.ServletHolder.prepare(ServletHolder.java:731)
    at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:524)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:131)
    at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:578)
    at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:122)
    at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:223)
    at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1571)
    at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:221)
    at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1378)
    at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:176)
    at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:484)
    at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1544)
    at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:174)
    at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1300)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:129)
    at org.eclipse.jetty.server.handler.HandlerList.handle(HandlerList.java:51)
    at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:122)
    at org.eclipse.jetty.server.Server.handle(Server.java:562)
    at org.eclipse.jetty.server.HttpChannel.lambda$handle$0(HttpChannel.java:505)
    at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:762)
    at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:497)
    at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:282)
    at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:319)
    at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:100)
    at org.eclipse.jetty.io.SelectableChannelEndPoint$1.run(SelectableChannelEndPoint.java:53)
    at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:894)
    at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1038)
    at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.lang.ClassCastException: class io.opentelemetry.javaagent.shaded.instrumentation.micrometer.v1_5.OpenTelemetryMeterRegistry cannot be cast to class io.micrometer.prometheus.PrometheusMeterRegistry (io.opentelemetry.javaagent.shaded.instrumentation.micrometer.v1_5.OpenTelemetryMeterRegistry and io.micrometer.prometheus.PrometheusMeterRegistry are in unnamed module of loader java.net.URLClassLoader @7c36db44)
    at com.redhat.amq.broker.core.server.metrics.plugins.ArtemisPrometheusMetricsPluginServlet.<init>(ArtemisPrometheusMetricsPluginServlet.java:40)
    ... 41 more
brusdev commented 1 year ago

@EdwardKuenen this issue should be easy to fix, maybe a check to exclude registries different from io.micrometer.prometheus.PrometheusMeterRegistry should work . Are you planning to create a pull request to fix this issue?

https://github.com/rh-messaging/artemis-prometheus-metrics-plugin/blob/v2.0.0/artemis-prometheus-metrics-plugin-servlet/src/main/java/com/redhat/amq/broker/core/server/metrics/plugins/ArtemisPrometheusMetricsPluginServlet.java#L40

EdwardKuenen commented 1 year ago

Hi @brusdev , thank you for your fast response! I sure will make a PR if I can fix it.

The OpenTelemetryMeterRegistry is a subclass of io.micrometer.core.instrument.MeterRegistry.

If I check the registry for a PrometheusMeterRegistry instance it will fail and I will not get any metrics if I understand correct. The scrape is not a method of MeterRegistry, so I can not check and cast to MeterRegistry.

Do you know if I can collect metrics from Artemis with the OTEL java agent without the need for the plugin? Micrometer should be supported, but I do not get any metrics with the agent on Artemis (I disabled the line with -Dotel.metrics.exporter=none).

https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/docs/supported-libraries.md

brusdev commented 1 year ago

I think this issue is caused by the Open Telemetry java agent that is injecting OpenTelemetryMeterRegistry registries while the artemis-prometheus-metrics-plugin only expects PrometheusMeterRegistry registries : https://github.com/rh-messaging/artemis-prometheus-metrics-plugin/blob/v2.0.0/artemis-prometheus-metrics-plugin-servlet/src/main/java/com/redhat/amq/broker/core/server/metrics/plugins/ArtemisPrometheusMetricsPluginServlet.java#L40

   public ArtemisPrometheusMetricsPluginServlet() {
      Set<MeterRegistry> registries = Metrics.globalRegistry.getRegistries();
      if (registries != null && registries.size() > 0) {
         registry = (PrometheusMeterRegistry) registries.toArray()[0];
      }
   }

I was thinking to add a check to exclude registries different from io.micrometer.prometheus.PrometheusMeterRegistry, i.e.

   public ArtemisPrometheusMetricsPluginServlet() {
      Set<MeterRegistry> registries = Metrics.globalRegistry.getRegistries();
      if (registries != null && registries.size() > 0) {
         for (MeterRegistry meterRegistry : registries) {
            if (meterRegistry instanceof PrometheusMeterRegistry) {
               registry = (PrometheusMeterRegistry) meterRegistry;
               break;
            }
         }
      }
   }
EdwardKuenen commented 1 year ago

@brusdev I think you are correct that this solves the cast exception. I will make a PR for that and we will see if I still got any metrics with the java agent loaded.

jbertram commented 1 year ago

It's not clear to me if the problem is that there's an additional meter registry (i.e. an instance of OpenTelemetryMeterRegistry) or if the OpenTelemetry instrumentation is replacing the existing instance of PrometheusMeterRegistry with an instance of OpenTelemetryMeterRegistry so that it can intercept the metrics and report them back to OpenTelemetry. If it's the former then the fix from @brusdev should work. If it's the latter then no fix will work since the plugin must use PrometheusMeterRegistry in order to invoke scrape().

In any case, if all you want to do is gather metrics for OpenTelemetry and you don't really care about Prometheus then I believe you can just use org.apache.activemq.artemis.core.server.metrics.plugins.SimpleMetricsPlugin as the metrics plugin in broker.xml, e.g.:

<metrics>
   <plugin class-name="org.apache.activemq.artemis.core.server.metrics.plugins.SimpleMetricsPlugin"/>
</metrics>

If there's no plugin configured then the broker doesn't actually use Micrometer at all. This is done explicitly so that the broker doesn't have a hard dependency on Micrometer.

EdwardKuenen commented 1 year ago

I tried the SimpleMetricsPlugin, but I did not get any metrics from the OTEL java agent. I should not have add something to the lib folder? I did got this message in the log [org.apache.activemq.hawtio.plugin.PluginContextListener] Initialized artemis-plugin plugin

EdwardKuenen commented 1 year ago

I made the change to the Prometheus plugin as suggested by Bruscino. Now I get the metrics by scraping /metrics with the agent loaded. The agent is not submitting metrics.

EdwardKuenen commented 1 year ago

Hi, I made your suggestion and bumped a lot of versions to a more recent version. Could you please check if this is OK?
Thank you!

brusdev commented 1 year ago

@EdwardKuenen you did more than a simple fix, nice work!!! I added some comments to the your PR.

EdwardKuenen commented 1 year ago

Thank you! I am not a very experienced developer, but I hope I can do something in return for your help!

EdwardKuenen commented 1 year ago

Thank you @brusdev and @jbertram! This solved my issue..

brusdev commented 1 year ago

This fix is included in v2.1.0