[Question] Possible unexpected behaviour when applying Meter to collector fails.

[tgodinho]

Hello guys,

I came across this behaviour when trying to register multiple meters with different set of tags.
Basically, I think that when we try to register a meter that fails the verification on line 414 (tags don't match), all the previously well-built meters will be scrapped; a direct consequence of returning null in ConcurrentMap#compute.

This behaviour caught me completely off guard. I was expecting the previous meters to remain untouched.
Is there any particular reason for this behaviour??

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

Lines 405 to 425 in def8d3d

	private void applyToCollector(Meter.Id id, Consumer<MicrometerCollector> consumer) {
	collectorMap.compute(getConventionName(id), (name, existingCollector) -> {
	if (existingCollector == null) {
	MicrometerCollector micrometerCollector = new MicrometerCollector(id, config().namingConvention(), prometheusConfig);
	consumer.accept(micrometerCollector);
	return micrometerCollector.register(registry);
	}
	List<String> tagKeys = getConventionTags(id).stream().map(Tag::getKey).collect(toList());
	if (existingCollector.getTagKeys().equals(tagKeys)) {
	consumer.accept(existingCollector);
	return existingCollector;
	}
	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(", ")) + "].");
	return null;
	});
	}

I know my use case is a bit messed up. I am basically overriding the GuavaCacheBinder to introduce counters for the reasons that led entries to be evicted. My approach was to replace the underlying eviction counter in the CacheMeterBinder class with N counters with the same name but an additional reason tag.
It works great for the first cache, as I remove the old counter before adding the new ones. However, for the second cache, this behaviour causes the meters to be wiped out when the CacheMeterBinder tries to register the old counter.

I'll have to play a bit with the API to see if I can get away with preventing the old meter to be registered in the first place.
Although, it is becoming clearer that this overriding approach might be troublesome in the future, specially when we start having multiple services registered in the same Prometheus instance.

BIG DISCLAIMER: I am a big noob, its basically my first day using this lib.

[shakuzen]

Sorry for the delay in triaging this. It looks like this is the same as #2399 (meaning that one was a duplicate of this, actually), which we believe was fixed with #2400 to avoid the error. Note this comment #2399 (comment), though, which is that the different sets of labels will not show up in the scrape results currently. Pending some more exploration and testing, we're considering making those show up in the scrape results in the Micrometer 1.7 release timeframe, tentatively.
Duplicate of #2399