JTaky commented on code in PR #27744:
URL: https://github.com/apache/flink/pull/27744#discussion_r3009606186
##########
flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java:
##########
@@ -32,13 +33,16 @@
import java.io.IOException;
import java.net.URL;
+import java.util.LinkedHashMap;
import java.util.Map;
/**
* {@link MetricReporter} that exports {@link Metric Metrics} via Prometheus
{@link PushGateway}.
*/
@PublicEvolving
public class PrometheusPushGatewayReporter extends AbstractPrometheusReporter
implements Scheduled {
+ public static final String REPORTER_ID_GROUPING_KEY =
Review Comment:
do we need to make this value public? I feel that package private would
allow to use it freely and encapsulate from the usage in another context
##########
flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java:
##########
@@ -64,7 +69,10 @@ public class PrometheusPushGatewayReporter extends
AbstractPrometheusReporter im
this.basicAuthEnabled = false;
}
this.jobName = Preconditions.checkNotNull(jobName);
- this.groupingKey = Preconditions.checkNotNull(groupingKey);
+ this.groupingKey =
+ metricsGroupingByReporter
+ ? groupWithReporterId(groupingKey)
+ : new LinkedHashMap<>(groupingKey);
Review Comment:
do we still need to wrap `groupingKey` into LinkedHashMap if new option
`metricsGroupingByReporter` is disabled? I think that groupingKey is not
modified later, thus LinkedHashMap could be miss-leading at least for human
reader.
--
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]
