Weighted histogram (#155)

* Add prometheus_histogram:observe_n/3,4,5 api calls This expected to help in collecting weighted histograms, when we need to update bucket adding given positive integer number (aka wight or count) * Fix weighted histogram implementation - add support to non-integer values - count the sum correctly (with weights) - add test cases - adjust documentation * Rename "Weights" to "Counts", cleanup the doc.
deadtrickster · Nov 23, 2023 · d1aad4a · d1aad4a
1 parent 06c2672
commit d1aad4a
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 23 deletions.
diff --git a/src/metrics/prometheus_histogram.erl b/src/metrics/prometheus_histogram.erl
@@ -29,6 +29,14 @@
 %%                                [Method], Time).
 %%
 %% </pre>
+%%
+%% The `prometheus_histogram:observe_n/3,4,5' adds limited support for the
+%% "weighted" histograms. It accepts the extra integer argument "Count" to
+%% update the number of observations in the bucket by adding that number.
+%% This allows for better accuracy in the case of irregular measurements,
+%% assuming that the "Count" conveys the observation time interval (for
+%% example, the number of time ticks when the recent value was observed).
+%%
 %% @end
 
 -module(prometheus_histogram).
@@ -42,6 +50,9 @@
          observe/2,
          observe/3,
          observe/4,
+         observe_n/3,
+         observe_n/4,
+         observe_n/5,
          pobserve/6,
          observe_duration/2,
          observe_duration/3,
@@ -184,38 +195,58 @@ observe(Name, LabelValues, Value) ->
 
 %% @doc Observes the given `Value'.
 %%
-%% Raises `{invalid_value, Value, Message}' if `Value'
-%% isn't an integer.<br/>
+%% Raises `{invalid_value, Value, Message}' if `Value' isn't a number.<br/>
+%% Raises `{unknown_metric, Registry, Name}' error if histogram with named
+%% `Name' can't be found in `Registry'.<br/>
+%% Raises `{invalid_metric_arity, Present, Expected}' error if labels count
+%% mismatch.
+%% @end
+observe(Registry, Name, LabelValues, Value) when is_number(Value) ->
+  observe_n(Registry, Name, LabelValues, Value, 1);
+observe(_Registry, _Name, _LabelValues, Value) ->
+  erlang:error({invalid_value, Value, "observe accepts only numbers"}).
+
+%% @equiv observe_n(default, Name, [], Value, Count)
+observe_n(Name, Value, Count) ->
+  observe_n(default, Name, [], Value, Count).
+
+%% @equiv observe_n(default, Name, LabelValues, Value, Count)
+observe_n(Name, LabelValues, Value, Count) ->
+  observe_n(default, Name, LabelValues, Value, Count).
+
+%% @doc Observes the given `Value' `Count' times.
+%%
+%% Raises `{invalid_value, Value, Message}' if `Value' isn't a number.<br/>
+%% Raises `{invalid_count, Count, Message}' if `Count' isn't integer.<br/>
 %% Raises `{unknown_metric, Registry, Name}' error if histogram with named
 %% `Name' can't be found in `Registry'.<br/>
 %% Raises `{invalid_metric_arity, Present, Expected}' error if labels count
 %% mismatch.
 %% @end
-observe(Registry, Name, LabelValues, Value) when is_integer(Value) ->
+observe_n(Registry, Name, LabelValues, Value, Count) when is_integer(Value), is_integer(Count) ->
   Key = key(Registry, Name, LabelValues),
   case ets:lookup(?TABLE, Key) of
     [Metric] ->
       BucketPosition = calculate_histogram_bucket_position(Metric, Value),
       ets:update_counter(?TABLE, Key,
-                         [{?ISUM_POS, Value},
-                          {?BUCKETS_START + BucketPosition, 1}]);
+                         [{?ISUM_POS, Value * Count},
+                          {?BUCKETS_START + BucketPosition, Count}]);
     [] ->
-      insert_metric(Registry, Name, LabelValues, Value, fun observe/4)
+      insert_metric(Registry, Name, LabelValues, Value, Count, fun observe_n/5)
   end,
   ok;
