-
Notifications
You must be signed in to change notification settings - Fork 1k
Description
Describe the bug
When using two caching implementation at the same time (namely JCache and Redis)
the error mentioned in the subject arises during a Spring Boot 2.5.3 application startup.
(I debugged the code deeply, and the bug is Prometheus registry related, this is why I reported it in here, not on Spring Boot forums.)
As far as I understand this problem is caused by a different Meter.Type for the two cache metrics mentioned.
RedisCacheMetrics.java from spring-boot-actuator.jar (e.g. version 2.5.3) uses a FunctionCounter.builder
whereas JCacheMetrics from micrometer-core.jar (e.g. 1.7.2) uses Gauge.builder. (These types are there for a long time now, so the problem is not the type difference, but a new meter registry content checking logic in the newest micrometer-metrics code base.)
Both metrics implementation register their metrics under the name "cache.removals" and (fortunately) the tag keys (namely "cache", "cacheManager" and "name") are the same as well.
When using micrometer-1.6.x there are no problems, however with micrometer-1.7.x the naming difference in PrometheusNamingConvention for the two Meter.Type cause desynchronization problem between the 'collectorMap' instance variable in PrometheusMeterRegistry.java (micrometer-registry-prometheus-1.7.2.jar) and the 'namesToCollectors' in CollectorRegistry.java (simpleclient-0-10-0-jar).
The following happens:
The first cache metrics (Redis in our case) is registered seamlessly.
However, when the second metrics (a JCache one) is registered, when the applyToCollector()
method of PrometheusMeterRegistry is called, the first condition in the compute block if (existingCollector == null)
evaluates to true,
because the Redis FunctionCounter (thanks to the naming convention for counters) is registered as cache_removals_total.
So a fresh new MicrometerCollector is created and a register()
call is attempted.
Unfortunately, in the (simpleclient) CollectorRegistry, in the public void register(Collector m)
method the if (namesToCollectors.containsKey(name))
evaluates to true, resulting in an IllegalArgumentException. The reason for this is that during the Redis registration, both the keys 'cache_removals' and 'cache_removals_total' are inserted into namesToCollectors. (Because both returned by collectorNames(Collector m)
.)
So the content of the two Maps ('collectoMap' and 'namesToCollectors') are out of sync.
I think the problem can be solved multiple ways:
- NOT to insert the 'cache_removals' into the 'namesToCollectors' during FunctionCounter registrations, just the 'cache_removals_total' value.
This means that returning the pure family.name incollectorNames(Collector m)
method of CollectorRegistry should be left out. (Probably for all types? BTW: why 'GAUGAGE' is missing from here?) - In the
name()
method of PrometheusNamingConvention.java a '_total' should be added in case of 'GAUGE' type as well. (Thus this might go against naming conventions...) - The 'cache.removals' should be changed to 'cache.removals.total' in JCacheMetrics.java.
I personally think the latter is the best and the second is the worst solution.
Any opinions?
Environment
- Micrometer version: 1.7.2 (version listed in Spring Boot 2.5.3 bom)
- Micrometer registry: prometheus
- OS: all
- Java version: 12
To Reproduce
How to reproduce the bug: see description.
Expected behavior
Using JCache and Redis in Spring Boot work together at the same time, I guess?...
Additional context
Add any other context about the problem here, e.g. related issues.