diff --git a/dev/bench.clj b/dev/bench.clj new file mode 100644 index 0000000..e8d16bd --- /dev/null +++ b/dev/bench.clj @@ -0,0 +1,81 @@ +(ns bench + (:require [clojure.test.check.generators :as gen] + [iapetos.core :as prometheus] + [iapetos.registry :as r] + [iapetos.registry.collectors :as c] + [iapetos.test.generators :as g] + [jmh.core :as jmh]) + (:import [iapetos.registry IapetosRegistry])) + +(defn metrics + [metric-count] + (gen/sample g/metric metric-count)) + +(def dirty-string-metric + (gen/let [first-char gen/char-alpha + invalid-chars (gen/return (apply str (map char (range 33 45)))) + last-char gen/char-alphanumeric + rest-chars gen/string-alphanumeric] + (gen/return + (str + (apply str first-char invalid-chars rest-chars) + last-char)))) + +(defn dirty-metrics + [metric-count] + (gen/sample dirty-string-metric metric-count)) + +;; JMH fns + +(defn collectors + [^IapetosRegistry registry] + (.-collectors registry)) + +(defn register-collectors + [metrics] + (reduce (fn [reg metric] + (r/register reg metric (prometheus/counter metric))) + (r/create) + metrics)) + +(defn lookup + [collectors metric] + (c/lookup collectors metric {})) + +(def bench-env + {:benchmarks [{:name :registry-lookup + :fn `lookup + :args [:state/collectors :state/metric]} + + {:name :dirty-registry-lookup + :fn `lookup + :args [:state/dirty-collectors :state/dirty-metric]}] + + :states {:dirty-metrics {:fn `dirty-metrics :args [:param/metric-count]} + :dirty-metric {:fn `rand-nth :args [:state/dirty-metrics]} + :dirty-registry {:fn `register-collectors :args [:state/dirty-metrics]} + :dirty-collectors {:fn `collectors :args [:state/dirty-registry]} + + :metrics {:fn `metrics :args [:param/metric-count]} + :metric {:fn `rand-nth :args [:state/metrics]} + :registry {:fn `register-collectors :args [:state/metrics]} + :collectors {:fn `collectors :args [:state/registry]}} + + :params {:metric-count 500} + + :options {:registry-lookup {:measurement {:iterations 1000}} + :dirty-registry-lookup {:measurement {:iterations 1000}} + :jmh/default {:mode :average + :output-time-unit :us + :measurement {:iterations 1000 + :count 1}}}}) + +(def bench-opts + {:type :quick + :params {:metric-count 500}}) + +(comment + + (map #(select-keys % [:fn :mode :name :score]) (jmh/run bench-env bench-opts)) + + ) diff --git a/project.clj b/project.clj index d24ef45..6b15cbc 100644 --- a/project.clj +++ b/project.clj @@ -17,7 +17,9 @@ [io.prometheus/simpleclient_hotspot "0.12.0" :scope "provided"]] :profiles {:dev {:dependencies [[org.clojure/test.check "1.1.0"] - [aleph "0.4.6"]] + [aleph "0.4.6"] + [jmh-clojure "0.4.1"]] + :source-paths ["dev"] :global-vars {*warn-on-reflection* true}} :codox {:plugins [[lein-codox "0.10.0"]] diff --git a/src/iapetos/collector.clj b/src/iapetos/collector.clj index 73ecd52..9059f37 100644 --- a/src/iapetos/collector.clj +++ b/src/iapetos/collector.clj @@ -15,6 +15,8 @@ registries.") (metric [this] "Return a `iapetos.metric/Metric` for this collector.") + (metric-id [this] + "Return user supplied (unsanitized) identifier for this collector.") (label-instance [this instance values] "Add labels to the given collector instance produced by `instantiate`.")) @@ -55,6 +57,7 @@ (defrecord SimpleCollectorImpl [type namespace name + metric-id description subsystem labels @@ -74,6 +77,8 @@ (metric [_] {:name name :namespace namespace}) + (metric-id [_] + metric-id) (label-instance [_ instance values] (set-labels instance labels values))) @@ -85,7 +90,8 @@ ^String subsystem ^String description labels - lazy?]} + lazy?] + ::metric/keys [id]} collector-type builder-constructor] {:pre [type name namespace description]} @@ -93,6 +99,7 @@ {:type collector-type :namespace namespace :name name + :metric-id id :description description :subsystem subsystem :labels (label-names labels) @@ -119,6 +126,8 @@ this) (metric [this] (raw-metric this)) + (metric-id [this] + (hash this)) (label-instance [_ instance values] (if-not (empty? values) (throw (UnsupportedOperationException.)) @@ -133,5 +142,7 @@ (instantiate collector options)) (metric [_] metric) + (metric-id [_] + metric) (label-instance [_ instance values] (label-instance collector instance values)))) diff --git a/src/iapetos/collector/exceptions.clj b/src/iapetos/collector/exceptions.clj index b12dd5f..34463c9 100644 --- a/src/iapetos/collector/exceptions.clj +++ b/src/iapetos/collector/exceptions.clj @@ -43,6 +43,8 @@ (collector/instantiate counter registry-options)) (metric [this] (collector/metric counter)) + (metric-id [this] + (collector/metric-id counter)) (label-instance [_ instance values] (->ExceptionCounterChild counter instance values))) diff --git a/src/iapetos/metric.clj b/src/iapetos/metric.clj index 064422d..40da147 100644 --- a/src/iapetos/metric.clj +++ b/src/iapetos/metric.clj @@ -62,6 +62,7 @@ (defn as-map [metric additional-keys] - (merge - (metric-name metric) - additional-keys)) + (merge (metric-name metric) + additional-keys + ;; user supplied (unsanitized) identifier e.g., :app/runs-total + {::id metric})) diff --git a/src/iapetos/registry.clj b/src/iapetos/registry.clj index 96f9082..f43545f 100644 --- a/src/iapetos/registry.clj +++ b/src/iapetos/registry.clj @@ -56,7 +56,7 @@ registry-name registry (update options :subsystem utils/join-subsystem subsystem-name) - {})) + (collectors/initialize))) (get [_ metric labels] (collectors/by collectors metric labels options)) (raw [_] diff --git a/src/iapetos/registry/collectors.clj b/src/iapetos/registry/collectors.clj index e03c678..24a2398 100644 --- a/src/iapetos/registry/collectors.clj +++ b/src/iapetos/registry/collectors.clj @@ -1,13 +1,18 @@ (ns iapetos.registry.collectors - (:require [iapetos.registry.utils :as utils] + (:require [iapetos.metric :as metric] + [iapetos.registry.utils :as utils] [iapetos.collector :as collector]) (:import [io.prometheus.client Collector CollectorRegistry])) ;; ## Init (defn initialize + "Initialize map for collectors. + + - ::path-cache meta used to prevent additional metric path computation + requiring metric name sanitization." [] - {}) + ^{::path-cache {}} {}) ;; ## Management @@ -39,6 +44,7 @@ instance (collector/instantiate collector options)] (-> {:collector collector :metric metric + :cache-key [(collector/metric-id collector) options] :path path :raw instance :register (register-collector-delay registry instance) @@ -46,15 +52,18 @@ (warn-lazy-deprecation!)))) (defn insert - [collectors {:keys [path] :as collector}] - (assoc-in collectors path collector)) + [collectors {:keys [path cache-key] :as collector}] + (-> collectors + (vary-meta update ::path-cache assoc cache-key path) + (assoc-in path collector))) (defn delete - [collectors {:keys [path] :as collector}] - (update-in collectors - (butlast path) - dissoc - (last path))) + [collectors {:keys [path cache-key] :as _collector}] + (-> collectors + (vary-meta update ::path-cache dissoc cache-key) + (update-in (butlast path) + dissoc + (last path)))) (defn unregister [collectors {:keys [register unregister] :as collector}] @@ -69,9 +78,9 @@ (defn clear [collectors] - (->> (for [[namespace vs] collectors - [subsystem vs] vs - [_ collector] vs] + (->> (for [[_namespace vs] collectors + [_subsystem vs] vs + [_name collector] vs] collector) (reduce unregister collectors))) @@ -83,7 +92,9 @@ (defn lookup [collectors metric options] - (some->> (utils/metric->path metric options) + (some->> (or (get-in (meta collectors) + [::path-cache [metric options]]) + (utils/metric->path metric options)) (get-in collectors))) (defn by diff --git a/test/iapetos/metric_test.clj b/test/iapetos/metric_test.clj index 40240c9..2fa7689 100644 --- a/test/iapetos/metric_test.clj +++ b/test/iapetos/metric_test.clj @@ -44,4 +44,5 @@ (let [{:keys [name namespace] :as r} (metric/as-map metric additional-keys)] (and (re-matches #"[a-zA-Z0-9_]+" name) (re-matches #"[a-zA-Z0-9_]+" namespace) - (= additional-keys (dissoc r :name :namespace)))))) + (= additional-keys (dissoc r :name :namespace ::metric/id)) + (= metric (::metric/id r)))))) diff --git a/test/iapetos/registry/collectors_test.clj b/test/iapetos/registry/collectors_test.clj new file mode 100644 index 0000000..618d9c3 --- /dev/null +++ b/test/iapetos/registry/collectors_test.clj @@ -0,0 +1,45 @@ +(ns iapetos.registry.collectors-test + (:require [clojure.test.check + [generators :as gen] + [properties :as prop] + [clojure-test :refer [defspec]]] + [clojure.test :refer :all] + [iapetos.test.generators :as g] + [iapetos.core :as prometheus] + [iapetos.collector :as collector] + [iapetos.registry.collectors :as c]) + (:import [iapetos.registry IapetosRegistry])) + +(defn- all-collectors + [^IapetosRegistry registry] + (for [[_namespace vs] (.-collectors registry) + [_subsystem vs] vs + [_name collector] vs] + collector)) + +(defn- path-cache + [^IapetosRegistry registry] + (::c/path-cache (meta (.-collectors registry)))) + +(defspec t-registry-should-cache-the-path-of-registered-collectors 50 + (prop/for-all + [registry (gen/return (prometheus/collector-registry)) + collector-constructor g/collector-constructor + collector-name g/metric] + (let [collector (collector-constructor collector-name) + registry (prometheus/register registry collector) + cache (path-cache registry)] + (is (every? (fn [{:keys [path cache-key] :as _collector}] + (= path (get cache cache-key))) + (all-collectors registry)))))) + +(defspec t-registry-should-evict-path-from-cache-for-unregistered-collectors 50 + (prop/for-all + [registry (gen/return (prometheus/collector-registry)) + collector-constructor g/collector-constructor + collector-name g/metric] + (let [collector (collector-constructor collector-name) + registry (-> registry + (prometheus/register collector) + (prometheus/unregister collector))] + (is (empty? (path-cache registry))))))