From 77a3f1d14bffa1e23c872d83d67e81e85e483ff5 Mon Sep 17 00:00:00 2001 From: Anil Madhavapeddy Date: Wed, 5 Aug 2026 13:37:14 +0100 Subject: [PATCH] Move the Lwt logic out of the prometheus core Complete the migration started in #66 so the core no longer depends on Lwt. Libraries that only define and record metrics no longer need to pull in a concurrency library. Review suggestions from @talex5 Co-authored-by: Mark Elvers --- CHANGES.md | 9 +++++ README.md | 11 ++++--- app/dune | 2 +- app/prometheus_app.ml | 2 +- lwt/prometheus_lwt.ml | 77 ++++++++++++++++++++++++++++++++++--------- lwt/test/dune | 2 +- prometheus-app.opam | 1 + prometheus-lwt.opam | 4 --- prometheus.opam | 1 - src/dune | 2 +- src/prometheus.ml | 68 +++----------------------------------- src/prometheus.mli | 50 +++++++--------------------- tests/dune | 2 +- tests/test.ml | 21 ++++++------ 14 files changed, 110 insertions(+), 142 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 5a671a2..f6e895e 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,14 @@ ## dev +- Remove Lwt from the `prometheus` core (@mtelvers @avsm #60 #65) + + The deprecated Lwt-typed functions are gone and `CollectorRegistry.collect` + now returns a snapshot directly. The Lwt collectors and timing helpers live + in `prometheus-lwt`, whose interface is unchanged, so code that already + migrated to `Prometheus_lwt` needs no further changes. Applications that + collected via the core in an Lwt context should use + `Prometheus_lwt.CollectorRegistry.collect`. + - Add a new `prometheus-lwt` metrics package (@avsm @mtelvers @talex5 #66 #60 #65) This release is not a breaking change, but instead prepares for diff --git a/README.md b/README.md index 18c4c10..9f5db7e 100644 --- a/README.md +++ b/README.md @@ -71,12 +71,13 @@ Unikernels can use `Prometheus_app` instead of `Prometheus_unix` to avoid the `U The `prometheus-lwt` opam package provides `Prometheus_lwt` with collectors that may suspend before producing their samples, and also Lwt versions of the -timing helpers. +timing helpers. + +The `prometheus` core does not depend on Lwt, so libraries that only define +and record metrics do not pull in a concurrency library. Code that used the +core's Lwt-typed functions should use `Prometheus_lwt`, or the synchronous +variants such as `Gauge.set_time`. -A future release will remove Lwt from the `prometheus` core. Existing code -using the core's Lwt-typed functions should migrate to `Prometheus_lwt`, or to -the synchronous variants such as `Gauge.set_time` now. Code will then keep -working unchanged when the core switch happens. ### API docs diff --git a/app/dune b/app/dune index 5006245..af79bb5 100644 --- a/app/dune +++ b/app/dune @@ -1,7 +1,7 @@ (library (name prometheus_app) (public_name prometheus-app) - (libraries prometheus lwt cohttp-lwt fmt re) + (libraries prometheus prometheus-lwt lwt cohttp-lwt fmt re) (modules Prometheus_app) (wrapped false)) diff --git a/app/prometheus_app.ml b/app/prometheus_app.ml index bd1c831..4a7334c 100644 --- a/app/prometheus_app.ml +++ b/app/prometheus_app.ml @@ -148,7 +148,7 @@ module Cohttp(Server : Cohttp_lwt.S.Server) = struct let uri = Request.uri req in match Request.meth req, Uri.path uri with | `GET, "/metrics" -> - Prometheus.CollectorRegistry.(collect default) >>= fun data -> + Prometheus_lwt.CollectorRegistry.(collect default) >>= fun data -> let body = Fmt.to_to_string TextFormat_0_0_4.output data in let headers = Header.init_with "Content-Type" "text/plain; version=0.0.4" in Server.respond_string ~status:`OK ~headers ~body () diff --git a/lwt/prometheus_lwt.ml b/lwt/prometheus_lwt.ml index d0cd169..2cc7dcc 100644 --- a/lwt/prometheus_lwt.ml +++ b/lwt/prometheus_lwt.ml @@ -1,34 +1,81 @@ -[@@@alert "-deprecated"] +(* The Lwt collectors for Prometheus metrics. *) + +open Prometheus -(* This transition module reexports the core operations under their - new names to allow the core package to drop the Lwt dependency - in a future release. This interface will not change. *) module CollectorRegistry = struct - type t = Prometheus.CollectorRegistry.t + type t = { + core : Prometheus.CollectorRegistry.t; + mutable metrics_lwt : (unit -> Sample_set.t LabelSetMap.t Lwt.t) MetricFamilyMap.t; + mutable pre_collect_lwt : (unit -> unit Lwt.t) list; + } + + let of_registry core = { + core; + metrics_lwt = MetricFamilyMap.empty; + pre_collect_lwt = []; + } + + let default = of_registry Prometheus.CollectorRegistry.default - let of_registry core = core + let core t = t.core - let default = Prometheus.CollectorRegistry.default + let register_pre_collect t f = t.pre_collect_lwt <- f :: t.pre_collect_lwt - let core t = t + let register t info collector = + if MetricFamilyMap.mem info t.metrics_lwt then + failwith (Format.asprintf "%a already registered" MetricName.pp info.MetricInfo.name); + t.metrics_lwt <- MetricFamilyMap.add info collector t.metrics_lwt - let collect = Prometheus.CollectorRegistry.collect + open Lwt.Infix - let register = Prometheus.CollectorRegistry.register_lwt + let map_p m = + MetricFamilyMap.fold (fun k f acc -> (k, f ()) :: acc) m [] + |> Lwt_list.fold_left_s + (fun acc (k, v) -> v >|= fun v -> MetricFamilyMap.add k v acc) + MetricFamilyMap.empty - let register_pre_collect = Prometheus.CollectorRegistry.register_pre_collect_lwt + let collect t = + Lwt_list.iter_p (fun f -> f ()) t.pre_collect_lwt >>= fun () -> + let sync = Prometheus.CollectorRegistry.collect t.core in + map_p t.metrics_lwt >|= fun metrics_lwt -> + MetricFamilyMap.merge + (fun _ v1 v2 -> + match v1 with + | Some v1 -> Some v1 + | None -> v2) + sync metrics_lwt end module Gauge = struct - let track_in_progress = Prometheus.Gauge.track_inprogress + let track_in_progress t fn = + Prometheus.Gauge.inc_one t; + Lwt.finalize fn (fun () -> Prometheus.Gauge.dec_one t; Lwt.return_unit) - let set_time = Prometheus.Gauge.time + let set_time t gettimeofday fn = + let start = gettimeofday () in + Lwt.finalize fn + (fun () -> + let finish = gettimeofday () in + Prometheus.Gauge.set t (finish -. start); + Lwt.return_unit) end module Summary = struct - let observe_time = Prometheus.Summary.time + let observe_time t gettimeofday fn = + let start = gettimeofday () in + Lwt.finalize fn + (fun () -> + let finish = gettimeofday () in + Prometheus.Summary.observe t (finish -. start); + Lwt.return_unit) end module Histogram (H : Prometheus.HISTOGRAM) = struct - let observe_time = H.time + let observe_time t gettimeofday fn = + let start = gettimeofday () in + Lwt.finalize fn + (fun () -> + let finish = gettimeofday () in + H.observe t (finish -. start); + Lwt.return_unit) end diff --git a/lwt/test/dune b/lwt/test/dune index a15a2c3..d0a2a73 100644 --- a/lwt/test/dune +++ b/lwt/test/dune @@ -1,4 +1,4 @@ (test (name test_lwt) - (package prometheus-lwt) + (package prometheus-app) (libraries prometheus prometheus-app prometheus-lwt alcotest alcotest-lwt lwt fmt)) diff --git a/prometheus-app.opam b/prometheus-app.opam index 8755858..ab6a90c 100644 --- a/prometheus-app.opam +++ b/prometheus-app.opam @@ -28,6 +28,7 @@ depends: [ "ocaml" {>= "4.11.0"} "dune" {>= "2.3"} "prometheus" {= version} + "prometheus-lwt" {= version} "fmt" {>= "0.8.7"} "re" "cohttp-lwt" {>= "4.0.0"} diff --git a/prometheus-lwt.opam b/prometheus-lwt.opam index 4b40ac6..c3be667 100644 --- a/prometheus-lwt.opam +++ b/prometheus-lwt.opam @@ -12,10 +12,6 @@ depends: [ "dune" {>= "2.3"} "prometheus" {= version} "lwt" {>= "2.5.0"} - "prometheus-app" {= version & with-test} - "fmt" {>= "0.8.7" & with-test} - "alcotest" {with-test} - "alcotest-lwt" {with-test} ] build: [ ["dune" "build" "-p" name "-j" jobs] diff --git a/prometheus.opam b/prometheus.opam index 287b197..fca6233 100644 --- a/prometheus.opam +++ b/prometheus.opam @@ -10,7 +10,6 @@ depends: [ "ocaml" {>= "4.11.0"} "dune" {>= "2.3"} "re" - "lwt" {>= "2.5.0"} ] build: [ ["dune" "build" "-p" name "-j" jobs] diff --git a/src/dune b/src/dune index e9f6946..4848dca 100644 --- a/src/dune +++ b/src/dune @@ -1,4 +1,4 @@ (library (name prometheus) (public_name prometheus) - (libraries lwt re)) + (libraries re)) diff --git a/src/prometheus.ml b/src/prometheus.ml index 4b28582..ede104f 100644 --- a/src/prometheus.ml +++ b/src/prometheus.ml @@ -98,60 +98,32 @@ end module CollectorRegistry = struct type t = { - mutable metrics : (unit -> Sample_set.t LabelSetMap.t ) MetricFamilyMap.t; - mutable metrics_lwt : (unit -> Sample_set.t LabelSetMap.t Lwt.t) MetricFamilyMap.t; - mutable pre_collect : (unit -> unit ) list; - mutable pre_collect_lwt : (unit -> unit Lwt.t) list; + mutable metrics : (unit -> Sample_set.t LabelSetMap.t) MetricFamilyMap.t; + mutable pre_collect : (unit -> unit) list; } type snapshot = Sample_set.t LabelSetMap.t MetricFamilyMap.t let create () = { metrics = MetricFamilyMap.empty; - metrics_lwt = MetricFamilyMap.empty; pre_collect = []; - pre_collect_lwt = []; } let default = create () let register_pre_collect t f = t.pre_collect <- f :: t.pre_collect - let register_pre_collect_lwt t f = t.pre_collect_lwt <- f :: t.pre_collect_lwt - let ensure_not_registered t info = - if MetricFamilyMap.mem info t.metrics || - MetricFamilyMap.mem info t.metrics_lwt + if MetricFamilyMap.mem info t.metrics then failwith (Format.asprintf "%a already registered" MetricName.pp info.MetricInfo.name) let register t info collector = ensure_not_registered t info; t.metrics <- MetricFamilyMap.add info collector t.metrics - let register_lwt t info collector = - ensure_not_registered t info; - t.metrics_lwt <- MetricFamilyMap.add info collector t.metrics_lwt - - open Lwt.Infix - - let map_p m = - MetricFamilyMap.fold (fun k f acc -> (k, f ()) :: acc) m [] - |> Lwt_list.fold_left_s - (fun acc (k, v) -> v >|= fun v -> MetricFamilyMap.add k v acc) - MetricFamilyMap.empty - let collect t = List.iter (fun f -> f ()) t.pre_collect; - Lwt_list.iter_p (fun f -> f ()) t.pre_collect_lwt >>= fun () -> - let metrics = MetricFamilyMap.map (fun f -> f ()) t.metrics in - map_p t.metrics_lwt >|= fun metrics_lwt -> - MetricFamilyMap.merge - (fun _ v1 v2 -> - match v1 with - | Some v1 -> Some v1 - | None -> v2) - metrics metrics_lwt - + MetricFamilyMap.map (fun f -> f ()) t.metrics end module type METRIC = sig @@ -249,23 +221,10 @@ module Gauge = struct let set t v = t := v - let track_inprogress t fn = - inc_one t; - Lwt.finalize fn (fun () -> dec_one t; Lwt.return_unit) - let track_in_progress t fn = inc_one t; Fun.protect fn ~finally:(fun () -> dec_one t) - let time t gettimeofday fn = - let start = gettimeofday () in - Lwt.finalize fn - (fun () -> - let finish = gettimeofday () in - inc t (finish -. start); - Lwt.return_unit - ) - let set_time t gettimeofday fn = let start = gettimeofday () in Fun.protect fn @@ -300,15 +259,6 @@ module Summary = struct t.count <- t.count +. 1.0; t.sum <- t.sum +. v - let time t gettimeofday fn = - let start = gettimeofday () in - Lwt.finalize fn - (fun () -> - let finish = gettimeofday () in - observe t (finish -. start); - Lwt.return_unit - ) - let observe_time t gettimeofday fn = let start = gettimeofday () in Fun.protect fn @@ -364,7 +314,6 @@ end module type HISTOGRAM = sig include METRIC val observe : t -> float -> unit - val time : t -> (unit -> float) -> (unit -> 'a Lwt.t) -> 'a Lwt.t [@@deprecated] val observe_time : t -> (unit -> float) -> (unit -> 'a) -> 'a end @@ -413,15 +362,6 @@ module Histogram (Buckets : BUCKETS) = struct t.counts.(index) <- t.counts.(index) +. 1.; t.sum <- t.sum +. v - let time t gettimeofday fn = - let start = gettimeofday () in - Lwt.finalize fn - (fun () -> - let finish = gettimeofday () in - observe t (finish -. start); - Lwt.return_unit - ) - let observe_time t gettimeofday fn = let start = gettimeofday () in Fun.protect fn diff --git a/src/prometheus.mli b/src/prometheus.mli index 14e1aa1..b46355b 100644 --- a/src/prometheus.mli +++ b/src/prometheus.mli @@ -5,14 +5,13 @@ Notes: - The Prometheus docs require that client libraries are thread-safe. We interpret this to mean safe - with Lwt threads, NOT with native threading. + with cooperatively-scheduled threads (e.g. Lwt), NOT with native threading. - This library is intended to be a dependency of any library that might need to report metrics, even though many applications will not enable it. Therefore it should have minimal dependencies. - - The Lwt-typed functions here are superseded by the [prometheus-lwt] - package. A future release will remove Lwt from this library. New code - should use [Prometheus_lwt] or the synchronous variants. + - Lwt applications should use the [prometheus-lwt] package to collect + metrics and to register collectors that suspend. *) type metric_type = @@ -92,26 +91,17 @@ module CollectorRegistry : sig val default : t (** The default registry. *) - val collect : t -> snapshot Lwt.t + val collect : t -> snapshot (** Read the current value of each metric. *) val register : t -> MetricInfo.t -> (unit -> Sample_set.t LabelSetMap.t) -> unit (** [register t metric collector] adds [metric] to the set of metrics being collected. It will call [collector ()] to collect the values each time [collect] is called. *) - val register_lwt : t -> MetricInfo.t -> (unit -> Sample_set.t LabelSetMap.t Lwt.t) -> unit - [@@deprecated "Use Prometheus_lwt.CollectorRegistry.register instead."] - (** [register_lwt t metric collector] is the same as [register t metrics collector] - but [collector] returns [Sample_set.t LabelSetMap.t Lwt.t]. *) - val register_pre_collect : t -> (unit -> unit) -> unit (** [register_pre_collect t fn] arranges for [fn ()] to be called at the start of each collection. This is useful if one expensive call provides information about multiple metrics. *) - - val register_pre_collect_lwt : t -> (unit -> unit Lwt.t) -> unit - [@@deprecated "Use Prometheus_lwt.CollectorRegistry.register_pre_collect instead."] - (** [register_pre_collect_lwt t fn] is like [register_pre_collect] but [fn] returns [unit Lwt.t]. *) end (** A collection of metric reporters. Usually, only {!CollectorRegistry.default} is used. *) @@ -167,22 +157,14 @@ module Gauge : sig val set : t -> float -> unit (** [set t v] sets the current value of the gauge to [v]. *) - val track_inprogress : t -> (unit -> 'a Lwt.t) -> 'a Lwt.t - [@@deprecated "Use track_in_progress (with two underscores in the name) instead if you don't need Lwt here, \ - or Prometheus_lwt.Gauge.track_in_progress if you do."] - (** [track_inprogress t f] increases the value of the gauge by one while [f ()] is running. *) - val track_in_progress : t -> (unit -> 'a) -> 'a - (** [track_in_progress t f] increases the value of the gauge by one while [f ()] runs. *) - - val time : t -> (unit -> float) -> (unit -> 'a Lwt.t) -> 'a Lwt.t - [@@deprecated "This increments the gauge, which is probably not what you want. Use set_time instead."] - (** [time t gettime f] calls [gettime ()] before and after executing [f ()] and - increases the metric by the difference. *) + (** [track_in_progress t f] increases the value of the gauge by one while [f ()] runs. + Use [Prometheus_lwt.Gauge.track_in_progress] to track an Lwt thread. *) val set_time : t -> (unit -> float) -> (unit -> 'a) -> 'a (** [set_time t gettime f] calls [gettime ()] before and after executing [f ()] and - sets [t] to the difference. *) + sets [t] to the difference. + Use [Prometheus_lwt.Gauge.set_time] to time an Lwt thread. *) end (** A gauge is a metric that represents a single numerical value that can arbitrarily go up and down. *) @@ -192,14 +174,10 @@ module Summary : sig val observe : t -> float -> unit (** [observe t v] increases the total by [v] and the count by one. *) - val time : t -> (unit -> float) -> (unit -> 'a Lwt.t) -> 'a Lwt.t - [@@deprecated "Use observe_time instead if you don't need Lwt here, or Prometheus_lwt.Summary.observe_time if you do."] - (** [time t gettime f] calls [gettime ()] before and after executing [f ()] and - observes the difference. *) - val observe_time : t -> (unit -> float) -> (unit -> 'a) -> 'a (** [observe_time t gettime f] calls [gettime ()] before and after executing [f ()] and - observes the difference. *) + observes the difference. + Use [Prometheus_lwt.Summary.observe_time] to time an Lwt thread. *) end (** A summary is a metric that records both the number of readings and their total. This allows calculating the average. *) @@ -230,14 +208,10 @@ module type HISTOGRAM = sig val observe : t -> float -> unit (** [observe t v] adds one to the appropriate bucket for v and adds v to the sum. *) - val time : t -> (unit -> float) -> (unit -> 'a Lwt.t) -> 'a Lwt.t - [@@deprecated "Use observe_time instead if you don't need Lwt here, or Prometheus_lwt.HISTOGRAM.observe_time if you do."] - (** [time t gettime f] calls [gettime ()] before and after executing [f ()] and - observes the difference. *) - val observe_time : t -> (unit -> float) -> (unit -> 'a) -> 'a (** [observe_time t gettime f] calls [gettime ()] before and after executing [f ()] and - observes the difference. *) + observes the difference. + Use [Prometheus_lwt.Histogram.observe_time] to time an Lwt thread. *) end module Histogram (Buckets : sig val spec : Histogram_spec.t end) : HISTOGRAM diff --git a/tests/dune b/tests/dune index 8a23e94..6b92551 100644 --- a/tests/dune +++ b/tests/dune @@ -1,4 +1,4 @@ (test (name test) (package prometheus-app) - (libraries prometheus prometheus-app alcotest alcotest-lwt)) + (libraries prometheus prometheus-app alcotest)) diff --git a/tests/test.ml b/tests/test.ml index 9848dec..9222e45 100644 --- a/tests/test.ml +++ b/tests/test.ml @@ -1,7 +1,8 @@ open Prometheus open Prometheus_app -open Lwt.Infix +(* These tests exercise the backend-free core: recording and synchronous + collection. No Lwt anywhere. *) let test_metrics () = let registry = CollectorRegistry.create () in @@ -18,7 +19,7 @@ let test_metrics () = Counter.inc post_login 2.; let post_login2 = Counter.labels requests ["POST"; "/login"] in Counter.inc_one post_login2; - CollectorRegistry.collect registry >|= fun collected -> + let collected = CollectorRegistry.collect registry in let output = Fmt.to_to_string TextFormat_0_0_4.output collected in Alcotest.(check string) "Text output" "# HELP dkci_tests_requests Requests\n\ @@ -49,7 +50,7 @@ let test_collectors () = in register_counter ~name:"counter_1" ~help:"The first counter" 1.0; register_counter ~name:"counter_2" ~help:"The second counter" 2.0; - CollectorRegistry.collect registry >|= fun collected -> + let collected = CollectorRegistry.collect registry in let output = Fmt.to_to_string TextFormat_0_0_4.output collected in Alcotest.(check string) "Text output" "# HELP counter_1 The first counter\n\ @@ -75,7 +76,7 @@ let test_histogram () = let bar = H.labels requests ["PUT"; "/bar"] in H.observe foo 0.12; H.observe bar 0.33; - CollectorRegistry.collect registry >|= fun collected -> + let collected = CollectorRegistry.collect registry in let output = Fmt.to_to_string TextFormat_0_0_4.output collected in Alcotest.(check string) "Text output" "# HELP dkci_tests_requests Requests\n\ @@ -104,7 +105,7 @@ let test_sync_timers () = Gauge.track_in_progress gauge (fun () -> ()); Summary.observe_time summary gettime (fun () -> clock := !clock +. 0.5); Summary.observe_time summary gettime (fun () -> clock := !clock +. 1.5); - CollectorRegistry.collect registry >|= fun collected -> + let collected = CollectorRegistry.collect registry in let output = Fmt.to_to_string TextFormat_0_0_4.output collected in Alcotest.(check string) "Text output" "# HELP gauge_time Time taken\n\ @@ -144,11 +145,11 @@ let check_invalid_label label () = failwith (label ^ " should be an invalid label") let test_valid_labels_set = List.map (fun label -> - label, `Quick, fun () -> Lwt.return @@ check_valid_label label () + label, `Quick, check_valid_label label ) valid_labels let test_invalid_labels_set = List.map (fun label -> - label, `Quick, fun () -> Lwt.return @@ check_invalid_label label () + label, `Quick, check_invalid_label label ) invalid_labels let check_valid_metric metric () = @@ -179,11 +180,11 @@ let invalid_metrics = [ ] let test_valid_metrics_set = List.map (fun metric -> - metric, `Quick, fun () -> Lwt.return @@ check_valid_metric metric () + metric, `Quick, check_valid_metric metric ) valid_metrics let test_invalid_metrics_set = List.map (fun metric -> - metric, `Quick, fun () -> Lwt.return @@ check_invalid_metric metric () + metric, `Quick, check_invalid_metric metric ) invalid_metrics let test_set = [ @@ -194,7 +195,7 @@ let test_set = [ ] let () = - Lwt_main.run @@ Alcotest_lwt.run "prometheus" [ + Alcotest.run "prometheus" [ "main", test_set; "valid_labels", test_valid_labels_set; "invalid_labels", test_invalid_labels_set;