Tyler K Van Gorder opened SPR-17350 and commented

Now that Spring Boot 2.x uses Micrometer for its metrics abstraction, it is helpful to have the caching abstraction instrumented such that metrics around cache size, hits, misses, puts, and evictions are published through Micrometer.

Spring Boot does do some limited auto-configuration to instrument the caching abstraction but is limited to only those caches that are known at startup time. This limitation is due to the fact that there is no way to observe when a new, dynamic cache has been created by the Spring Caching abstraction.

The current functionality means that any methods that are marked as @Cacheable will NOT have their metrics recorded through Micrometer.

To solve this problem without having to embed knowledge of Micrometer into each cache manager implementation, it would be better to have those cache managers publish an application event, CacheCreatedEvent.

This would allow for Spring Boot to auto-configure a listener for that event and bind all caches, dynamic or otherwise, to Micrometer's meter registry.

Looking at the code, the CaceCreateEvent should be emitted from the CacheManager.getCache(String name) method. The event should have both the cache name and the cache manager that was used to create the cache.

Most of the cache manager implementations extend AbstractCacheManager, and therefore this will be a good place to publish the event. However, there are a couple of additional CacheManager implementations (CaffineCacheMAnager, ConcurrentMapCacheManager) that do not extends the abstract base class and will also need to be modified to publish the event.

Affects: 5.0.9, 5.1 GA

Reference URL: https://github.com/spring-projects/spring-boot/issues/14576

4 votes, 6 watchers

Comment From: spring-projects-issues

Stéphane Nicoll commented

The current functionality means that any methods that are marked as @Cacheable will NOT have their metrics recorded through Micrometer.

That's a very wide statement that doesn't seem accurate to me. If you use a cache library with a configuration file that lists the caches, which arguably is what you should be doing, they will be instrumented just fine.

it would be better to have those cache managers publish an application event, CacheCreatedEvent.

I see a semantic problem with this. When the CacheManager initializes itself, it is also going to create caches (the ones that it knows upfront because the underlying cache library provides them right away). We can't really call the event like that and not fire an event in such a case.

The other end can accommodate to this contract as long as it moves the handling of metrics to a listener only. Unfortunately, we don't manage the hazelcast cache manager so we'll have to sync with them before being able to do that.

Triggering an event is a nice idea to let other components know what is happening. There is no way to enforce that in the contract and there are several external implementations so I am not 100% sure that's the right way to go. Let's tentatively assign this to 5.2 and investigate a bit more. At this point, I am tempted to have a restricted MissingCacheCreatedEvent or something like that to narrow the contract a bit.

Comment From: spring-projects-issues

Tyler K Van Gorder commented

Sorry, you are right, in that my statement is too broad. It really should read "The current functionality means that any methods that are marked as @Cacheable that have not had their cache names defined in configuration will not have their metrics recorded through Micrometer."

Ugh, you are totally correct in that the code paths for the initial creation of caches vs the "on the fly" cache creation are different. I like your idea of narrowing the scope of this change to MissingCacheCreatedEvent, but like you pointed out, there is no way to concretely enforce that each cache manager implementation generates that event. It is a deceivingly tricky problem and I appreciate you looking into it.

I think I have a third idea on how to fix this at the Spring Boot level, I am going to leave a comment on there. 

Comment From: snicoll

I've been spending a fair amount of time on this one and also brainstorming with @jhoeller. I am not keen to implement this at framework level at this point.

The use case is very specific, yet the impact at framework level is quite facing. I've prototyped a MissingCacheCreatedEvent which feels a bit odd as that's the only event that the CacheManager would fire. We've discussed the possibility to rename that to CacheCreatedEvent with a flag to indicate that the cache was loaded from config or missing. This would fire a number of events on startup and the presence of such flag is hard to resonate from a framework API perspective.

All in all, this is a nice way to fix the related problem in Spring Boot but the proposal as is is not generally useful so I don't think we should implement it that way. I am now considering other options to fix the related issue, potentially via some improvements in Micrometer or Spring Boot.