Rest utils metrics #23
Conversation
Most importantly, this changes the default prefix for the metrics. However, the bulk of the change makes minor adjustments to work with the common-utils Time interface.
There was a problem hiding this comment.
@ewencp To speed up review, could you post the jmx metric name that is created as a result of this annotation. This example should suffice to understand the rest.
There was a problem hiding this comment.
Sure. This generates:
- brokers.list.request-rate
- brokers.list.request-byte-rate
- brokers.list.request-size-avg
- brokers.list.request-size-max
- brokers.list.response-rate
- brokers.list.response-byte-rate
- brokers.list.response-size-avg
- brokers.list.response-size-max
- brokers.list.request-latency-avg
- brokers.list.request-latency-max
- brokers.list.request-error-rate
That's in addition to the global ones for Jersey (same as above, but without the brokers.list prefix) and global ones for Jetty:
- connections-accepted-rate
- connections-opened-rate
- connections-closed-rate
- connections-active
All the Jersey ones are collected under a "jersey-metrics" group and Jetty ones under a "jetty-metrics" group.
There was a problem hiding this comment.
@ewencp These look good. What do you think about 90, 99 and 999 percentiles for some of these, like for example the latency and byte-rate ones. One potential risk is that we don't know how well the percentiles work in common-metrics.
There was a problem hiding this comment.
That's actually exactly why I didn't add them in :) The new producer code only seemed to use avg and max, so I stuck with that. But I agree, 90, 99, and 999 would be great.
There was a problem hiding this comment.
Ok. Let's just leave it then and come back to it when we are slightly more confident of the quality of the percentiles :)
Other than that, this patch looks good.
Conflicts: src/main/java/io/confluent/kafkarest/KafkaRestConfig.java
Add support for metrics generated automatically by rest-utils. This required a few updates due to changes to the framework. Blocked by confluentinc/rest-utils#6