micrometer icon indicating copy to clipboard operation
micrometer copied to clipboard

JMX Registry always reports timers in milliseconds

Open pcasaes opened this issue 4 years ago • 2 comments

Describe the bug When overriding TimeUnit getBaseTimeUnit() to return SECONDS timers are still reporting values in MILLISECONDS

Environment

  • Micrometer version 1.6.11
  • Micrometer registry JMX
  • OS: macOs
  • Java version: openjdk version "11.0.9.1" 2020-11-04 OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.9.1+1) OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.9.1+1, mixed mode)

To Reproduce How to reproduce the bug: Override getBaseTimeUnit in JmxMeterRegistry

@Override protected TimeUnit getBaseTimeUnit() { return TimeUnit.SECONDS; }

Set up a simple timer.

Timer.builder("simple_timer")
  .publishPercentiles(0.5, 0.75, 0.95)
  .register(registry)

It will report in MILLISECONDS.

Expected behavior Timers should respect getBaseTimeUnit

Additional context I'm fairly confident that the issue is in this line https://github.com/micrometer-metrics/micrometer/blob/67c41db552cac463e44cd45545e5a7b8f3513f1b/micrometer-core/src/main/java/io/micrometer/core/instrument/dropwizard/DropwizardTimer.java#L35

It's creating timers with hardcoded MILLISECONDS base unit.

pcasaes avatar Sep 19 '21 15:09 pcasaes

@pcasaes Thanks for your report. I wonder if the millisecond baseunit is a limitation on the JMX integration itself. Which is why it is hardcoded as you saw.

Can you tell me a bit more about your usecase for wanting to change the baseunit to seconds? I wouldn't be surprised if JMX is using longs instead of doubles, and thus can't be changed (but I'm pulling from ancient memories here)

If my guesses were correct, would the proper fix be to throw an exception when a non-millisecond baseunit is used to eliminate confusion that nothing else is supported?

checketts avatar Sep 19 '21 21:09 checketts

The JMX registry relies on the Dropwizard JmxReporter. While it may be an oversight that we're hardcoding the time unit where you mentioned, it ultimately wouldn't change the output. If you want to customize that, you can provide a JmxReporter configured to convert durations to seconds and pass that to the JmxMeterRegistry constructor. I tried this locally and was able to get seconds for timers in JMX.

JmxReporter reporter = JmxReporter.forRegistry(metricRegistry)
                .convertDurationsTo(TimeUnit.SECONDS)
                .inDomain(config.domain())
                .build();
JmxMeterRegistry registry = new JmxMeterRegistry(JmxConfig.DEFAULT, Clock.SYSTEM, HierarchicalNameMapper.DEFAULT, metricRegistry, jmxReporter);

I'm also interested in the use case, since we generally don't expect JMX to be used in production for metrics.

shakuzen avatar Sep 20 '21 02:09 shakuzen