zentol commented on a change in pull request #7820: [FLINK-11742][Metrics]Push
metrics to Pushgateway without "instance"
URL: https://github.com/apache/flink/pull/7820#discussion_r267339633
##########
File path:
flink-metrics/flink-metrics-prometheus/src/main/java/org/apache/flink/metrics/prometheus/PrometheusPushGatewayReporter.java
##########
@@ -73,7 +77,7 @@ public void open(MetricConfig config) {
@Override
public void report() {
try {
- pushGateway.push(CollectorRegistry.defaultRegistry,
jobName);
+ pushGateway.push(CollectorRegistry.defaultRegistry,
jobName, instance);
Review comment:
I get that, but I'm very much against introducing the concept of a
"cluster_id" in a single reporter.
There is value in having it, but I'd much rather have a a) solution that
works for all reporters and b) has value outside the metric system.
Note that this isn't really a metric issue as such an ID should also be
visible (and realistically contained) within each JM/TM/RM/Dispatcher who
forward this to the metric system as they do with other IDs.
When that is in place we can discuss whether we rework the jobName/instance
stuff. jobName could be the cluster ID, and instance the ID of the JM/TM/etc. .
However my feeling is that such a change would be unnecessary as the only
requirement is that _some_ label allows filtering by cluster ID, not
necessarily the `instance` one.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
