Incorrect putCount() implementation with CaffeineCacheMetrics

[DanielYWoo]

In CaffeineCacheMetrics, the counter for put(...) is loadCount(). But the loadCount works with get(key, mappingFunc) , it seems not working with put(...)

    @Override
    protected long putCount() {
        return cache.stats().loadCount();
    }

I did not see any call to cache().statsCounter().recordPutSuccess() or recordLoadSuccess() in put(...)

Guess I misunderstood something? Shall we have a recordPutSuccess() and a putCount() method in CaffeineCacheMetrics?

[shakuzen]

I'm not sure I understand the issue here. Do you mean that the value returned by Caffeine's cache.stats().loadCount() is not increased when using the Cache's put method? If that were the issue, it would be something to bring up to the Caffeine library maintainers, as we are opaquely using the cache statistics provided by the library. Let me know if I'm understanding correctly or not.

[ben-manes]

A load is a computation, e.g. computeIfAbsent, to retrieve the value atomically on a miss. The stats maintain the average load time to estimate the miss penalty. We promote loading through the cache to avoid cache stampedes.

A put is a precomputed value to insert or replace an entry. We don’t capture any stats of here, as our stats are not exhaustive and this is rarely a good metric/method. This mirrors Guava.

It looks like your metrics try to mirror Ehcache 2’s which was often not used with their self populating decorator. In that time frame and legacy apis that metric probably made more sense.

You might just document the inconsistency as the resolution here.

[DanielYWoo]

HI @shakuzen, they are two concepts, as @ben-manes explained the difference between putCount and loadCount. Here maybe we can just simply return 0 and document the reason in javadoc (Caffeine does not have putCount), but definitely we should not use loadCount as putCount here.

[izeye]

Javadoc already mentions it, so I think it's working as designed.

See https://www.javadoc.io/static/io.micrometer/micrometer-core/1.1.17/io/micrometer/core/instrument/binder/cache/CaffeineCacheMetrics.html#putCount--

[ben-manes]

...as well as manual puts. is wrong since Guava/Caffeine don't track those. Since loads are tracked if a LoadingCache, maybe @DanielYWoo is right to return a dummy value. It looks like other adapters do that for unsupported features (null or 0L).

[izeye]

@ben-manes Thanks for the feedback! I created https://github.com/micrometer-metrics/micrometer/pull/2241 to try to refine it.

For changing its return value, someone might rely on it, so I think we can't change it in maintenance releases. We might want to change it in a new feature release though.

[izeye]

We might want to change it in a new feature release though.

I created https://github.com/micrometer-metrics/micrometer/issues/2242 for this.

[shakuzen]

Thank you everyone for bringing this to our attention and providing the helpful details. With @izeye's update to the JavaDoc, I'll close this and we can follow-up with #2242.