diff --git a/project.clj b/project.clj index c9b20e9f9..59ab15cb0 100644 --- a/project.clj +++ b/project.clj @@ -55,7 +55,7 @@ [commons-codec "1.22.1"] [commons-io "2.22.0"] [grimradical/clj-semver "0.3.0" :exclusions [org.clojure/clojure]] - [io.dropwizard.metrics/metrics-core "3.2.6"] + [io.dropwizard.metrics5/metrics-core "5.0.7"] [lambdaisland/uri "1.19.155"] [liberator "0.15.3"] ;; NOTE: Versions after 8.1 bring in Jackson 3.x. @@ -76,15 +76,15 @@ [org.openvoxproject/kitchensink "3.5.8" :classifier "test"] [org.openvoxproject/rbac-client "1.3.0"] [org.openvoxproject/rbac-client "1.3.0" :classifier "test"] - [org.openvoxproject/ring-middleware "2.2.1"] + [org.openvoxproject/ring-middleware "2.2.1-SNAPSHOT"] [org.openvoxproject/ssl-utils "3.7.1"] [org.openvoxproject/trapperkeeper "5.0.5"] [org.openvoxproject/trapperkeeper "5.0.5" :classifier "test"] - [org.openvoxproject/trapperkeeper-comidi-metrics "1.1.1"] + [org.openvoxproject/trapperkeeper-comidi-metrics "1.1.1-SNAPSHOT"] [org.openvoxproject/trapperkeeper-authorization "2.4.1"] [org.openvoxproject/trapperkeeper-filesystem-watcher "1.6.1"] - [org.openvoxproject/trapperkeeper-metrics "2.3.2"] - [org.openvoxproject/trapperkeeper-metrics "2.3.2" :classifier "test"] + [org.openvoxproject/trapperkeeper-metrics "2.3.1-SNAPSHOT"] + [org.openvoxproject/trapperkeeper-metrics "2.3.1-SNAPSHOT" :classifier "test"] [org.openvoxproject/trapperkeeper-scheduler "1.4.1"] [org.openvoxproject/trapperkeeper-status "1.5.1"] [org.openvoxproject/trapperkeeper-webserver "12.1.1"] @@ -106,7 +106,7 @@ [clj-commons/fs] [commons-io] [grimradical/clj-semver :exclusions [org.clojure/clojure]] - [io.dropwizard.metrics/metrics-core] + [io.dropwizard.metrics5/metrics-core] [liberator] ;; We do not currently use this dependency directly, but ;; we have documentation that shows how users can use it to diff --git a/src/clj/puppetlabs/services/jruby/jruby_metrics_core.clj b/src/clj/puppetlabs/services/jruby/jruby_metrics_core.clj index 8fabf1577..d381ca894 100644 --- a/src/clj/puppetlabs/services/jruby/jruby_metrics_core.clj +++ b/src/clj/puppetlabs/services/jruby/jruby_metrics_core.clj @@ -8,7 +8,7 @@ [puppetlabs.comidi :as comidi] [puppetlabs.i18n.core :refer [trs]] [puppetlabs.services.protocols.jruby-puppet :as jruby-protocol]) - (:import (com.codahale.metrics MetricRegistry Gauge Counter Histogram Meter Timer) + (:import (io.dropwizard.metrics5 MetricFilter MetricRegistry Gauge Counter Histogram Meter Timer) (clojure.lang Atom IFn) (java.time ZoneOffset ZonedDateTime) (java.time.format DateTimeFormatter) @@ -201,15 +201,8 @@ (schema/defn borrow-timers :- java.util.Map "Returns a map of borrow timers from JRuby Metrics" [{:keys [metric-registry hostname]} :- JRubyMetrics] - (let [metric-namespace (metrics/host-metric-name hostname "jruby.borrow-timer") - ;; TODO: v4.0 of Dropwizard Metrics has a MetricFilter/startsWith - ;; static method that returns a filter which can be passed - ;; directly to .getTimers. - metric-filter (partial filter (fn [[k _]] - (str/starts-with? k metric-namespace)))] - (->> (.getTimers metric-registry) - metric-filter - (into {})))) + (let [metric-namespace (metrics/host-metric-name hostname "jruby.borrow-timer")] + (.getTimers metric-registry (MetricFilter/startsWith metric-namespace)))) (schema/defn summarize-borrow-timers "Generates a summary for each JRuby borrow timer @@ -219,10 +212,11 @@ [{:keys [hostname] :as metrics} :- JRubyMetrics] (let [timer-namespace (metrics/host-metric-name hostname "jruby.borrow-timer") get-name (fn [timer-name] - (if (= timer-namespace timer-name) + (let [timer-name (str timer-name)] + (if (= timer-namespace timer-name) "total" (str/replace-first timer-name - (str timer-namespace ".") ""))) + (str timer-namespace ".") "")))) timer-summary (fn [timer] (let [snapshot (.getSnapshot timer)] {:count (.getCount timer) diff --git a/src/clj/puppetlabs/services/jruby/jruby_puppet_core.clj b/src/clj/puppetlabs/services/jruby/jruby_puppet_core.clj index 29fc61667..feb8127f7 100644 --- a/src/clj/puppetlabs/services/jruby/jruby_puppet_core.clj +++ b/src/clj/puppetlabs/services/jruby/jruby_puppet_core.clj @@ -12,7 +12,7 @@ (:import (com.puppetlabs.puppetserver PuppetProfiler JRubyPuppet) (clojure.lang IFn) (java.util HashMap) - (com.codahale.metrics MetricRegistry))) + (io.dropwizard.metrics5 MetricRegistry))) ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;; Constants diff --git a/src/clj/puppetlabs/services/master/master_core.clj b/src/clj/puppetlabs/services/master/master_core.clj index 1489499c6..581aa485d 100644 --- a/src/clj/puppetlabs/services/master/master_core.clj +++ b/src/clj/puppetlabs/services/master/master_core.clj @@ -25,7 +25,7 @@ [schema.core :as schema] [slingshot.slingshot :refer [throw+ try+]]) (:import (clojure.lang IFn) - (com.codahale.metrics Gauge MetricRegistry) + (io.dropwizard.metrics5 Gauge MetricRegistry) (com.fasterxml.jackson.core JsonParseException) (java.io FileInputStream) (java.lang.management ManagementFactory) @@ -1205,10 +1205,10 @@ (defn register-gauge! [registry hostname metric-name metric-fn] - (.register registry (metrics/host-metric-name hostname metric-name) - (proxy [Gauge] [] - (getValue [] - (metric-fn))))) + (.registerGauge registry (metrics/host-metric-name hostname metric-name) + (proxy [Gauge] [] + (getValue [] + (metric-fn))))) (schema/defn register-jvm-metrics! [registry :- MetricRegistry diff --git a/src/clj/puppetlabs/services/master/master_service.clj b/src/clj/puppetlabs/services/master/master_service.clj index 2a4f63b98..a883fe00c 100644 --- a/src/clj/puppetlabs/services/master/master_service.clj +++ b/src/clj/puppetlabs/services/master/master_service.clj @@ -1,5 +1,6 @@ (ns puppetlabs.services.master.master-service (:require [clojure.tools.logging :as log] + [clojure.walk :as walk] [puppetlabs.trapperkeeper.core :refer [defservice]] [puppetlabs.puppetserver.certificate-authority :as ca] [puppetlabs.puppetserver.jruby-request :as jruby-request] @@ -14,36 +15,55 @@ (def master-service-status-version 1) +;; Dropwizard 5 JMX rejects wildcard pattern characters in ObjectName values. +;; We replace route wildcards with a stable token before metrics registration. +(def metrics-route-wildcard-token "ANY") + +(defn sanitize-metrics-route-id + [route-id] + (if (string? route-id) + (str/replace route-id "*" metrics-route-wildcard-token) + route-id)) + +(defn sanitize-route-metadata-for-metrics + [route-metadata] + (walk/postwalk + (fn [node] + (if (and (map? node) (string? (:route-id node))) + (update node :route-id sanitize-metrics-route-id) + node)) + route-metadata)) + ;; Default list of allowed histograms/timers (def default-metrics-allowed-hists ["http.active-histo" - "http.puppet-v3-catalog-/*/-requests" - "http.puppet-v3-environment-/*/-requests" - "http.puppet-v3-environment_classes-/*/-requests" + "http.puppet-v3-catalog-/ANY/-requests" + "http.puppet-v3-environment-/ANY/-requests" + "http.puppet-v3-environment_classes-/ANY/-requests" "http.puppet-v3-environments-requests" - "http.puppet-v3-file_bucket_file-/*/-requests" - "http.puppet-v3-file_content-/*/-requests" - "http.puppet-v3-file_metadata-/*/-requests" - "http.puppet-v3-file_metadatas-/*/-requests" - "http.puppet-v3-node-/*/-requests" - "http.puppet-v3-report-/*/-requests" - "http.puppet-v3-static_file_content-/*/-requests"]) + "http.puppet-v3-file_bucket_file-/ANY/-requests" + "http.puppet-v3-file_content-/ANY/-requests" + "http.puppet-v3-file_metadata-/ANY/-requests" + "http.puppet-v3-file_metadatas-/ANY/-requests" + "http.puppet-v3-node-/ANY/-requests" + "http.puppet-v3-report-/ANY/-requests" + "http.puppet-v3-static_file_content-/ANY/-requests"]) ;; Default list of allowed values/counts (def default-metrics-allowed-vals ["http.active-requests" - "http.puppet-v3-catalog-/*/-percentage" - "http.puppet-v3-environment-/*/-percentage" - "http.puppet-v3-environment_classes-/*/-percentage" + "http.puppet-v3-catalog-/ANY/-percentage" + "http.puppet-v3-environment-/ANY/-percentage" + "http.puppet-v3-environment_classes-/ANY/-percentage" "http.puppet-v3-environments-percentage" - "http.puppet-v3-file_bucket_file-/*/-percentage" - "http.puppet-v3-file_content-/*/-percentage" - "http.puppet-v3-file_metadata-/*/-percentage" - "http.puppet-v3-file_metadatas-/*/-percentage" - "http.puppet-v3-node-/*/-percentage" - "http.puppet-v3-report-/*/-percentage" - "http.puppet-v3-static_file_content-/*/-percentage" - "http.puppet-v3-status-/*/-percentage" + "http.puppet-v3-file_bucket_file-/ANY/-percentage" + "http.puppet-v3-file_content-/ANY/-percentage" + "http.puppet-v3-file_metadata-/ANY/-percentage" + "http.puppet-v3-file_metadatas-/ANY/-percentage" + "http.puppet-v3-node-/ANY/-percentage" + "http.puppet-v3-report-/ANY/-percentage" + "http.puppet-v3-static_file_content-/ANY/-percentage" + "http.puppet-v3-status-/ANY/-percentage" "http.total-requests" ; num-cpus is registered in trapperkeeper-comidi-metrics, see ; https://github.com/puppetlabs/trapperkeeper-comidi-metrics/blob/0.1.1/src/puppetlabs/metrics/http.clj#L117-L120 @@ -151,7 +171,9 @@ boltlib-path certname)) routes (comidi/context path ring-app) - route-metadata (comidi/route-metadata routes) + route-metadata (-> routes + comidi/route-metadata + sanitize-route-metadata-for-metrics) comidi-handler (comidi/routes->handler routes) registry (get-metrics-registry :puppetserver) http-metrics (http-metrics/initialize-http-metrics! @@ -159,8 +181,16 @@ metrics-server-id route-metadata) http-client-metric-ids-for-status (atom master-core/puppet-server-http-client-metrics-for-status) + ;; Middleware to sanitize live request route-ids so they match + ;; the sanitized metric names registered during initialization. + wrap-with-route-id-sanitization (fn [handler] + (fn [req] + (if (get-in req [:route-info :route-id]) + (handler (update-in req [:route-info :route-id] sanitize-metrics-route-id)) + (handler req)))) ring-handler (-> comidi-handler (http-metrics/wrap-with-request-metrics http-metrics) + wrap-with-route-id-sanitization (comidi/wrap-with-route-metadata routes)) hostcrl (get-in config [:puppetserver :hostcrl])] (log-java-deprecation-message (System/getProperty "java.version")) diff --git a/src/clj/puppetlabs/services/puppet_profiler/puppet_profiler_core.clj b/src/clj/puppetlabs/services/puppet_profiler/puppet_profiler_core.clj index 9b1dd598f..e9ad7c5b0 100644 --- a/src/clj/puppetlabs/services/puppet_profiler/puppet_profiler_core.clj +++ b/src/clj/puppetlabs/services/puppet_profiler/puppet_profiler_core.clj @@ -1,5 +1,5 @@ (ns puppetlabs.services.puppet-profiler.puppet-profiler-core - (:import (com.codahale.metrics MetricRegistry) + (:import (io.dropwizard.metrics5 MetricRegistry) (com.puppetlabs.puppetserver MetricsPuppetProfiler PuppetProfiler)) (:require [clojure.string :as str] [schema.core :as schema] diff --git a/src/java/com/puppetlabs/puppetserver/MetricsPuppetProfiler.java b/src/java/com/puppetlabs/puppetserver/MetricsPuppetProfiler.java index 0eff3ea47..e5b576e1c 100644 --- a/src/java/com/puppetlabs/puppetserver/MetricsPuppetProfiler.java +++ b/src/java/com/puppetlabs/puppetserver/MetricsPuppetProfiler.java @@ -1,7 +1,7 @@ package com.puppetlabs.puppetserver; -import com.codahale.metrics.MetricRegistry; -import com.codahale.metrics.Timer; +import io.dropwizard.metrics5.MetricRegistry; +import io.dropwizard.metrics5.Timer; import java.util.ArrayList; import java.util.Arrays; @@ -190,8 +190,14 @@ private Map getOrCreateTimersByIDs(String[] metric_id) { } private String getMetricName(List metric_id) { - metric_id.add(0, hostname); - return MetricRegistry.name("puppetlabs", metric_id.toArray(new String[metric_id.size()])); + StringBuilder sb = new StringBuilder(); + sb.append("puppetlabs.").append(hostname); + for (String segment : metric_id) { + if (segment != null && !segment.isEmpty()) { + sb.append('.').append(segment); + } + } + return sb.toString(); } private void registerMetricName(String metric_name) { diff --git a/test/integration/puppetlabs/services/jruby/jruby_puppet_pool_int_test.clj b/test/integration/puppetlabs/services/jruby/jruby_puppet_pool_int_test.clj index d8b43fd32..c2baa45fc 100644 --- a/test/integration/puppetlabs/services/jruby/jruby_puppet_pool_int_test.clj +++ b/test/integration/puppetlabs/services/jruby/jruby_puppet_pool_int_test.clj @@ -32,7 +32,7 @@ [puppetlabs.trapperkeeper.services.protocols.metrics :as metrics-protocol] [puppetlabs.trapperkeeper.testutils.bootstrap :as tk-testutils] [puppetlabs.trapperkeeper.testutils.logging :as logutils]) - (:import (com.codahale.metrics MetricRegistry) + (:import (io.dropwizard.metrics5 MetricRegistry) (java.io ByteArrayOutputStream) (org.jruby RubyInstanceConfig$CompileMode RubyInstanceConfig$ProfilingMode) (org.jruby.embed EvalFailedException))) diff --git a/test/integration/puppetlabs/services/master/master_service_test.clj b/test/integration/puppetlabs/services/master/master_service_test.clj index 0c1059a2e..460ee97dc 100644 --- a/test/integration/puppetlabs/services/master/master_service_test.clj +++ b/test/integration/puppetlabs/services/master/master_service_test.clj @@ -1,7 +1,7 @@ (ns puppetlabs.services.master.master-service-test (:require [clojure.test :refer [deftest is testing use-fixtures]] - ; [puppetlabs.services.master.master-service :refer :all] + [puppetlabs.services.master.master-service :refer [sanitize-route-metadata-for-metrics]] [cheshire.core :as json] [clojure.set :as setutils] [clojure.string :as str] @@ -152,13 +152,13 @@ (is (= 1 (-> http-metrics :route-timers :other .getCount))) (is (= 1 (-> http-metrics :route-timers - (get "puppet-v3-node-/*/") + (get "puppet-v3-node-/ANY/") .getCount))) (is (= 1 (-> http-metrics :route-timers - (get "puppet-v3-catalog-/*/") + (get "puppet-v3-catalog-/ANY/") .getCount))) (is (= 0 (-> http-metrics :route-timers - (get "puppet-v3-report-/*/") + (get "puppet-v3-report-/ANY/") .getCount))) (testing "Catalog compilation increments catalog metrics and adds timing data" (let [profiler-status (puppet-profiler-core/v1-status puppet-profiler :debug) @@ -196,13 +196,13 @@ (is (nil? (schema/check master-core/MasterStatusV1 (get-in status [:master :status])))) (testing "HTTP metrics in status endpoint are sorted in order of aggregate amount of time spent" - (let [hit-routes #{"total" "puppet-v3-node-/*/" - "puppet-v3-catalog-/*/" "other"} + (let [hit-routes #{"total" "puppet-v3-node-/ANY/" + "puppet-v3-catalog-/ANY/" "other"} http-metrics (get-in status [:master :status :experimental :http-metrics])] (testing "'total' should come first since it is the sum of the other endpoints" (is (= "total" (:route-id (first http-metrics))))) (testing "The other two routes that actually received requests should come next" - (is (= #{"puppet-v3-node-/*/" "puppet-v3-catalog-/*/"} + (is (= #{"puppet-v3-node-/ANY/" "puppet-v3-catalog-/ANY/"} (set (map :route-id (rest (take 3 http-metrics))))))) (testing "The aggregate times should be in descending order" (let [aggregate-times (map :aggregate http-metrics)] @@ -211,8 +211,8 @@ (let [find-route (fn [route-metrics route-id] (first (filter #(= (:route-id %) route-id) route-metrics)))] (is (= 3 (:count (find-route http-metrics "total")))) - (is (= 1 (:count (find-route http-metrics "puppet-v3-node-/*/")))) - (is (= 1 (:count (find-route http-metrics "puppet-v3-catalog-/*/")))) + (is (= 1 (:count (find-route http-metrics "puppet-v3-node-/ANY/")))) + (is (= 1 (:count (find-route http-metrics "puppet-v3-catalog-/ANY/")))) (is (= 1 (:count (find-route http-metrics "other")))))) (testing "The counts should be zero for endpoints that we didn't hit" (is (every? #(= 0 %) (map :count @@ -310,7 +310,7 @@ (is (= 1 (count requested-instances))) (is (= {:request {:request-method "get" - :route-id "puppet-v3-catalog-/*/" + :route-id "puppet-v3-catalog-/ANY/" :uri "/puppet/v3/catalog/localhost"}} (:reason requested-instance))) (is (>= (:time requested-instance) time-before-second-borrow)) @@ -319,6 +319,50 @@ (+ (:duration-millis requested-instance) (:time requested-instance)))))))))))))) +(deftest test-sanitize-route-metadata-for-metrics + (testing "Flat route map transformation" + (let [input {:route-id "http.puppet-v3-catalog-/*/-requests" :method :get} + output (sanitize-route-metadata-for-metrics input)] + (is (= "http.puppet-v3-catalog-/ANY/-requests" (:route-id output)) + "Should replace the * wildcard with the stable token 'ANY' inside flat maps."))) + + (testing "Deeply nested and complex tree structures (Comidi style)" + (let [complex-tree [{:route-id "http.puppet-v3-environment-/*/-requests" + :nested [{:route-id "http.puppet-v3-environment_classes-/*/-percentage" + :leaf "keep-me-unmodified"} + {:route-id "http.static-route-no-wildcard"}]} + {:route-id "http.puppet-v3-file_content-/*/-requests" + :metadata {:route-id "http.nested-meta-/*/-metric"}}] + + expected [{:route-id "http.puppet-v3-environment-/ANY/-requests" + :nested [{:route-id "http.puppet-v3-environment_classes-/ANY/-percentage" + :leaf "keep-me-unmodified"} + {:route-id "http.static-route-no-wildcard"}]} + {:route-id "http.puppet-v3-file_content-/ANY/-requests" + :metadata {:route-id "http.nested-meta-/ANY/-metric"}}] + + actual (sanitize-route-metadata-for-metrics complex-tree)] + + (is (= expected actual) + "The postwalk function should recursively sanitize every :route-id found at any depth of a complex tree."))) + + (testing "Type resilience and non-target exclusion" + (let [input {:route-id "http.puppet-v3-node-/*/-requests" + :other-wildcard "leave-*-alone" ;; * character present but not a :route-id key + :integer-route 12345 ;; non-string route-id representation fallback + :nil-route-id nil} + actual (sanitize-route-metadata-for-metrics input)] + + (is (= "http.puppet-v3-node-/ANY/-requests" (:route-id actual)) + "Targeted :route-id strings containing wildcards should be sanitized.") + (is (= "leave-*-alone" (:other-wildcard actual)) + "Asterisks belonging to keys other than :route-id must be completely ignored.") + (is (= 12345 (:integer-route actual)) + "Non-string route identifiers should pass through cleanly without causing runtime type exceptions.") + (is (nil? (:nil-route-id actual)) + "Nil values under target keys should pass through without throwing a NullPointerException.")))) + + (def graphite-enabled-config {:metrics {:server-id "localhost" :reporters {:graphite {:update-interval-seconds 5000 @@ -398,9 +442,12 @@ (let [registry (:registry (get-puppetserver-registry-context app)) get-memory-map (fn [mem-type] - (ks/mapvals #(.getValue %) - (filter #(.matches (key %) (format "puppetlabs.localhost.memory.%s.*" mem-type)) - (.getMetrics registry)))) + (->> (.getMetrics registry) + (filter (fn [[k _]] + (re-matches (re-pattern (format "puppetlabs\\.localhost\\.memory\\.%s\\..*" mem-type)) + (str k)))) + (map (fn [[k v]] [(str k) (.getValue v)])) + (into {}))) heap-memory-map (get-memory-map "heap") non-heap-memory-map (get-memory-map "non-heap") total-memory-map (get-memory-map "total")] @@ -431,10 +478,11 @@ (is (every? #(< 0 %) (vals total-memory-map)))) (testing "uptime metric works" - (let [get-uptime (fn [] (-> registry - .getMetrics - (get "puppetlabs.localhost.uptime") - .getValue)) + (let [get-uptime (fn [] + (->> (.getMetrics registry) + (some (fn [[k v]] + (when (= "puppetlabs.localhost.uptime" (str k)) + (.getValue v)))))) uptime (get-uptime)] (is (< 0 uptime)) ;; Make sure uptime can be updated after initialization. diff --git a/test/integration/puppetlabs/services/puppet_profiler/puppet_profiler_metrics_test.clj b/test/integration/puppetlabs/services/puppet_profiler/puppet_profiler_metrics_test.clj new file mode 100644 index 000000000..9b2294b5b --- /dev/null +++ b/test/integration/puppetlabs/services/puppet_profiler/puppet_profiler_metrics_test.clj @@ -0,0 +1,40 @@ +(ns puppetlabs.services.puppet-profiler.puppet-profiler-metrics-test + (:require [clojure.test :refer [deftest testing is]]) + (:import (io.dropwizard.metrics5 MetricRegistry Timer) + (com.puppetlabs.puppetserver MetricsPuppetProfiler))) + +(deftest test-metrics-puppet-profiler-integration + (testing "Metric name construction via start/finish using StringBuilder refactor" + (let [registry (MetricRegistry.) + hostname "localhost.test" + profiler (MetricsPuppetProfiler. hostname registry) + metric-id (into-array String ["compiler" "evaluate" "resource_types"]) + ctx (.start profiler "sample-event" metric-id)] + (.finish profiler ctx "sample-event" metric-id) + (testing "Registers hierarchical timer metrics with correct dotted names" + (let [metrics-map (.getMetrics registry) + registered-names (set (map str (.keySet metrics-map)))] + (is (contains? registered-names "puppetlabs.localhost.test.compiler") + "Should register the 1-segment prefix timer") + (is (contains? registered-names "puppetlabs.localhost.test.compiler.evaluate") + "Should register the 2-segment prefix timer") + (is (contains? registered-names "puppetlabs.localhost.test.compiler.evaluate.resource_types") + "Should register the full 3-segment timer") + (doseq [n ["puppetlabs.localhost.test.compiler" + "puppetlabs.localhost.test.compiler.evaluate" + "puppetlabs.localhost.test.compiler.evaluate.resource_types"]] + (let [metric-key (first (filter #(= (str %) n) (.keySet metrics-map)))] + (is (instance? Timer (.get metrics-map metric-key)) + (str n " should be a Timer instance")))))) + (testing "List mutation isolation - successive calls don't accumulate prefixes" + (let [second-metric-id (into-array String ["parser" "lexing"]) + ctx2 (.start profiler "second-event" second-metric-id)] + (.finish profiler ctx2 "second-event" second-metric-id) + (let [metrics-map (.getMetrics registry) + registered-names (set (map str (.keySet metrics-map)))] + (is (contains? registered-names "puppetlabs.localhost.test.parser.lexing") + "Second independent metric should register correctly") + (is (not (contains? registered-names "puppetlabs.localhost.test.compiler.evaluate.resource_types.parser.lexing")) + "Metric IDs from separate profile events must not bleed into each other") + (is (not (some #(re-find #"puppetlabs\.localhost\.test\.puppetlabs" %) registered-names)) + "The hostname prefix must not accumulate across repeated getMetricName calls"))))))) diff --git a/test/unit/puppetlabs/services/puppet_profiler/puppet_profiler_core_test.clj b/test/unit/puppetlabs/services/puppet_profiler/puppet_profiler_core_test.clj index 684335b4e..8059b7556 100644 --- a/test/unit/puppetlabs/services/puppet_profiler/puppet_profiler_core_test.clj +++ b/test/unit/puppetlabs/services/puppet_profiler/puppet_profiler_core_test.clj @@ -3,7 +3,7 @@ (org.slf4j LoggerFactory) (ch.qos.logback.classic Logger Level) (com.puppetlabs.puppetserver PuppetProfiler) - (com.codahale.metrics MetricRegistry Timer)) + (io.dropwizard.metrics5 MetricRegistry Timer)) (:require [clojure.test :refer [deftest is testing use-fixtures]] [puppetlabs.services.puppet-profiler.puppet-profiler-core :refer [initialize metrics-profiler v1-status]] [puppetlabs.trapperkeeper.testutils.logging :as logutils] @@ -53,13 +53,14 @@ (profile profiler "bye" ["compile" "init-environment"]) (testing "keeps timers for all metrics" (let [metrics-map (.getMetrics registry) + metrics-by-name (into {} (map (fn [[k v]] [(str k) v]) metrics-map)) expected-metrics ["puppetlabs.localhost.function" "puppetlabs.localhost.function.hiera-lookup" "puppetlabs.localhost.compile" "puppetlabs.localhost.compile.init-environment"]] (is (= (set expected-metrics) - (.keySet metrics-map))) - (is (every? #(instance? Timer (.get metrics-map %)) expected-metrics))))))))) + (set (map str (.keySet metrics-map))))) + (is (every? #(instance? Timer (get metrics-by-name %)) expected-metrics))))))))) (deftest test-profiler-via-ruby (let [sc (jruby-internal/create-scripting-container @@ -76,7 +77,7 @@ require 'java' java_import com.puppetlabs.puppetserver.MetricsPuppetProfiler - java_import com.codahale.metrics.MetricRegistry + java_import io.dropwizard.metrics5.MetricRegistry registry = MetricRegistry.new profiler = MetricsPuppetProfiler.new('testhost', registry) @@ -89,7 +90,7 @@ (is (= #{"puppetlabs.testhost.foo" "puppetlabs.testhost.foo.bar" "puppetlabs.testhost.foo.bar.baz"} - (into #{} (.getNames registry))))) + (into #{} (map str (.getNames registry)))))) (finally (.terminate sc)))))