eclipse / microprofile-metrics

microprofile-metrics
Apache License 2.0
100 stars 66 forks source link

Remove existing MetricRegistry's retrieve/register gauge() methods and introduce new ones that make use of Functions #598

Closed Channyboy closed 4 years ago

Channyboy commented 4 years ago

See #592

Was going to add the below methods without tag parameters but would cause ambiguity issues.


1. <T> Gauge<T> gauge(String name, Supplier<T> supplier);
2. <T> Gauge<T> gauge(Metadata metadata, Supplier<T> supplier);

3. <T> Gauge<Long> gauge(String name, T object, ToDoubleFunction<T> func;
4. <T> Gauge<Long> gauge(Metadata metadata, T object, ToDoubleFunction<T> func;

For example, wanting to calling gauge("name", supplier, null) is ambiguous with 3 and similarily calling gauge(metadata, supplier, null) is ambigious with 4 Signed-off-by: David Chan chdavid@ca.ibm.com

jmartisk commented 4 years ago

cc @ebullient

jmartisk commented 4 years ago

Sorry that it did not occur to me sooner, but I guess we could go without forcing the gauges to be Double-only by using Function instead of ToDoubleFunction, so for example this

<T> Gauge<Double> gauge(MetricID metricID, T object, ToDoubleFunction<T> func);

would become

<T, R extends Number> Gauge<R> gauge(MetricID metricID, T object, Function<T, R> func);

where T is the state object and R is the return type of the gauge, and R extends Number, so it can be a Long as well as a Double

@ebullient, @Channyboy would this make sense to you? Not sure if it might make something more complicated for Micrometer integration, but from an API perspective it's much better.

ebullient commented 4 years ago

Stackdriver uses doubles. That's where ToDoubleFunction comes from. It's easy to cast long/int/whatever to double, it is hard to go the other way.

EDIT of Edits. I was wrong all over the places. ;)

This is fine, provided you can match T and R properly. The comments need to be updated. I've tried this w/ Quarkus MetricsFactory and it works

https://github.com/quarkusio/quarkus/pull/10626/files#diff-4ab3b880aaf9863b27db515136e03bb2R89