Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ::path-cache to registry collectors map metadata #59

Merged
merged 1 commit into from
Sep 15, 2022

Conversation

kgann
Copy link
Contributor

@kgann kgann commented Sep 15, 2022

An update to the Java Client Library was merged that improved performance of metric name sanitization. Unfortunately it also includes the move to OpenMetrics that would cause many issues at Nubank. We would like to get the benefit of the metric sanitization performance improvement but cannot upgrade the client at this time.

We shifted our focus to Iapetos in an attempt to implement the improvement here. During the investigation we noticed that collector lookup from the registry sanitizes the metric names on every invocation. In order to limit, as much as possible, the redundant sanitization, we added a simple cache holding the paths to the collectors.

If the cache does not result in a hit, perhaps because the collector was registered with a keyword and then referenced by another supported type such as a vector, the implementation falls back to computing the path again.

For example:

(defonce registry
  (-> (prometheus/collector-registry)
      (prometheus/register
       (prometheus/counter :app/runs-!!!total))))

(.get registry [:app :runs-!!!total] {}) ;; cache MISS, path computed
(.get registry :app/runs-total {}) ;; cache MISS, path computed
(.get registry :app/runs-!!!total {}) ;; cache HIT

Included in the PR is a benchmark of the results:

On the master branch we see:

 {:fn bench/lookup,
  :mode :average,
  :name :registry-lookup,                          ;; metric names do not need to be sanitized
  :score [1.014698602638397 "us/op"]}

 {:fn bench/lookup,
  :mode :average,
  :name :dirty-registry-lookup,                    ;; metric names NEED to be sanitized
  :score [1.9870187159904702 "us/op"]}

With the addition of a cache we observed the following performance improvement:

 {:fn bench/lookup,
  :mode :average,
  :name :registry-lookup,                          ;; metric names do not need to be sanitized
  :score [0.18071412341478607 "us/op"]}

 {:fn bench/lookup,
  :mode :average,
  :name :dirty-registry-lookup,                    ;; metric names NEED to be sanitized
  :score [0.1301492753507884 "us/op"]}

@slipset slipset merged commit 493b38c into clj-commons:master Sep 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants