From f537c8006992acc6bc6d0178f57c513b685d57ca Mon Sep 17 00:00:00 2001 From: appleby <86076+appleby@users.noreply.github.com> Date: Wed, 23 Oct 2024 08:10:41 -0700 Subject: [PATCH] Give more specific name to metricsv2 prometheus metrics (#49001) This is to allow for the possibiliy of adding more metricsv2 usage and error metrics in the future while still distinguishing the source of the errors. --- src/metabase/analytics/prometheus.clj | 8 +++--- .../query_processor/middleware/metrics.clj | 4 +-- .../middleware/metrics_test.clj | 28 +++++++++---------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/metabase/analytics/prometheus.clj b/src/metabase/analytics/prometheus.clj index 844ec651d7b..f67e3d15d60 100644 --- a/src/metabase/analytics/prometheus.clj +++ b/src/metabase/analytics/prometheus.clj @@ -207,10 +207,10 @@ {:description "Number of successful responses from SCIM endpoints"}) (prometheus/counter :metabase-scim/response-error {:description "Number of error responses from SCIM endpoints"}) - (prometheus/counter :metabase-query-processor/metrics - {:description "Number of queries consuming metrics processed by the query processor."}) - (prometheus/counter :metabase-query-processor/metric-errors - {:description "Number of errors when processing metrics."})]) + (prometheus/counter :metabase-query-processor/metrics-adjust + {:description "Number of queries with metrics processed by the metrics adjust middleware."}) + (prometheus/counter :metabase-query-processor/metrics-adjust-errors + {:description "Number of errors when processing metrics in the metrics adjust middleware."})]) (defn- setup-metrics! "Instrument the application. Conditionally done when some setting is set. If [[prometheus-server-port]] is not set it diff --git a/src/metabase/query_processor/middleware/metrics.clj b/src/metabase/query_processor/middleware/metrics.clj index 805a36d9a93..c9ea89c100c 100644 --- a/src/metabase/query_processor/middleware/metrics.clj +++ b/src/metabase/query_processor/middleware/metrics.clj @@ -254,7 +254,7 @@ (if-not (find-first-metric query) query (do - (prometheus/inc! :metabase-query-processor/metrics) + (prometheus/inc! :metabase-query-processor/metrics-adjust) (try (let [query (lib.walk/walk query @@ -266,5 +266,5 @@ (when-let [metric (find-first-metric <>)] (throw (ex-info "Failed to replace metric" {:metric metric}))))) (catch Throwable e - (prometheus/inc! :metabase-query-processor/metric-errors) + (prometheus/inc! :metabase-query-processor/metrics-adjust-errors) (throw e)))))) diff --git a/test/metabase/query_processor/middleware/metrics_test.clj b/test/metabase/query_processor/middleware/metrics_test.clj index e32643568d8..4b97c95ac52 100644 --- a/test/metabase/query_processor/middleware/metrics_test.clj +++ b/test/metabase/query_processor/middleware/metrics_test.clj @@ -63,8 +63,8 @@ (comp #'metrics/adjust #'fetch-source-query/resolve-source-cards)) (defn- check-prometheus-metrics! - [& {expected-metrics-count :metabase-query-processor/metrics - expected-metrics-errors :metabase-query-processor/metric-errors + [& {expected-metrics-count :metabase-query-processor/metrics-adjust + expected-metrics-errors :metabase-query-processor/metrics-adjust-errors metric-and-mp :metric-and-mp query-fn :query-fn check-fn :check-fn}] @@ -76,14 +76,14 @@ read-metric #(% @metrics 0)] (with-redefs [prometheus/inc! #(swap! metrics update % (fnil inc 0))] (check-fn query) - (is (= expected-metrics-count (read-metric :metabase-query-processor/metrics))) - (is (= expected-metrics-errors (read-metric :metabase-query-processor/metric-errors)))))) + (is (= expected-metrics-count (read-metric :metabase-query-processor/metrics-adjust))) + (is (= expected-metrics-errors (read-metric :metabase-query-processor/metrics-adjust-errors)))))) (deftest adjust-prometheus-metrics-test (testing "adjustment of query with no metrics does not increment either counter" (check-prometheus-metrics! - :metabase-query-processor/metrics 0 - :metabase-query-processor/metric-errors 0 + :metabase-query-processor/metrics-adjust 0 + :metabase-query-processor/metrics-adjust-errors 0 :query-fn (fn [_mp _metric] (-> (lib/query meta/metadata-provider (meta/table-metadata :products)) (lib/aggregate (lib/avg (meta/field-metadata :products :rating))))) @@ -92,15 +92,15 @@ (adjust %))))) (testing "successful adjustment does not increment error counter" (check-prometheus-metrics! - :metabase-query-processor/metrics 1 - :metabase-query-processor/metric-errors 0 + :metabase-query-processor/metrics-adjust 1 + :metabase-query-processor/metrics-adjust-errors 0 :check-fn #(is (=? {:stages [{:source-table (meta/id :products) :aggregation [[:avg {} [:field {} (meta/id :products :rating)]]]}]} (adjust %))))) (testing "failure to adjust :metric clauses increments error counter" (check-prometheus-metrics! - :metabase-query-processor/metrics 1 - :metabase-query-processor/metric-errors 1 + :metabase-query-processor/metrics-adjust 1 + :metabase-query-processor/metrics-adjust-errors 1 :check-fn (fn [query] (with-redefs [metrics/adjust-metric-stages (fn [_ _ stages] stages)] (is (thrown-with-msg? @@ -109,8 +109,8 @@ (adjust query))))))) (testing "exceptions from other libs also increment error counter" (check-prometheus-metrics! - :metabase-query-processor/metrics 1 - :metabase-query-processor/metric-errors 1 + :metabase-query-processor/metrics-adjust 1 + :metabase-query-processor/metrics-adjust-errors 1 :check-fn (fn [query] (with-redefs [lib.metadata/bulk-metadata-or-throw (fn [& _] (throw (Exception. "Test exception")))] (is (thrown-with-msg? @@ -119,8 +119,8 @@ (adjust query))))))) (testing "metric missing aggregation increments counter and throws exception" (check-prometheus-metrics! - :metabase-query-processor/metrics 1 - :metabase-query-processor/metric-errors 1 + :metabase-query-processor/metrics-adjust 1 + :metabase-query-processor/metrics-adjust-errors 1 :metric-and-mp (mock-metric (-> (lib/query meta/metadata-provider (meta/table-metadata :products)))) :query-fn (fn [mp metric] (-> (lib/query mp (meta/table-metadata :products)) -- GitLab