-observe(Registry, Name, LabelValues, Value) when is_number(Value) ->
+observe_n(Registry, Name, LabelValues, Value, Count) when is_number(Value), is_integer(Count) ->
   Key = key(Registry, Name, LabelValues),
   case ets:lookup(?TABLE, Key) of
     [Metric] ->
-      fobserve_impl(Key, Metric, Value);
+      fobserve_impl(Key, Metric, Value, Count);
     [] ->
-      insert_metric(Registry, Name, LabelValues, Value,
-                    fun(_, _, _, _) ->
-                        observe(Registry, Name, LabelValues, Value)
-                    end)
+      insert_metric(Registry, Name, LabelValues, Value, Count, fun observe_n/5)
   end;
-observe(_Registry, _Name, _LabelValues, Value) ->
-  erlang:error({invalid_value, Value, "observe accepts only numbers"}).
+observe_n(_Registry, _Name, _LabelValues, Value, Count) when is_number(Value) ->
+  erlang:error({invalid_count, Count, "observe_n accepts only integer counts"});
+observe_n(_Registry, _Name, _LabelValues, Value, _Count) ->
+  erlang:error({invalid_value, Value, "observe_n accepts only number values"}).
 
 %% @private
 pobserve(Registry, Name, LabelValues, Buckets, BucketPos, Value) when is_integer(Value) ->
@@ -234,11 +265,11 @@ pobserve(Registry, Name, LabelValues, Buckets, BucketPos, Value) when is_integer
 pobserve(Registry, Name, LabelValues, Buckets, BucketPos, Value) when is_number(Value) ->
   Key = key(Registry, Name, LabelValues),
   case
-    fobserve_impl(Key, Buckets, BucketPos, Value) of
+    fobserve_impl(Key, Buckets, BucketPos, Value, 1) of
     0 ->
       insert_metric(Registry, Name, LabelValues, Value,
                     fun(_, _, _, _) ->
-                        fobserve_impl(Key, Buckets, BucketPos, Value)
+                        fobserve_impl(Key, Buckets, BucketPos, Value, 1)
                     end);
     1 ->
       ok
@@ -431,13 +462,17 @@ insert_metric(Registry, Name, LabelValues, Value, CB) ->
   insert_placeholders(Registry, Name, LabelValues),
   CB(Registry, Name, LabelValues, Value).
 
-fobserve_impl(Key, Metric, Value) ->
+insert_metric(Registry, Name, LabelValues, Value, Count, CB) ->
+  insert_placeholders(Registry, Name, LabelValues),
+  CB(Registry, Name, LabelValues, Value, Count).
+
+fobserve_impl(Key, Metric, Value, Count) ->
   Buckets = metric_buckets(Metric),
   BucketPos = calculate_histogram_bucket_position(Metric, Value),
-  fobserve_impl(Key, Buckets, BucketPos, Value).
+  fobserve_impl(Key, Buckets, BucketPos, Value, Count).
 
-fobserve_impl(Key, Buckets, BucketPos, Value) ->
-  ets:select_replace(?TABLE, generate_select_replace(Key, Buckets, BucketPos, Value)).
+fobserve_impl(Key, Buckets, BucketPos, Value, Count) ->
+  ets:select_replace(?TABLE, generate_select_replace(Key, Buckets, BucketPos, Value, Count)).
 
 insert_placeholders(Registry, Name, LabelValues) ->
   MF = prometheus_metric:check_mf_exists(?TABLE, Registry, Name, LabelValues),
@@ -456,13 +491,13 @@ calculate_histogram_bucket_position(Metric, Value) ->
   Buckets = metric_buckets(Metric),
   prometheus_buckets:position(Buckets, Value).
 
-generate_select_replace(Key, Bounds, BucketPos, Value) ->
+generate_select_replace(Key, Bounds, BucketPos, Value, Count) ->
   BoundPlaceholders = gen_query_bound_placeholders(Bounds),
   HistMatch = list_to_tuple([Key, '$2', '$3', '$4'] ++ BoundPlaceholders),
   BucketUpdate = lists:sublist(BoundPlaceholders, BucketPos)
