Custom labels for caffeine cache

[aaabramov]

Adds an option to specify custom labels for metrics. This may be (and is!) useful in microservice environment where service caches are being used in different domains. Using custom labels, users will be able to distinguish different cache metrics without [pre/post]fixing cache (cacheName) parameter.

Also upgrades caffeine cache to v2.8.6

This feature keeps backwards binary & source compatibility.

[aaabramov]

Should we initialize this this List each time? Maybe simply initialize it once as instance field?

[aaabramov]

It may be a good idea to store label inside children value. This would require adding custom class as a value but since it is internal we can implement this without breaking changes.

[brian-brazil]

Can you expand on what you mean by different domains here?

[aaabramov]

Sure.
Let's say we are having k8s cluster with 10 different apps. Each of it uses caffeine as a cache. But single prometheus scrapper.

Currently, the only way to distinguish cache metrics provided by this library between apps is to modify somehow cacheName parameter (either we can prefix it with domain, e.g. users_the_cache_name, dashboards_the_cache_name, etc. or with postfix). This makes creating Grafana dashboards pretty difficult as we have to rely on regexes of cache label.

In order to create cache dashboard for users service we have to write something like

sum(rate(caffeine_cache_miss_total{cache~="users_.*"}[5m])) by (cache)

That's why I decided it would be easier for users to specify their own labels and create Grafana dashboard like this:

Map<String, String> labels = new HashMap<String, String>() {{
    put("app", "users-service");
}};

CacheMetricsCollector coll = new CacheMetricsCollector(labels);

sum(rate(caffeine_cache_miss_total{app="users-service"}[5m])) by (cache)

This PR is more like a proposal, I am totally open to discussion 👨‍💻

[brian-brazil]

What happens if there's different libraries in the application that are using different label names with this? We can't make any assumptions in that regard.

[aaabramov]

Is it an issue in this case? Wouldn't we expose different metrics in this case? I am not so strong enough in prometheus but I would expect to have different metric rows from prometheus, e.g.:

caffeine_cache_miss_total{app="app-1"} 1
caffeine_cache_miss_total{app="app-2"} 2
caffeine_cache_miss_total{app="app-3"} 3

Or am I missing something?

Anyway, we are simply allowing users to take control of what is exposed. We are not making any decisions for them.

[brian-brazil]

Is it an issue in this case? Wouldn't we expose different metrics in this case?

Yes, you'd break downstream PromQL and depending on implementation could also produce invalid exposition (what happens if two libraries clash on labels?). If you're varying instrumentation labels, then the metric name must also change.

[aaabramov]

Thanks, now I see.

But this could happen without this library? We are not introducing some sort of "failure injection".

It's user's responsibility to keep an eye on such things. We are just providing an instrument (facade, if you want) to make life easier.

[brian-brazil]

It's user's responsibility to keep an eye on such things.

My point is that the user can't, as they don't have full control of all their libraries. If you have code like this, you have to presume that other unknown parts of your application are using it in arbitrary ways, and then considering if it's possible for you to write correct code given that. For this proposal as it stands it isn't.

[aaabramov]

I see your point.

What is your proposal to solve the issue? Playing around with cacheName?

Well, it is still possible to inherit CacheMetricsCollector class and override collect method (that's what I'll probably do in case of rejecting this feature). But that will look like copy-pasting code because collect contains most of the logic of the class.

[brian-brazil]

Given that there has only been one request for this thus far, I think it would be best to handle it on your side for now. Any form of wrapper that adjusts the metric name and labels should do the trick.

[aaabramov]

Sadly, but decision is up to you.

I will create issue referencing this PR to get community feedback.

[aaabramov]

@brian-brazil closing this for now, described proposal in #603