Micrometer: Add Dropwizard like annotations

[paul-ovchinnikov]

Add annotations like in Dropwizard

@Metered
@ExceptionMetered
@Counted
@Gauge
@CachedGauge
@Metric
@ResponseMetered

and Spring support for that

[mweirauch]

I've never been using any of these with Dropwizard-Metrics, so bear with me for not getting anything wrong during this assessment.

• @ResponseMetered: Should be covered with the Spring Web, Webflux and Jersey instrumentation support.
• @Metered/Counted: I can't figure the difference; Could be a sibbling to Micrometers @Timed. Allthough the credo is: When you can count something, you can also time it. (At least I've read that numerous times and I can only agree with that.)
• @Gauge/@CachedGauge: Convenience over building a function based Meter yourself. Yeah.
• @Metric: A convenience over using the fluent Meter Builders from Micrometers API if I get that right. Dunno if it's worthwhile when you can just build these - with more control - in the constructor of the holding class yourself.
• @ExceptionMetered: Instead of just metering for specific exceptions, wouldn't you wan't to always meter the invocations (ideally time) and not just when exceptions occur?

The last point brings me to: Shouldn't the Micrometer TimedAspect track exceptions by default too like we do with the web based instrumentation where we ship a tag with exception="NONE" in case all is good? TimedAspect is still incubating and thus marked experimental.

[gavlyukovskiy]

I was also thinking about easier way to create metrics as we're going to have a lot of metrics which will look exactly the same except metric name.
My first idea was to create annotations for easier binding fields to metrics:

@BindGauge(name = "my.gauge", tags = { "Tag", "Value" })
private AtomicInteger myGauge;
@BindCounter(name = "my.counter", tags = { "Tag", "Value" })
private Counter myCounter;

which will be injected by bean post processor and then registered in MeterRegistry.
Now you have to inject MeterRegistry and register all of them there or, which is worst, implement MeterBinder and split creating metrics and binding them into registry. Those annotation would help to not get into such details.

[jkschneider]

@gavlyukovskiy Is this easier than:

private AtomicInteger myGauge = Metrics.gauge("my.gauge", Tags.of("Tag", "Value"), new AtomicInteger(0));
private Counter myCounter = Metrics.counter("my.counter", Tags.of("Tag", "Value"));

??

The latter doesn't require any AOP magic.

[gavlyukovskiy]

Oh, I didn't know about this static class, I was injecting MeterRegistry and did this ugly stuff. This one looks very easy. Is it safe to do it during class initialization?

P.S. there is no AOP magic needed here, just BPP and pinch of reflection :)

[jkschneider]

Yep, safe to do during class init. Ultimately, the composite that Spring configures is added to the global composite provided that management.metrics.use-global-registry is not turned off. After that, the chain is like:

global CompositeMeterRegistry -> spring configured CompositeMeterRegistry -> specific implementations

Anything meters registered against a composite before a child is added are added to the child as soon as that link is made.

[jkschneider]

P.S. there is no AOP magic needed here, just BPP and pinch of reflection :)

Fair point :)

[shivaniverma606]

Do you have any working example about how to implement metrics for REST APIs monitoring

[jozsefszabo1]

Annotation based usage is cleaner because don't mess up the code and the test code with technical dependencies and unnecessary complexity.

[jonatan-ivanov]

My two cents:

@ResponseMetered: I agree, it should be covered with the Spring Web, Webflux and Jersey instrumentation support.

@Metered/Counted:
@Counted creates a Counter which is just a single value, looks like this:

myCounter: {
    count: 58
}

@Metered creates a Meter, which is a counter plus a couple of rates (main, 1-5-15 minutes rolling windows), looks like this:

myMeter: {
    count: 0,
    m15_rate: 0,
    m1_rate: 0,
    m5_rate: 0,
    mean_rate: 0,
    units: "events/second"
}

@Timed creates a Timer which is a meter + durations (min, max, mean, std deviation, TP values), looks like this:

