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

    https://github.com/apache/flink/pull/6211#discussion_r198460116
  
    --- Diff: 
flink-metrics/flink-metrics-prometheus/src/test/java/org/apache/flink/metrics/prometheus/PrometheusReporterTest.java
 ---
    @@ -159,6 +162,27 @@ public void histogramIsReportedAsPrometheusSummary() 
throws UnirestException {
                }
        }
     
    +   @Test
    +   public void metricIsRemovedWhenCollectorIsNotUnregisteredYet() throws 
UnirestException {
    +           TaskManagerMetricGroup tmMetricGroup = new 
TaskManagerMetricGroup(registry, HOST_NAME, TASK_MANAGER);
    +
    +           String metricName = "numRecordsOut";
    +
    +           Counter metric1 = new SimpleCounter();
    +           FrontMetricGroup<TaskManagerJobMetricGroup> metricGroup1 = new 
FrontMetricGroup<>(1, new TaskManagerJobMetricGroup(registry, tmMetricGroup, 
JobID.generate(), "job_1"));
    +           reporter.notifyOfAddedMetric(metric1, metricName, metricGroup1);
    +
    +           Counter metric2 = new SimpleCounter();
    +           FrontMetricGroup<TaskManagerJobMetricGroup> metricGroup2 = new 
FrontMetricGroup<>(2, new TaskManagerJobMetricGroup(registry, tmMetricGroup, 
JobID.generate(), "job_2"));
    --- End diff --
    
    nit: the first argument to `FrontMetricGroup` should be identical since 
you're passing it to the same reporter. Actually it shouldn't be necessary to 
create a `FrontMetricGroup` at all, as it is only need to introduce reporter 
specific configuration properties.


---

Reply via email to