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_r261411132
##########
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:
No, a Flink job does not correspond to a Prometheus job. This doesn't work
for session clusters (i.e. running multiple jobs), or for clusters that are
just idling. Even if it did you wouldn't want an arbitrary random ID (you
_never_ want random data) but something like the job ID.
As of right now we expect the jobName to be unique. It is thus safe to use
as the `instance` as well.
Actually, this begs the rather interesting question whether you've set the
jobName to some constant across multiple containers (as of right now, you
shouldn't!)
Alternatively the instance label should also be made configurable, in a
similar vein as the job label. In hindsight we made a mistake and should've
made the `jobName` configuration apply to `instance` instead; the more you know.
One could argue that `jobName` could be something like `dispatcher` or
`taskmanager`, with `instance` being the corresponding ID, but unfortunately
not all IDs are properly exposed (see
FLINK-9543)
----------------------------------------------------------------
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