timers: {
myTimer: {
    count: 0,
    max: 0,
    mean: 0,
    min: 0,
    p50: 0,
    p75: 0,
    p95: 0,
    p98: 0,
    p99: 0,
    p999: 0,
    stddev: 0,
    m15_rate: 0,
    m1_rate: 0,
    m5_rate: 0,
    mean_rate: 0,
    duration_units: "milliseconds",
    rate_units: "calls/second"
}

I think Micrometer's @Timed should be able to cover these.

@Gauge/@CachedGauge: These are pretty nice, especally the cached gauge. Does Micrometer provide a cached gauge?

@ExceptionMetered: @Timed could cover this with an exception flag.

[jkschneider]

• @ResponseMetered - Unnecessary. Framework instrumentation already adds status codes to default timings, among other tags. This was only really necessary in Dropwizard because it lacked dimensional metrics. In a hierarchical world timing and response status counting have to be collected separately.
• @Metered/Counted - Unnecessary. We have this principal: "never gauge something you can count, never count something you can time." Since Micrometer Timers ship counts, there is no reason to limit yourself to a Counter when decorating a method call. The distinction between @Metered and @Counted is also unimportant. When Micrometer ships counts to hierarchical systems for which pre-aggregated moving window rates are helpful, those rates are shipped. Most monitoring systems don't benefit from pre-aggregated rates.
• @Timed - Already covered.
• @Gauge/@CachedGauge - Unnecessary. Micrometer gauges are always defined according to a ToDoubleFunction that samples instantaneous values on publish. Dropwizard gauges required setting values as the underlying thing changed, resulting in many intermediate values that were never observed by the monitoring system. With the addition of a Supplier-based strong reference gauge in Micrometer 1.1.0, there is no reason for us to hang a @Gauge on a method. When the instantaneous value is determined from an expensive call such as a DB query, it is likely MultiGauge is the construct that will be used.

This leaves us with:

• @ExceptionMetered - As @jonatan-ivanov suggests, an exception flag on @Timed is probably all that we need to do.

[jkschneider]

Annotation based usage is cleaner because don't mess up the code and the test code with technical dependencies and unnecessary complexity.

@jozsefszabo1 I don't follow. An annotation must come from some package, which is going to be the same binary dependency as declarative Metrics.counter(..), etc. I don't see how either one is relevant to test code. Metrics.xxx uses an empty global composite registry that is a no-op in the default case.

[jkschneider]

P.S. there is no AOP magic needed here, just BPP and pinch of reflection :)

@gavlyukovskiy Reflecting once more on this: BPP assumes a Spring implementation. Micrometer isn't Spring-specific. I don't see how to do it generally without AOP. I'm not keen on making each framework that uses Micrometer implement something like BPP just to satisfy an annotation-driven metering approach that is just as easily satisfied declaratively.

[jozsefszabo1]

Annotation based usage is cleaner because don't mess up the code and the test code with technical dependencies and unnecessary complexity.

@jozsefszabo1 I don't follow. An annotation must come from some package, which is going to be the same binary dependency as declarative Metrics.counter(..), etc. I don't see how either one is relevant to test code. Metrics.xxx uses an empty global composite registry that is a no-op in the default case.

@jkschneider
Annotation requires compile dependency on interfaces and runtime for implementation,
Static declarative call creates more stronger connections and strong connascence between the classes.
Static calls add the complexity and fixture of the Metrics.xxx method to the embedding method, had side effects, Modifies the context of the test. you should test it with your code, you shoe see after side effects.
So with the code which using static calls you hurts the single responsibility principle and you should test metering and the main function of the method, otherwise the checking of the metering is ambiguous.
So I think to using annotations had several good benefits over the declarative call. Also makes adding annotations makes the upgrade to micmometer from dropwizard to easier.

[Fetsivalen]

In the case of adding exception counter to @Timed annotation, it seems to be usefull to record time only on success case.
As far positive and exceptional flow could take different amount of time.
It seems like a breaking change, but needed one.
Moreover I think that exception counter could be enabled by default, as far this isn't a breaking change.
@jkschneider what do you think?

[mweirauch]

@Fetsivalen the TimedAspect in the 1.1.x line is already taking care of catching the exception in a tag. Thus you will get different metrics (because the tags are part of the metric identification) on which you can reason on regarding success/failure.

Here's a sample (in Prometheus style exposition):

# HELP timedservice_behave_seconds_max  
# TYPE timedservice_behave_seconds_max gauge
timedservice_behave_seconds_max{application="micrometered2",class="io.github.mweirauch.micrometered2.service.TimedService",exception="none",method="behave",} 0.065967173
timedservice_behave_seconds_max{application="micrometered2",class="io.github.mweirauch.micrometered2.service.TimedService",exception="RuntimeException",method="behave",} 5.94422E-4
# HELP timedservice_behave_seconds  
# TYPE timedservice_behave_seconds summary
timedservice_behave_seconds_count{application="micrometered2",class="io.github.mweirauch.micrometered2.service.TimedService",exception="none",method="behave",} 1.0
timedservice_behave_seconds_sum{application="micrometered2",class="io.github.mweirauch.micrometered2.service.TimedService",exception="none",method="behave",} 0.065967173
timedservice_behave_seconds_count{application="micrometered2",class="io.github.mweirauch.micrometered2.service.TimedService",exception="RuntimeException",method="behave",} 1.0
timedservice_behave_seconds_sum{application="micrometered2",class="io.github.mweirauch.micrometered2.service.TimedService",exception="RuntimeException",method="behave",} 5.94422E-4

