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

["Bowen Li (JIRA)" <ji...@apache.org>]

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

Bowen Li commented on FLINK-5095:
---------------------------------

[~Zentol] Can you list your proposed interfaces here? Want to make sure we are in the same page for the discussion.

> 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)