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_r261541662
##########
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:
We had a similar discussion back when we added the pushgateway reporter.
The gist is that 2)/3) are not desirable since it makes the internals of the
reporter rather complex (additionally for 2) the question remains what to do
with clusters that aren't running a job) and 1) requires additional work
outside of the metric system first to actually identify clusters. As of right
now there's no concept of a cluster ID _anywhere_, and it doesn't make sense to
only introduce it for the Prometheus reporter specifically. The current
implementation for `jobName` was very much just a compromise.
The thing is the following: Ultimately a user can already group metrics in
any way he/she desires. The jobName/instance stuff is, for our purposes,
exclusively used to prevent conflicts between different containers. That's it,
there's no semantic value in them since they (will, after FLINK-9543) only
duplicate the container id label that we attach anyway for consistency with
other reporters.
As such it isn't necessary to introduce additional complexity or significant
reporter-specific logic.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