[jkschneider]

@Fetsivalen You are absolutely correct that successful/non-successful times can be radically different. As @mweirauch shows, a dimensional monitoring system will allow you to drill down on successes/failures to see the timing contributions of each.

Again, since timers already contain counts, there is no need to separately count exceptions.

[Fetsivalen]

@mweirauch, @jkschneider
Thanks a lot, seems I've checked an older version of Timed aspect, and also i'd underestimate using of tags in the case.

[jkschneider]

Static calls add the complexity and fixture of the Metrics.xxx method to the embedding method, had side effects, Modifies the context of the test. you should test it with your code, you shoe see after side effects.

@jozsefszabo1 I'd genuinely like to hear more about what side effects it causes in your code. I have a hard time understanding how a no-op composite has any effect at all.

So with the code which using static calls you hurts the single responsibility principle

So far, I'm not buying an argument about single responsibility for two reasons:

1. The component has precisely the same number of responsibilities whether monitoring is accomplished via annotation or declaratively.
2. Monitoring is a cross-cutting concern no different than logging, and you don't log entirely with annotations on account of "single responsibility principle".

Also makes adding annotations makes the upgrade to micmometer from dropwizard to easier.

I have more sympathy for this point, but I think it is important for folks to consider what is different between a hierarchical instrumentation library like Dropwizard (at least historically hierarchical) and a dimensional one like Micrometer. We've mentioned a few here, but for example consider @Counted. We want to actively _discourage_ counting method bodies because you can do one better by timing them (which contains a count).

Convince me otherwise :)

[jozsefszabo1]

Convince me otherwise :)

I will try :-),
Without micrometer annotations In practice I expecting lots of mixed applications, changing to micrometer only dependency will requires more complex (manual) changes and new unit tests. I expecting lots of team will make his own annotations. I know this is the strongest practical reason to add the annotations and the existence of these annotations are trade-offs.

[jonatan-ivanov]

@jkschneider Do you have any plans for the release date for 1.1.x?
Or is there any chance to merge the changes for the exception flag back to 1.0.x?

https://github.com/micrometer-metrics/micrometer/blob/master/micrometer-core/src/main/java/io/micrometer/core/aop/TimedAspect.java#L84

(I would be happy to create a PR as needed)

[jkschneider]

@jonatan-ivanov It needs to be released before Oct 26 in order to make it into Spring Boot 2.1.0. So that is a pretty firm deadline.

I'd like to avoid merging feature updates back into 1.0.x, if that's ok.

[jonatan-ivanov]

@jkschneider Cool, thanks! :)

[jkschneider]

Without micrometer annotations In practice I expecting lots of mixed applications, changing to micrometer only dependency will requires more complex (manual) changes and new unit tests.

I understand the concern to a point. Changing an annotation to a programmatic call isn't just an import change. I haven't heard a convincing argument why unit tests need to change at all.

Unfortunately I don't feel like this justifies more annotations. I think folks really need to consider whether their instrumentation makes sense in a dimensional world when coming from Dropwizard rather than just changing package names on annotations. Seems like a painful but necessary step.

[paul-ovchinnikov]

Ok, let's forget about DropWizard and its weird architecture.
The main concern about micrometer that it's a Pivotal project and a Spring Framework is the main project of Pivotal too.
So why not to make micrometer more useful to integrate with Spring. Not as part of micrometer but as an additional bridge library 'micrometer-spring' for example.
With annotations and BPP or static generator and blackjack. Annotations definitely make development easier and code cleaner.
This issue the most discussed in micrometer project, but you just closed it. Maybe it is worth extending a little more attention to this issue and consider it up with your colleagues?

[jkschneider]

@paul-ovchinnikov There are separate issues open for extending @Timed integration further, for JMS and for general purpose methods. I closed this issue because I don't see any reason to add any further annotations. We've carefully looked at every other annotation in the Dropwizard package and don't see any that are helpful.