XComp commented on code in PR #19555:
URL: https://github.com/apache/flink/pull/19555#discussion_r858357105


##########
flink-metrics/flink-metrics-prometheus/src/test/java/org/apache/flink/metrics/prometheus/PrometheusReporterTaskScopeTest.java:
##########
@@ -52,228 +43,159 @@
  * different subtasks.
  */
 class PrometheusReporterTaskScopeTest {
-    private static final String[] LABEL_NAMES = {
-        "job_id",
-        "task_id",
-        "task_attempt_id",
-        "host",
-        "task_name",
-        "task_attempt_num",
-        "job_name",
-        "tm_id",
-        "subtask_index"
-    };
+    private static final String[] LABEL_NAMES = {"label1", "label2"};
+    private static final String[] LABEL_VALUES_1 = new String[] {"value1_1", 
"value1_2"};
+    private static final String[] LABEL_VALUES_2 = new String[] {"value2_1", 
"value2_2"};
+    private static final String LOGICAL_SCOPE = "logical_scope";
+    private static final String METRIC_NAME = "myMetric";
+
+    private final MetricGroup metricGroup1 =
+            TestUtils.createTestMetricGroup(
+                    LOGICAL_SCOPE, TestUtils.toMap(LABEL_NAMES, 
LABEL_VALUES_1));
+    private final MetricGroup metricGroup2 =
+            TestUtils.createTestMetricGroup(
+                    LOGICAL_SCOPE, TestUtils.toMap(LABEL_NAMES, 
LABEL_VALUES_2));
 
-    private static final String TASK_MANAGER_HOST = "taskManagerHostName";
-    private static final String TASK_MANAGER_ID = "taskManagerId";
-    private static final String JOB_NAME = "jobName";
-    private static final String TASK_NAME = "taskName";
-    private static final int ATTEMPT_NUMBER = 0;
-    private static final int SUBTASK_INDEX_1 = 0;
-    private static final int SUBTASK_INDEX_2 = 1;
-
-    private final JobID jobId = new JobID();
-    private final JobVertexID taskId1 = new JobVertexID();
-    private final ExecutionAttemptID taskAttemptId1 = new ExecutionAttemptID();
-    private final String[] labelValues1 = {
-        jobId.toString(),
-        taskId1.toString(),
-        taskAttemptId1.toString(),
-        TASK_MANAGER_HOST,
-        TASK_NAME,
-        "" + ATTEMPT_NUMBER,
-        JOB_NAME,
-        TASK_MANAGER_ID,
-        "" + SUBTASK_INDEX_1
-    };
-    private final JobVertexID taskId2 = new JobVertexID();
-    private final ExecutionAttemptID taskAttemptId2 = new ExecutionAttemptID();
-    private final String[] labelValues2 = {
-        jobId.toString(),
-        taskId2.toString(),
-        taskAttemptId2.toString(),
-        TASK_MANAGER_HOST,
-        TASK_NAME,
-        "" + ATTEMPT_NUMBER,
-        JOB_NAME,
-        TASK_MANAGER_ID,
-        "" + SUBTASK_INDEX_2
-    };
-
-    private TaskMetricGroup taskMetricGroup1;
-    private TaskMetricGroup taskMetricGroup2;
-
-    private MetricRegistryImpl registry;
     private PrometheusReporter reporter;
 
     @BeforeEach
     void setupReporter() {
-        registry =
-                new MetricRegistryImpl(
-                        
MetricRegistryTestUtils.defaultMetricRegistryConfiguration(),
-                        Collections.singletonList(createReporterSetup("test1", 
"9400-9500")));
-        reporter = (PrometheusReporter) registry.getReporters().get(0);
-
-        TaskManagerMetricGroup tmMetricGroup =
-                TaskManagerMetricGroup.createTaskManagerMetricGroup(
-                        registry, TASK_MANAGER_HOST, new 
ResourceID(TASK_MANAGER_ID));
-        taskMetricGroup1 =
-                tmMetricGroup
-                        .addJob(jobId, JOB_NAME)
-                        .addTask(
-                                taskId1,
-                                taskAttemptId1,
-                                TASK_NAME,
-                                SUBTASK_INDEX_1,
-                                ATTEMPT_NUMBER);
-
-        taskMetricGroup2 =
-                tmMetricGroup
-                        .addJob(jobId, JOB_NAME)
-                        .addTask(
-                                taskId2,
-                                taskAttemptId2,
-                                TASK_NAME,
-                                SUBTASK_INDEX_2,
-                                ATTEMPT_NUMBER);
+        reporter = new 
PrometheusReporter(NetUtils.getPortRangeFromString("9400-9500"));
     }
 
     @AfterEach
-    void shutdownRegistry() throws Exception {
-        if (registry != null) {
-            registry.shutdown().get();
+    void tearDown() {
+        if (reporter != null) {
+            reporter.close();
         }
     }
 
     @Test
-    void countersCanBeAddedSeveralTimesIfTheyDifferInLabels() throws 
UnirestException {
+    void countersCanBeAddedSeveralTimesIfTheyDifferInLabels() {
         Counter counter1 = new SimpleCounter();
         counter1.inc(1);
         Counter counter2 = new SimpleCounter();
         counter2.inc(2);
 
-        taskMetricGroup1.counter("my_counter", counter1);
-        taskMetricGroup2.counter("my_counter", counter2);
+        reporter.notifyOfAddedMetric(counter1, METRIC_NAME, metricGroup1);
+        reporter.notifyOfAddedMetric(counter2, METRIC_NAME, metricGroup2);
 
         assertThat(
                         CollectorRegistry.defaultRegistry.getSampleValue(
-                                "flink_taskmanager_job_task_my_counter", 
LABEL_NAMES, labelValues1))
+                                getLogicalScope(METRIC_NAME), LABEL_NAMES, 
LABEL_VALUES_1))
                 .isEqualTo(1.);
         assertThat(
                         CollectorRegistry.defaultRegistry.getSampleValue(
-                                "flink_taskmanager_job_task_my_counter", 
LABEL_NAMES, labelValues2))
+                                getLogicalScope(METRIC_NAME), LABEL_NAMES, 
LABEL_VALUES_2))
                 .isEqualTo(2.);
     }
 
     @Test
