Add option for time-based FunctionCounter

[shakuzen]

• Add timeUnit to FunctionCounter.Builder
• Add package-private method functionTimeCounter to MeterRegistry.More to create a FunctionCounter that scales its value to the registry's base time unit
• This is analogous to how TimeGauge works, and provides a way to register monotonically increasing time values (like jvm.gc.cpu.time or process.cpu.time) in a way that automatically scales to the preferred time unit of the metrics backend.

See comments on #7297 for background motivating this change.

This intentionally only adds a new public method to FunctionCounter's Builder to reduce the surface area of this change to the minimum to achieve the feature.

An alternative would be introducing a dedicated type such as FunctionTimeCounter which would perhaps make the feature more discoverable but obviously has more public API to support when the difference to a FunctionCounter is tiny.

[shakuzen]

Perhaps it's more clear to name the method functionTimeUnit to distinguish from the time unit that will end up on the Meter (Id), which is actually the registry's base time unit?

[jonatan-ivanov]

I was thinking about this, I remember people getting confused with the TimeUnit parameter of FunctionTimer/TimeGauge.

[shakuzen]

Do you mean to say it would be clear to name it functionTimeUnit or should we do more/something else to make it more clear?

[jonatan-ivanov]

Oh sorry, I meant to say that using functionTimeUnit would be clear.

[shakuzen]

I'm out of time right now, but I think this documentation can be improved. Either we should link to our documentation on the mentioned metrics (if we have any) or instead reference the JMX method from which they're derived, as that is more universally understandable.

[jonatan-ivanov]

I was thinking about this, I remember people getting confused with the TimeUnit parameter of FunctionTimer/TimeGauge.

[shakuzen]

Another idea I had was adding a variant of the builder method that takes a function with return type Duration, and then we don't need to take a TimeUnit or baseUnit at all (we can return a Builder that doesn't have those methods). I mostly like this because it removes the issue now where timeUnit and baseUnit are mutually exclusive options, which is something users need to understand about the API. That said, I don't know if it is worth adding now unless we have a clear idea where it would be used. I don't think many (any?) backing functions for typical FunctionCounters currently return a Duration. Converting number return types to Duration is straightforward, but it requires wrapping a method instead of using it directly, as is most common for backing functions.

[jonatan-ivanov]

I really like this idea. I'm also thinking if we should call the time-based property out in the method name:

static <T> TimeBasedBuilder<T> timeBasedBuilder(String name, T obj, Function<T, Duration> f) { ... }

static <T> TimeBasedBuilder<T> timeBasedBuilder(String name, T obj, ToDoubleFunction<T> f, TimeUnit sourceUnit) { ... }

Other than providing an option with Duration, I think we should still provide an option with a ToDoubleFunction + TimeUnit since one tricky part with Duration is that it only accepts long values so if for some reason someone only have double, that conversion must be made (right now we can avoid this conversion by using our own TimeUtils.convert that works with double).

For the straightforward conversions to Duration (long -> Duration), we could provide a helper, like:

static <T> Function<T, Duration> toDuration(Function<T, Long> f, TemporalUnit sourceUnit) { ... }

So users can do this:

FunctionCounter.timeBasedBuilder("test", Demo.class, toDuration(Demo::getElapsedTimeInNanos, ChronoUnit.NANOS));

But now that I just wrote this down, that helper seems unnecessarry if we provide the extra option above (ToDoubleFunction<T> f, TimeUnit sourceUnit):

FunctionCounter.timeBasedBuilder("test", Demo.class, Demo::getElapsedTimeInNanos, TimeUnit.NANOSECONDS);

[shakuzen]

As discussed offline, if we want to go this route, I think the changes need to wait for 1.18.0-M1 so we have more time to give this the thought it deserves and leave room for feedback. RC1 is too late to introduce that much new public API.