Fix appending custom values to specs (#187)

Fixes a bug introduced at #181 when we introduced maps as specs, as histograms and summaries would append values as if they were still lists.

Author: NelsonVides

src/metrics/prometheus_histogram.erl

@@ -520,7 +520,7 @@ validate_histogram_spec(Spec) ->
     validate_histogram_labels(Labels),
     RBuckets = prometheus_metric_spec:get_value(buckets, Spec, default),
     Buckets = prometheus_buckets:new(RBuckets),
-    [{data, Buckets} | Spec].
+    prometheus_metric_spec:add_value(data, Buckets, Spec).
 
 validate_histogram_labels(Labels) ->
     [raise_error_if_le_label_found(Label) || Label <- Labels].

src/metrics/prometheus_quantile_summary.erl

@@ -457,14 +457,12 @@ validate_summary_spec(Spec) ->
     validate_summary_labels(Labels),
     {Invariant, QNs} = invariant_and_quantiles_from_spec(Spec),
     CompressLimit = compress_limit_from_spec(Spec),
-    [
-        {data, #{
-            quantiles => QNs,
-            invariant => Invariant,
-            compress_limit => CompressLimit
-        }}
-        | Spec
-    ].
+    Data = #{
+        quantiles => QNs,
+        invariant => Invariant,
+        compress_limit => CompressLimit
+    },
+    prometheus_metric_spec:add_value(data, Data, Spec).
 
 validate_summary_labels(Labels) ->
     [raise_error_if_quantile_label_found(Label) || Label <- Labels].

test/eunit/metric/prometheus_histogram_tests.erl

@@ -6,7 +6,8 @@
 
 prometheus_format_test_() ->
     {foreach, fun prometheus_eunit_common:start/0, fun prometheus_eunit_common:stop/1, [
-        fun test_registration/1,
+        fun test_registration_as_list/1,
+        fun test_registration_as_map/1,
         fun test_errors/1,
         fun test_buckets/1,
         fun test_observe/1,
@@ -23,7 +24,26 @@ prometheus_format_test_() ->
         fun test_collector3/1
     ]}.
 
-test_registration(_) ->
+test_registration_as_list(_) ->
+    Name = request_duration,
+    SpecWithRegistry = #{
+        name => Name,
+        buckets => [100, 300, 500, 750, 1000],
+        help => "",
+        registry => qwe
+    },
+    [
+        ?_assertEqual(
+            true,
+            prometheus_histogram:declare(SpecWithRegistry)
+        ),
+        ?_assertError(
+            {mf_already_exists, {qwe, Name}, "Consider using declare instead."},
+            prometheus_histogram:new(SpecWithRegistry)
+        )
+    ].
+
+test_registration_as_map(_) ->
     Name = request_duration,
     SpecWithRegistry = [
         {name, Name},

test/eunit/metric/prometheus_quantile_summary_tests.erl

@@ -8,7 +8,8 @@
 
 prometheus_format_test_() ->
     {foreach, fun prometheus_eunit_common:start/0, fun prometheus_eunit_common:stop/1, [
-        fun test_registration/1,
+        fun test_registration_as_list/1,
+        fun test_registration_as_map/1,
         fun test_errors/1,
         fun test_observe/1,
         fun test_observe_quantiles/1,
@@ -36,7 +37,7 @@ test_merge_logic_when_fetching_value(_) ->
     collect_monitors(Monitors),
     [?_assertMatch({_, _, _}, prometheus_quantile_summary:value(Name))].
 
-test_registration(_) ->
+test_registration_as_list(_) ->
     Name = orders_summary,
     SpecWithRegistry = [{name, Name}, {help, ""}, {registry, qwe}],
     [
@@ -50,6 +51,20 @@ test_registration(_) ->
         )
     ].
 
+test_registration_as_map(_) ->
+    Name = orders_summary,
+    SpecWithRegistry = #{name => Name, help => "", registry => qwe},
+    [
+        ?_assertEqual(
+            true,
+            prometheus_quantile_summary:declare(SpecWithRegistry)
+        ),
+        ?_assertError(
+            {mf_already_exists, {qwe, Name}, "Consider using declare instead."},
+            prometheus_quantile_summary:new(SpecWithRegistry)
+        )
+    ].
+
 test_errors(_) ->
     prometheus_quantile_summary:new([{name, db_query_duration}, {labels, [repo]}, {help, ""}]),
     %% basic name/labels/help validations test