-    void gaugesCanBeAddedSeveralTimesIfTheyDifferInLabels() throws 
UnirestException {
-        Gauge<Integer> gauge1 =
-                new Gauge<Integer>() {
-                    @Override
-                    public Integer getValue() {
-                        return 3;
-                    }
-                };
-        Gauge<Integer> gauge2 =
-                new Gauge<Integer>() {
-                    @Override
-                    public Integer getValue() {
-                        return 4;
-                    }
-                };
+    void gaugesCanBeAddedSeveralTimesIfTheyDifferInLabels() {
+        Gauge<Integer> gauge1 = () -> 3;
+        Gauge<Integer> gauge2 = () -> 4;
 
-        taskMetricGroup1.gauge("my_gauge", gauge1);
-        taskMetricGroup2.gauge("my_gauge", gauge2);
+        reporter.notifyOfAddedMetric(gauge1, METRIC_NAME, metricGroup1);
+        reporter.notifyOfAddedMetric(gauge2, METRIC_NAME, metricGroup2);
 
         assertThat(
                         CollectorRegistry.defaultRegistry.getSampleValue(
-                                "flink_taskmanager_job_task_my_gauge", 
LABEL_NAMES, labelValues1))
+                                getLogicalScope(METRIC_NAME), LABEL_NAMES, 
LABEL_VALUES_1))
                 .isEqualTo(3.);
         assertThat(
                         CollectorRegistry.defaultRegistry.getSampleValue(
-                                "flink_taskmanager_job_task_my_gauge", 
LABEL_NAMES, labelValues2))
+                                getLogicalScope(METRIC_NAME), LABEL_NAMES, 
LABEL_VALUES_2))
                 .isEqualTo(4.);
     }
 
     @Test
-    void metersCanBeAddedSeveralTimesIfTheyDifferInLabels() throws 
UnirestException {
-        Meter meter = new TestMeter();
+    void metersCanBeAddedSeveralTimesIfTheyDifferInLabels() {
+        Meter meter1 = new TestMeter(1, 1.0);
+        Meter meter2 = new TestMeter(2, 2.0);
 
-        taskMetricGroup1.meter("my_meter", meter);
-        taskMetricGroup2.meter("my_meter", meter);
+        reporter.notifyOfAddedMetric(meter1, METRIC_NAME, metricGroup1);
+        reporter.notifyOfAddedMetric(meter2, METRIC_NAME, metricGroup2);
 
         assertThat(
                         CollectorRegistry.defaultRegistry.getSampleValue(
-                                "flink_taskmanager_job_task_my_meter", 
LABEL_NAMES, labelValues1))
-                .isEqualTo(5.);
+                                getLogicalScope(METRIC_NAME), LABEL_NAMES, 
LABEL_VALUES_1))
+                .isEqualTo(meter1.getRate());
         assertThat(
                         CollectorRegistry.defaultRegistry.getSampleValue(
-                                "flink_taskmanager_job_task_my_meter", 
LABEL_NAMES, labelValues2))
-                .isEqualTo(5.);
+                                getLogicalScope(METRIC_NAME), LABEL_NAMES, 
LABEL_VALUES_2))
+                .isEqualTo(meter2.getRate());
     }
 
     @Test
     void histogramsCanBeAddedSeveralTimesIfTheyDifferInLabels() throws 
UnirestException {
-        Histogram histogram = new TestHistogram();
+        TestHistogram histogram1 = new TestHistogram();
+        histogram1.setCount(1);
+        TestHistogram histogram2 = new TestHistogram();
+        histogram2.setCount(2);
 
-        taskMetricGroup1.histogram("my_histogram", histogram);
-        taskMetricGroup2.histogram("my_histogram", histogram);
+        reporter.notifyOfAddedMetric(histogram1, METRIC_NAME, metricGroup1);
+        reporter.notifyOfAddedMetric(histogram2, METRIC_NAME, metricGroup2);
 
         final String exportedMetrics = 
pollMetrics(reporter.getPort()).getBody();
-        assertThat(exportedMetrics)
-                .contains("subtask_index=\"0\",quantile=\"0.5\",} 0.5"); // 
histogram
-        assertThat(exportedMetrics)
-                .contains("subtask_index=\"1\",quantile=\"0.5\",} 0.5"); // 
histogram
+        assertThat(exportedMetrics).contains("label2=\"value1_2\",} 1.0"); // 
histogram
+        assertThat(exportedMetrics).contains("label2=\"value2_2\",} 2.0"); // 
histogram

Review Comment:
   ```suggestion
           assertThat(exportedMetrics).contains("label2=\"value1_2\",} 1.0");
           assertThat(exportedMetrics).contains("label2=\"value2_2\",} 2.0");
   ```
   nit: I know that the comments were also part of my proposal. But they really 
don't add any value...



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to