[jira] [Comment Edited] (FLINK-5095) Add explicit notifyOfAddedX methods to MetricReporter interface

["Chesnay Schepler (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/FLINK-5095?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15974410#comment-15974410 ] 

Chesnay Schepler edited comment on FLINK-5095 at 4/19/17 10:08 AM:
-------------------------------------------------------------------

My current thinking goes i the direction to abstract away the specific metric type for the reporter. In a way, counters, meters and Gauge<Number> can all be handled identically, like a Gauge<Number>. Similarly, all remaining Gauges<!Number> can be handled like a Gauge<String>. The Histogram can be generalized into a Multi-Gauge<Number>, which is useful anyway.

This would reduce the total number of metric types that the reporter have to deal with to 3. This number also wouldn't necessarily increase over time; a Timer for example would also just be a Gauge<Number>.


was (Author: zentol):
My current thinking goes i the direction to abstract away the specific metric type. In a way, counters, meters and Gauge<Number> can all be handled identically, like a Gauge<Number>. Similarly, all remaining Gauges<!Number> can be handled like a Gauge<String>. The Histogram can be generalized into a Multi-Gauge<Number>, which is useful anyway.

This would reduce the total number of metric types that the reporter have to deal with to 3. This number also wouldn't necessarily increase over time; a Timer for example would also just be a Gauge<Number>.

> Add explicit notifyOfAddedX methods to MetricReporter interface
> ---------------------------------------------------------------
>
>                 Key: FLINK-5095
>                 URL: https://issues.apache.org/jira/browse/FLINK-5095
>             Project: Flink
>          Issue Type: Improvement
>          Components: Metrics
>    Affects Versions: 1.1.3
>            Reporter: Chesnay Schepler
>            Priority: Minor
>
> I would like to start a discussion on the MetricReporter interface, specifically the methods that notify a reporter of added or removed metrics.
> Currently, the methods are defined as follows:
> {code}
> void notifyOfAddedMetric(Metric metric, String metricName, MetricGroup group);
> void notifyOfRemovedMetric(Metric metric, String metricName, MetricGroup group);
> {code}
> All metrics, regardless of their actual type, are passed to the reporter with these methods.
> Since the different metric types have to be handled differently we thus force every reporter to do something like this:
> {code}
> if (metric instanceof Counter) {
>         Counter c = (Counter) metric;
> 	// deal with counter
> } else if (metric instanceof Gauge) {
> 	// deal with gauge
> } else if (metric instanceof Histogram) {
> 	// deal with histogram
> } else if (metric instanceof Meter) {
> 	// deal with meter
> } else {
> 	// log something or throw an exception
> }
> {code}
> This has a few issues
> * the instanceof checks and castings are unnecessary overhead
> * it requires the implementer to be aware of every metric type
> * it encourages throwing an exception in the final else block
> We could remedy all of these by reworking the interface to contain explicit add/remove methods for every metric type. This would however be a breaking change and blow up the interface to 12 methods from the current 4. We could also add a RichMetricReporter interface with these methods, which would require relatively little changes but add additional complexity.
> I was wondering what other people think about this.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)