flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bowen Li (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (FLINK-5095) Add explicit notifyOfAddedX methods to MetricReporter interface
Date Wed, 15 Mar 2017 18:12:41 GMT

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

Bowen Li commented on FLINK-5095:

Agree that MetricReporter interface is not extendable and scalable, and will definitely causing
problems while Flink's metrics and monitoring systems grow.

I'll take some time to think about this as well as https://issues.apache.org/jira/browse/FLINK-6053

> 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

View raw message