Merge pull request #614 from albertored/fix-histo-buckets

Histogram buckets should be 1 more than boundaries

Author: Tristan Sloughter

CHANGELOG.md

@@ -25,6 +25,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - [Add `instrument_unit` to view criteria](https://github.com/open-telemetry/opentelemetry-erlang/pull/604)
 - [Validate instrument name](https://github.com/open-telemetry/opentelemetry-erlang/pull/604)
 
+### Changes
+
+- [Align histogram default boundaries with specification](https://github.com/open-telemetry/opentelemetry-erlang/pull/614)
+
+### Fixes
+
+ - [Correctly record histogram values greater than last boundary](https://github.com/open-telemetry/opentelemetry-erlang/pull/614)
+
 ## SDK 1.3.1 - 2023-08-15
 
 ### Added

apps/opentelemetry_experimental/src/otel_aggregation_histogram_explicit.erl

@@ -32,7 +32,7 @@
 
 -export_type([t/0]).
 
--define(DEFAULT_BOUNDARIES, [0.0, 5.0, 10.0, 25.0, 50.0, 75.0, 100.0, 250.0, 500.0, 1000.0]).
+-define(DEFAULT_BOUNDARIES, [0.0, 5.0, 10.0, 25.0, 50.0, 75.0, 100.0, 250.0, 500.0, 750.0, 1000.0, 2500.0, 5000.0, 7500.0, 10000.0]).
 
 -include_lib("stdlib/include/ms_transform.hrl").
 
@@ -280,8 +280,6 @@ find_bucket(Boundaries, Value) ->
 
 find_bucket([X | _Rest], Value, Pos) when Value =< X ->
     Pos;
-find_bucket([_X], _Value, Pos) ->
-    Pos;
 find_bucket([_X | Rest], Value, Pos) ->
     find_bucket(Rest, Value, Pos+1);
 find_bucket(_, _, Pos) ->
@@ -290,12 +288,12 @@ find_bucket(_, _, Pos) ->
 get_buckets(BucketCounts, Boundaries) ->
     lists:foldl(fun(Idx, Acc) ->
                         Acc ++ [counters_get(BucketCounts, Idx)]
-                end, [], lists:seq(1, length(Boundaries))).
+                end, [], lists:seq(1, length(Boundaries) + 1)).
 
 counters_get(undefined, _) ->
     0;
 counters_get(Counter, Idx) ->
     counters:get(Counter, Idx).
 
 new_bucket_counts(Boundaries) ->
-    counters:new(length(Boundaries), [write_concurrency]).
+    counters:new(length(Boundaries) + 1, [write_concurrency]).

apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl

@@ -311,7 +311,7 @@ float_histogram(_Config) ->
                                          min=Min,
                                          max=Max,
                                          sum=Sum}  <- Datapoints],
-            ?assertEqual([], [{#{<<"c">> => <<"b">>}, [0,1,1,2,0,0,0,0,0,0], 5, 10.3, 31.1}]
+            ?assertEqual([], [{#{<<"c">> => <<"b">>}, [0,1,1,2,0,0,0,0,0,0,0,0,0,0,0,0], 5, 10.3, 31.1}]
                          -- AttributeBuckets, AttributeBuckets)
     after
         5000 ->
@@ -514,11 +514,11 @@ explicit_histograms(_Config) ->
 
     otel_meter_server:add_view(#{instrument_name => a_histogram}, #{}),
 
-
     ?assertEqual(ok, otel_histogram:record(Histogram, 20, #{<<"c">> => <<"b">>})),
     ?assertEqual(ok, otel_histogram:record(Histogram, 30, #{<<"a">> => <<"b">>, <<"d">> => <<"e">>})),
     ?assertEqual(ok, otel_histogram:record(Histogram, 44, #{<<"c">> => <<"b">>})),
     ?assertEqual(ok, otel_histogram:record(Histogram, 100, #{<<"c">> => <<"b">>})),
+    ?assertEqual(ok, otel_histogram:record(Histogram, 20000, #{<<"c">> => <<"b">>})),
 
     otel_meter_server:force_flush(),
 
@@ -531,8 +531,8 @@ explicit_histograms(_Config) ->
                                                                                          min=Min,
                                                                                          max=Max,
                                                                                          sum=Sum}  <- Datapoints]),
-            ?assertEqual([], [{#{<<"c">> => <<"b">>}, [0,0,0,1,1,0,1,0,0,0], 20, 100, 164},
-                              {#{<<"a">> => <<"b">>, <<"d">> => <<"e">>}, [0,0,0,0,1,0,0,0,0,0], 30, 30, 30}]
+            ?assertEqual([], [{#{<<"c">> => <<"b">>}, [0,0,0,1,1,0,1,0,0,0,0,0,0,0,0,1], 20, 20000, 20164},
+                              {#{<<"a">> => <<"b">>, <<"d">> => <<"e">>}, [0,0,0,0,1,0,0,0,0,0,0,0,0,0,0,0], 30, 30, 30}]
                          -- AttributeBuckets, AttributeBuckets)
     after
         5000 ->
@@ -580,8 +580,8 @@ delta_explicit_histograms(_Config) ->
                                                                                          min=Min,
                                                                                          max=Max,
                                                                                          sum=Sum}  <- Datapoints]),
-            ?assertEqual([], [{#{<<"c">> => <<"b">>}, [0,0,0,1,1,0,1,0,0,0], 20, 100, 164},
-                              {#{<<"a">> => <<"b">>, <<"d">> => <<"e">>}, [0,0,0,0,1,0,0,0,0,0], 30, 30, 30}]
+            ?assertEqual([], [{#{<<"c">> => <<"b">>}, [0,0,0,1,1,0,1,0,0,0,0,0,0,0,0,0], 20, 100, 164},
+                              {#{<<"a">> => <<"b">>, <<"d">> => <<"e">>}, [0,0,0,0,1,0,0,0,0,0,0,0,0,0,0,0], 30, 30, 30}]
                          -- AttributeBuckets, AttributeBuckets)
     after
         5000 ->
@@ -601,9 +601,9 @@ delta_explicit_histograms(_Config) ->
                                                                               min=Min,
                                                                               max=Max,
                                                                               sum=Sum}  <- Datapoints1],
-            ?assertEqual([], [{#{<<"c">> => <<"b">>}, [0,0,0,0,0,0,1,0,0,0], 88, 88, 88},
+            ?assertEqual([], [{#{<<"c">> => <<"b">>}, [0,0,0,0,0,0,1,0,0,0,0,0,0,0,0,0], 88, 88, 88},
                               {#{<<"a">> => <<"b">>,<<"d">> => <<"e">>},
-                               [0,0,0,0,0,0,0,0,0,0],
+                               [0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0],
                                infinity,-9.223372036854776e18,0}
                              ]
                          -- AttributeBuckets1, AttributeBuckets1)