prometheus / client_java

Prometheus instrumentation library for JVM applications
http://prometheus.github.io/client_java/
Apache License 2.0
2.18k stars 797 forks source link

Enabling per-query metrics in simpleclient_hibernate fails with Hibernate 4.2.0.Final #416

Open reftel opened 6 years ago

reftel commented 6 years ago

If per-query metrics is enabled and Hibernate 4.2.0.Final is used (which, according to simpleclient_hibernate/pom.xml, is supported), HibernateStatisticsCollector fails due with an exception: java.lang.NoSuchMethodError: org.hibernate.stat.QueryStatistics.getExecutionTotalTime(), since that method was not added until version 5.2.0.

brian-brazil commented 6 years ago

That's expected. What change are you suggesting we make?

reftel commented 6 years ago

Well, I see three options: either

  1. simply state that versions below 5.2.0 are not supported, or
  2. split the module into two (simpleclient_hibernate, which supports only 5.2.0 and above, and simpleclient_hibernate4, which does not provide the metric), or
  3. add the metric conditionally.

I can do a PR for either of them if needed.

brian-brazil commented 6 years ago

This is already a setting that you explicitly have to enable in your code, and the pom already states the dependency.

reftel commented 6 years ago

I guess I wasn't clear in option 3. I meant checking using introspection whether the hibernate_per_query_execution_seconds_total metric should be added (and then fetching it in a roundabout way, e.g. introspection or dynamically loaded or generated code).

As for the pom, the comment by the hibernate dependency states "We support Hibernate versions >= 4.2". If the intention is to not support per-query metrics in versions before 5.2.0, then the comment should probably make that clear. "We support Hibernate versions >= 4.2, with per-query metrics in versions >= 5.2", or something like that?

brian-brazil commented 6 years ago

Ah, that comment was missed in code review. It should say 5.2.0 now.

reftel commented 1 year ago

The comment still says "We support Hibernate versions >= 4.2": https://github.com/prometheus/client_java/blob/main/simpleclient_hibernate/pom.xml#L43