PrometheusMeterRegistry not exporting all data

[rdehuyss]

Describe the bug
Hi there! I'm Ronald, the author of JobRunr.

For JobRunr Pro, we've added support for MicroMeter. We noticed that when using the PrometheusMeterRegistry, only the first timer is exported.

Is this related to #877?

Environment

• Micrometer version: 1.11.0
• Micrometer registry: prometheus
• OS: macOS
• Java version: openjdk 17.0.1 2021-10-19, OpenJDK Runtime Environment (build 17.0.1+12-39), OpenJDK 64-Bit Server VM (build 17.0.1+12-39, mixed mode, sharing)

To Reproduce
How to reproduce the bug:

Timer.Builder timerBuilder = Timer.builder("jobrunr.jobs.all")
                .description("JobRunr All Jobs")
                .tag("job.signature", job.getJobSignature())
                .tag("job.server-tag", job.getServerTag());
// jobs don't always have a dynamic queue.
if (isNotNullOrEmpty(job.getDynamicQueue())) {
        timerBuilder.tag("job.dynamic-queue", job.getDynamicQueue());
}
timerBuilder.register(meterRegistry)

The important part to note is that the job.getJobSignature() is different for each job.

In Spring Boot, I see the correct info using the /actuator/metrics endpoint yet the /actuator/prometheus/ endpoint is only showing the first recorded jobSignature. This contradicts the documentation which says:

Add the timer to a single registry, or return an existing timer in that registry. The returned timer will be unique for each registry, but each registry is guaranteed to only create one timer for the same combination of name and tags.

Expected behavior
Prometheus also showing all the different timers (so also with other tags)

Additional context
Perhaps related to #877?

This is quite an important issue in my point of view as it makes the PrometheusMeterRegistry not usable.

[jonatan-ivanov]

Hi,

Having different set of tag keys for the same metric name is not recommended by the Prometheus server and as far as I remember, the Prometheus Java client also has issues, so this:

jobrunr_jobs_count{server="A", queue="Q1"} 1.0
jobrunr_jobs_count{server="A"} 2.0

should look like this instead:

jobrunr_jobs_count{server="A", queue="Q1"} 1.0
jobrunr_jobs_count{server="A", queue="none"} 2.0

So instead of this:

if (isNotNullOrEmpty(job.getDynamicQueue())) {
        timerBuilder.tag("job.dynamic-queue", job.getDynamicQueue());
}

You should do something like this:

if (isNotNullOrEmpty(job.getDynamicQueue())) {
    timerBuilder.tag("job.dynamic-queue", job.getDynamicQueue());
}
else {
    timerBuilder.tag("job.dynamic-queue", "none");
}

WIth this let me close the issue, please let us know if this does not work and/or I misunderstood something and we can reopen.

[rdehuyss]

Hi @jonatan-ivanov ! Thanks for the feedback, that's what I've done at the end.

Would it be possible to add a warning message if a different set of tag keys for the same metric name is being used? Or add it in the documentation when registering a counter/meter in the registry?

[jonatan-ivanov]

We send out a notification about it:

micrometer/implementations/micrometer-registry-prometheus/src/main/java/io/micrometer/prometheus/PrometheusMeterRegistry.java

Lines 590 to 596 in 8d18d8c

	meterRegistrationFailed(id,
	"Prometheus requires that all meters with the same name have the same"
	+ " set of tag keys. There is already an existing meter named '" + id.getName()
	+ "' containing tag keys ["
	+ String.join(", ", collectorMap.get(getConventionName(id)).getTagKeys())
	+ "]. The meter you are attempting to register" + " has keys ["
	+ getConventionTags(id).stream().map(Tag::getKey).collect(joining(", ")) + "].");

You need to add a "meter registration failed" listener to subscribe to these events:

micrometer/micrometer-core/src/main/java/io/micrometer/core/instrument/MeterRegistry.java

Lines 803 to 806 in 8d18d8c

	public Config onMeterRegistrationFailed(BiConsumer<Id, String> meterRegistrationFailedListener) {
	meterRegistrationFailedListeners.add(meterRegistrationFailedListener);
	return this;
	}