[GitHub] flink pull request #4586: [FLINK-7502] [metrics] Improve PrometheusReporter

[zentol <gi...@git.apache.org>]

Github user zentol commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4586#discussion_r144805511
  
    --- Diff: flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusReporter.java ---
    @@ -114,39 +120,78 @@ public void notifyOfAddedMetric(final Metric metric, final String metricName, fi
     			dimensionValues.add(CHARACTER_FILTER.filterCharacters(dimension.getValue()));
     		}
     
    -		final String validMetricName = scope + SCOPE_SEPARATOR + CHARACTER_FILTER.filterCharacters(metricName);
    -		final String metricIdentifier = group.getMetricIdentifier(metricName);
    +		final String scopedMetricName = getScopedName(metricName, group);
    +		final String helpString = metricName + " (scope: " + getLogicalScope(group) + ")";
    +
     		final Collector collector;
    -		if (metric instanceof Gauge) {
    -			collector = createGauge((Gauge) metric, validMetricName, metricIdentifier, dimensionKeys, dimensionValues);
    -		} else if (metric instanceof Counter) {
    -			collector = createGauge((Counter) metric, validMetricName, metricIdentifier, dimensionKeys, dimensionValues);
    -		} else if (metric instanceof Meter) {
    -			collector = createGauge((Meter) metric, validMetricName, metricIdentifier, dimensionKeys, dimensionValues);
    -		} else if (metric instanceof Histogram) {
    -			collector = createSummary((Histogram) metric, validMetricName, metricIdentifier, dimensionKeys, dimensionValues);
    +		Integer count = 0;
    +
    +		if (!collectorsWithCountByMetricName.containsKey(scopedMetricName)) {
    +			if (metric instanceof Gauge) {
    +				collector = newGauge(scopedMetricName, helpString, dimensionKeys, dimensionValues, gaugeFrom((Gauge) metric));
    --- End diff --
    
    We can simplify this block a little bit by not registering the metric right away.
    
    Instead of
    
    ```
    if(!collectorExists) {
    	//create collector and add metric
    } else {
    	//get collector and add metric
    }
    ```
    
    do
    
    ```
    if(!collectorExists) {
    	//create collector
    } else {
    	//get collector
    }
    add metric
    ```


---