-    ++ [{'+', gen_query_placeholder(?BUCKETS_START + BucketPos), 1}]
+    ++ [{'+', gen_query_placeholder(?BUCKETS_START + BucketPos), Count}]
     ++ lists:nthtail(BucketPos + 1, BoundPlaceholders),
-  HistUpdate = list_to_tuple([{Key}, '$2', '$3', {'+', '$4', Value}] ++ BucketUpdate),
+  HistUpdate = list_to_tuple([{Key}, '$2', '$3', {'+', '$4', Value * Count}] ++ BucketUpdate),
   [{HistMatch,
     [],
     [{HistUpdate}]}].

diff --git a/test/eunit/metric/prometheus_histogram_tests.erl b/test/eunit/metric/prometheus_histogram_tests.erl
@@ -12,6 +12,7 @@ prometheus_format_test_() ->
     fun test_errors/1,
     fun test_buckets/1,
     fun test_observe/1,
+    fun test_observe_n/0,
     fun test_observe_duration_seconds/1,
     fun test_observe_duration_milliseconds/1,
     fun test_deregister/1,
@@ -56,8 +57,12 @@ test_errors(_) ->
    %% mf/arity errors
    ?_assertError({unknown_metric, default, unknown_metric},
                  prometheus_histogram:observe(unknown_metric, 1)),
+   ?_assertError({unknown_metric, default, unknown_metric},
+                 prometheus_histogram:observe_n(unknown_metric, 1, 1)),
    ?_assertError({invalid_metric_arity, 2, 1},
                  prometheus_histogram:observe(db_query_duration, [repo, db], 1)),
+   ?_assertError({invalid_metric_arity, 2, 1},
+                 prometheus_histogram:observe_n(db_query_duration, [repo, db], 1, 1)),
    ?_assertError({unknown_metric, default, unknown_metric},
                  prometheus_histogram:observe_duration(unknown_metric, fun() -> 1 end)),
    ?_assertError({invalid_metric_arity, 2, 1},
@@ -100,6 +105,10 @@ test_errors(_) ->
                                            {buckets, [1, 3, 2]}])),
    ?_assertError({invalid_value, "qwe", "observe accepts only numbers"},
                  prometheus_histogram:observe(request_duration, "qwe")),
+   ?_assertError({invalid_value, "qwe", "observe_n accepts only number values"},
+                 prometheus_histogram:observe_n(request_duration, "qwe", 3)),
+   ?_assertError({invalid_count, "qwe", "observe_n accepts only integer counts"},
+                 prometheus_histogram:observe_n(request_duration, 300, "qwe")),
    ?_assertError({invalid_value, "qwe", "observe_duration accepts only functions"},
                  prometheus_histogram:observe_duration(pool_size, "qwe"))
   ].
@@ -167,6 +176,21 @@ test_observe(_) ->
                  when Sum > 6974.5 andalso Sum < 6974.55, Value),
    ?_assertEqual({[0, 0, 0, 0, 0, 0], 0}, RValue)].
 
+test_observe_n() ->
+  prometheus_histogram:new([{name, temp}, {help, "temp"}, {buckets, [10, 20, 30, 40, 50]}]),
+  ?assertEqual({[0, 0, 0, 0, 0, 0], 0}, prometheus_histogram:value(temp)),
+
+  prometheus_histogram:observe_n(temp, 5.5, 2), Sum1 = 5.5 * 2,
+  ?assertEqual({[2, 0, 0, 0, 0, 0], Sum1}, prometheus_histogram:value(temp)),
+
+  prometheus_histogram:observe(temp, 15.5), Sum2 = Sum1 + 15.5,
+  ?assertEqual({[2, 1, 0, 0, 0, 0], Sum2}, prometheus_histogram:value(temp)),
+
+  prometheus_histogram:observe(temp, 1), Sum3 = Sum2 + 1,
+  ?assertEqual({[3, 1, 0, 0, 0, 0], Sum3}, prometheus_histogram:value(temp)),
+
+  ok.
+
 test_observe_duration_seconds(_) ->
   prometheus_histogram:new([{name, fun_duration_seconds},
                             {buckets, [0.5, 1.1]},