prometheus-cpp
prometheus-cpp copied to clipboard
jupp0r
Optimistically return metrics from the cache (if present)
@jupp0r any advice on how to address this jenkins error:
no coveralls.io token specified and no travis job id found
It doesn't seem related to this PR?
@jupp0r any advice on how to address this jenkins error
Ignore it, it's a know issue that I though I had fixed.
I've been thinking about adding a test but I'm struggling to come with anything. This is a transparent change and I can't come up with a way to externally check whether a new unique_ptr was created or not.
PS: I'm not super happy about the code duplication either but again I couldn't come up with a better approach.
Is the idea that hashing the labels is less expensive than the call to make_unique? Seems doubtful but not sure. Regardless, seems like labels should only be hashed once if the metric is missing.
The idea is that the vast majority of the time Add() is called it will return the metric from the cache. Such that the first call with a new set of labels will be indeed more expensive (2 hashes + 1 make_unique), but it will be amortized over time since subsequent calls will be cheaper (1 hash with no make_unique and no ~unique_ptr).
As an illustration, we have the following pseudo-code:
Family<Counter> counter{"total_requests", "Count all requests"};
void handle_request(...) {
auto vhost = <get virtual host>;
counter.Add({ "vhost", vhost }).Increment();
...
}
The first request for each vhost initializes the cache (ie. Family<Counter>::metrics_, Family<Counter>::labels_, and Family<Counter>::labels_reverse_lookup_) after that all subsequent requests simply retrieve the Counter from there, saving a call to make_unique<Counter> and ~unique_ptr<Counter>.
I get it. How about implementing a helper which accepts a pre-computed hash value, thereby avoiding the new first-lookup penalty and avoiding the duplication mentioned earlier?
Could you please reset your branch to "our" forgeAI-nick/optimistic-family-add?