From da20256fc9612c4d232f7dd48d7b6c077f64064b Mon Sep 17 00:00:00 2001 From: Michael Angelo Calimlim Date: Wed, 12 Jun 2019 21:15:30 +0800 Subject: [PATCH] remove the topic-name tag when the namespace does not have a topic name --- src/ziggurat/mapper.clj | 2 +- src/ziggurat/metrics.clj | 9 ++- test/ziggurat/metrics_test.clj | 143 +++++++++++++++++++++++---------- 3 files changed, 107 insertions(+), 47 deletions(-) diff --git a/src/ziggurat/mapper.clj b/src/ziggurat/mapper.clj index 5de1703f..8e791596 100644 --- a/src/ziggurat/mapper.clj +++ b/src/ziggurat/mapper.clj @@ -33,7 +33,7 @@ execution-time-namespace "handler-fn-execution-time" multi-execution-time-namespaces [[topic-entity-name execution-time-namespace] [execution-time-namespace]]] - (metrics/multi-ns-report-time multi-execution-time-namespaces time-val) + (metrics/multi-ns-report-time multi-execution-time-namespaces time-val additional-tags) (case return-code :success (do (metrics/multi-ns-increment-count multi-namespaces success-metric additional-tags)) :retry (do (metrics/multi-ns-increment-count multi-namespaces retry-metric additional-tags) diff --git a/src/ziggurat/metrics.clj b/src/ziggurat/metrics.clj index df69bfc7..251aa669 100644 --- a/src/ziggurat/metrics.clj +++ b/src/ziggurat/metrics.clj @@ -39,10 +39,15 @@ [names] (apply str (interpose "." names))) +(defn filter-topic-tag-by-namespace + [additional-tags ns] + (let [topic-name (:topic-name additional-tags)] + (dissoc additional-tags (when-not (some #(= % topic-name) ns) :topic-name)))) + (defn- inc-or-dec-count [sign metric-namespaces metric additional-tags] (let [metric-namespace (intercalate-dot metric-namespaces) - meter ^Meter (mk-meter metric-namespace metric additional-tags)] + meter ^Meter (mk-meter metric-namespace metric (filter-topic-tag-by-namespace additional-tags metric-namespaces))] (.mark meter (sign 1)))) (def increment-count (partial inc-or-dec-count +)) @@ -56,7 +61,7 @@ (defn report-time [metric-namespaces time-val additional-tags] (let [metric-namespace (intercalate-dot metric-namespaces) - histogram ^Histogram (mk-histogram metric-namespace "all" additional-tags)] + histogram ^Histogram (mk-histogram metric-namespace "all" (filter-topic-tag-by-namespace additional-tags metric-namespaces))] (.update histogram (int time-val)))) (defn multi-ns-report-time diff --git a/test/ziggurat/metrics_test.clj b/test/ziggurat/metrics_test.clj index 00d18ea6..6a9e2fdf 100644 --- a/test/ziggurat/metrics_test.clj +++ b/test/ziggurat/metrics_test.clj @@ -18,81 +18,136 @@ (is (instance? Histogram meter))))) (deftest increment-count-test - (let [metric-ns ["metric-ns"] - metric "metric3"] + (let [metric "metric3"] (testing "increases count on the meter" - (let [mk-meter-args (atom nil) + (let [expected-topic-entity-name "expected-topic-entity-name" + expected-metric-namespaces [expected-topic-entity-name "metric-ns"] + mk-meter-args (atom nil) meter (Meter.) - expected-topic-entity-name "expected-topic-entity-name"] - (with-redefs [metrics/mk-meter (fn [metric-namespace metric topic-entity-name] - (is (= topic-entity-name expected-topic-entity-name)) - (reset! mk-meter-args {:metric-namespace metric-namespace - :metric metric}) + expected-additional-tags {:topic-name expected-topic-entity-name}] + (with-redefs [metrics/mk-meter (fn [metric-namespaces metric additional-tags] + (is (= additional-tags expected-additional-tags)) + (reset! mk-meter-args {:metric-namespaces metric-namespaces + :metric metric}) meter)] - (metrics/increment-count metric-ns metric expected-topic-entity-name) + (metrics/increment-count expected-metric-namespaces metric expected-additional-tags) (is (= 1 (.getCount meter))) - (is (= (apply str (interpose "." metric-ns)) (:metric-namespace @mk-meter-args))) + (is (= (apply str (interpose "." expected-metric-namespaces)) (:metric-namespaces @mk-meter-args))) + (is (= metric (:metric @mk-meter-args)))))) + (testing "increases count on the meter - without topic name on the namespace" + (let [expected-topic-entity-name "expected-topic-entity-name" + expected-metric-namespaces ["metric-ns"] + mk-meter-args (atom nil) + meter (Meter.) + input-additional-tags {:topic-name expected-topic-entity-name} + expected-additional-tags {}] + (with-redefs [metrics/mk-meter (fn [metric-namespaces metric additional-tags] + (is (= additional-tags expected-additional-tags)) + (reset! mk-meter-args {:metric-namespaces metric-namespaces + :metric metric}) + meter)] + (metrics/increment-count expected-metric-namespaces metric input-additional-tags) + (is (= 1 (.getCount meter))) + (is (= (apply str (interpose "." expected-metric-namespaces)) (:metric-namespaces @mk-meter-args))) (is (= metric (:metric @mk-meter-args)))))) (testing "increases count on the meter when topic-entity-name is nil" - (let [mk-meter-args (atom nil) - meter (Meter.) - expected-additional-tags nil] - (with-redefs [metrics/mk-meter (fn [metric-namespace metric additional-tags] + (let [expected-topic-entity-name "expected-topic-entity-name" + expected-metric-namespaces [expected-topic-entity-name "metric-ns"] + mk-meter-args (atom nil) + meter (Meter.) + expected-additional-tags nil] + (with-redefs [metrics/mk-meter (fn [metric-namespaces metric additional-tags] (is (= additional-tags expected-additional-tags)) - (reset! mk-meter-args {:metric-namespace metric-namespace - :metric metric}) + (reset! mk-meter-args {:metric-namespaces metric-namespaces + :metric metric}) meter)] - (metrics/increment-count metric-ns metric expected-additional-tags) + (metrics/increment-count expected-metric-namespaces metric expected-additional-tags) (is (= 1 (.getCount meter))) - (is (= (apply str (interpose "." metric-ns)) (:metric-namespace @mk-meter-args))) + (is (= (apply str (interpose "." expected-metric-namespaces)) (:metric-namespaces @mk-meter-args))) (is (= metric (:metric @mk-meter-args)))))))) (deftest decrement-count-test - (let [metric-ns ["metric-ns"] - metric "metric3" - mk-meter-args (atom nil) - meter (Meter.)] + (let [expected-topic-name "expected-topic-name" + metric "metric3" + mk-meter-args (atom nil) + meter (Meter.)] (testing "decreases count on the meter" - (let [expected-additional-tags {:topic-name "expected-topic-name"}] - (with-redefs [metrics/mk-meter (fn [metric-namespace metric additional-tags] + (let [expected-additional-tags {:topic-name expected-topic-name} + expected-metric-namespaces [expected-topic-name "metric-ns"]] + (with-redefs [metrics/mk-meter (fn [metric-namespaces metric additional-tags] + (is (= additional-tags expected-additional-tags)) + (reset! mk-meter-args {:metric-namespaces metric-namespaces + :metric metric}) + meter)] + (metrics/increment-count expected-metric-namespaces metric expected-additional-tags) + (is (= 1 (.getCount meter))) + (metrics/decrement-count expected-metric-namespaces metric expected-additional-tags) + (is (zero? (.getCount meter))) + (is (= (apply str (interpose "." expected-metric-namespaces)) (:metric-namespaces @mk-meter-args))) + (is (= metric (:metric @mk-meter-args)))))) + (testing "decreases count on the meter - without topic name on the namespace" + (let [input-additional-tags {:topic-name expected-topic-name} + expected-additional-tags {} + expected-metric-namespaces ["metric-ns"]] + (with-redefs [metrics/mk-meter (fn [metric-namespaces metric additional-tags] (is (= additional-tags expected-additional-tags)) - (reset! mk-meter-args {:metric-namespace metric-namespace - :metric metric}) + (reset! mk-meter-args {:metric-namespaces metric-namespaces + :metric metric}) meter)] - (metrics/increment-count metric-ns metric expected-additional-tags) + (metrics/increment-count expected-metric-namespaces metric input-additional-tags) (is (= 1 (.getCount meter))) - (metrics/decrement-count metric-ns metric expected-additional-tags) + (metrics/decrement-count expected-metric-namespaces metric input-additional-tags) (is (zero? (.getCount meter))) - (is (= (apply str (interpose "." metric-ns)) (:metric-namespace @mk-meter-args))) + (is (= (apply str (interpose "." expected-metric-namespaces)) (:metric-namespaces @mk-meter-args))) (is (= metric (:metric @mk-meter-args)))))) (testing "decreases count on the meter when additional-tags is nil" - (let [expected-additional-tags nil] - (with-redefs [metrics/mk-meter (fn [metric-namespace metric additional-tags] + (let [expected-additional-tags nil + expected-metric-namespaces [expected-topic-name "metric-ns"]] + (with-redefs [metrics/mk-meter (fn [metric-namespaces metric additional-tags] (is (= additional-tags expected-additional-tags)) - (reset! mk-meter-args {:metric-namespace metric-namespace - :metric metric}) + (reset! mk-meter-args {:metric-namespaces metric-namespaces + :metric metric}) meter)] - (metrics/increment-count metric-ns metric expected-additional-tags) + (metrics/increment-count expected-metric-namespaces metric expected-additional-tags) (is (= 1 (.getCount meter))) - (metrics/decrement-count metric-ns metric expected-additional-tags) + (metrics/decrement-count expected-metric-namespaces metric expected-additional-tags) (is (zero? (.getCount meter))) - (is (= (apply str (interpose "." metric-ns)) (:metric-namespace @mk-meter-args))) + (is (= (apply str (interpose "." expected-metric-namespaces)) (:metric-namespaces @mk-meter-args))) (is (= metric (:metric @mk-meter-args)))))))) (deftest report-time-test (testing "updates time-val" - (let [metric-ns ["message-received-delay-histogram"] + (let [expected-topic-entity-name "expected-topic-entity-name" + expected-metric-namespaces [expected-topic-entity-name "message-received-delay-histogram"] + time-val 10 + mk-histogram-args (atom nil) + reservoir (UniformReservoir.) + histogram (Histogram. reservoir) + expected-additional-tags {:topic-name expected-topic-entity-name}] + (with-redefs [metrics/mk-histogram (fn [metric-namespaces metric additional-tags] + (is (= additional-tags expected-additional-tags)) + (reset! mk-histogram-args {:metric-namespaces metric-namespaces + :metric metric}) + histogram)] + (metrics/report-time expected-metric-namespaces time-val expected-additional-tags) + (is (= 1 (.getCount histogram))) + (is (= (apply str (interpose "." expected-metric-namespaces)) (:metric-namespaces @mk-histogram-args))) + (is (= "all" (:metric @mk-histogram-args)))))) + (testing "updates time-val - - without topic name on the namespace" + (let [expected-topic-entity-name "expected-topic-entity-name" + expected-metric-namespaces ["message-received-delay-histogram"] time-val 10 mk-histogram-args (atom nil) reservoir (UniformReservoir.) histogram (Histogram. reservoir) - expected-topic-entity-name "expected-topic-entity-name"] - (with-redefs [metrics/mk-histogram (fn [metric-ns metric topic-entity-name] - (is (= topic-entity-name expected-topic-entity-name)) - (reset! mk-histogram-args {:metric-namespace metric-ns - :metric metric}) + input-additional-tags {:topic-name expected-topic-entity-name} + expected-additional-tags {}] + (with-redefs [metrics/mk-histogram (fn [metric-namespaces metric additional-tags] + (is (= additional-tags expected-additional-tags)) + (reset! mk-histogram-args {:metric-namespaces metric-namespaces + :metric metric}) histogram)] - (metrics/report-time metric-ns time-val expected-topic-entity-name) + (metrics/report-time expected-metric-namespaces time-val input-additional-tags) (is (= 1 (.getCount histogram))) - (is (= (apply str (interpose "." metric-ns)) (:metric-namespace @mk-histogram-args))) + (is (= (apply str (interpose "." expected-metric-namespaces)) (:metric-namespaces @mk-histogram-args))) (is (= "all" (:metric @mk-histogram-args)))))))