Include a meter id's base unit and description in the metrics endpoint's output

[joshiste]

Spring Boot Admin takes the various time gauges from the metrics endpoint (e.g. process.uptime) . Unfortunately the time unit is not given in the actuator output and can vary depending on the underlying micrometer registry.
Therefore it would be great if either the time unit is fixed in the metrics endpoint or included in the output. Currently the time unit can only be guessed.

[joshiste]

/cc @jkschneider

[wilkinsona]

This looks like a Micrometer bug to me. Both Statistic.TOTAL_TIME and Statistic.DURATION say that they are "always reported in nanoseconds". Statistic.MAX says the same when it represents a time. MetricsEndpoint handles Measurements which are a Statistic and value pair. For measurements for a TOTAL_TIME, DURATION, or MAX statistic, I'd expect the value to always be in nanoseconds, irrespective of the MeterRegistry that returned the Meter.

@joshiste Can you please open a Micrometer issue? I'll close this one for now but we can re-open it if changes are needed in Boot after all.

[jkschneider]

@wilkinsona The javadocs are wrong and have been changed. I think I've mentioned before that it would be better if Spring Boot Admin did not rely on the metrics endpoint. I think it should provide a meter registry implementation that exposes metrics in a way that is peculiar to it.

If we standardize the base unit of time in the metrics endpoint, somebody will complain that the values aren't the same as what they see in their monitoring system. If we align with the monitoring system (what we are doing) somebody will complain about inconsistency depending on implementation. It is a lose-lose situation that highlights the fact that the metrics endpoint is a diagnostic only tool.

[joshiste]

@jkschneider

I think it should provide a meter registry implementation that exposes metrics in a way that is peculiar to it.

We try to support spring boot applications without the user having to do modifications to it.

If we standardize the base unit of time in the metrics endpoint, ...

Agreed. What's about the other option: Including the base time unit for the meter in the endpoint's output? This way we don't get inconsistent with other implementations and SBA could recalculate.
(BTW: Also having the meter's description in the endpoint would be awesome)

@wilkinsona could you reopen the issue?

[wilkinsona]

Thanks, @jkschneider.

Even if it's only a diagnostic tool, and putting aside Spring Boot Admin for now, I can see some value in the endpoint exposing the base unit of time of the underlying registry. Do you think it would be worth making getBaseTimeUnit() on MeterRegistry public so that we could include that information in the endpoint's response?

[wilkinsona]

@wilkinsona could you reopen the issue?

We can re-open it if/when we agree that there's a change to be made. Right now, Boot's hands are tied as Micrometer does not expose a registry's base time unit.

[joshiste]

@wilkinsona Imho we should not get the MeterRegistry's base time unit, but the one for the meter via Meter.getId().getBaseUnit() in case it differs from the registry's.

[joshiste]

@wilkinsona updated my last comment... Meter.getId().getBaseUnit() isn't package scoped. So it would be possible right now. I'd provide a PR if you want me to.

[jkschneider]

Adding description text and base unit are useful enhancements to the metrics endpoint anyway, and if that would help Spring Boot Admin scale the value then I think that is a workable solution.

Good suggestion @joshiste!

[wilkinsona]

@jkschneider Can we use the base unit from the meter id? Judging by the updated javadoc, a statistic’s value will be in the registry’s base unit whereas the meter id makes no such promises. That makes me think that it’s the registry’s that we need. Regardless, I’d prefer to deal with the strongly-typed TimeUnit from the registry rather than a String from the Id as it will give endpoint clients a fixed set of possible values to deal with.

[joshiste]

@jkschneider meter.getId().getBaseUnit() is null for process.uptime.
Having a quick look at the micrometer source tells me that it's always null for TimeGauges...
So this issue still depends on a change in micrometer...

[jkschneider]

Great, Github API issues caused the same issue to be opened 6 times.

[jkschneider]

The TimeGauge issue has been resolved for Micrometer 1.0.6.

[wilkinsona]

Thanks, Jon. Any thoughts on using the registry’s base time unit vs the id’s?

[jkschneider]

IMO use the ID's. While they should always be matched up to the registry base unit of time on Micrometer-provided meter types, folks are free to do whatever they want on custom meter types.

[wilkinsona]

Thanks again, @jkschneider.

@joshiste I think we've got what we need to be able to implement this. A pull request would be much appreciated.

[wilkinsona]

Closing in favour of PR #13813.