Merge pull request #713 from tsloughter/fix-default-temporality

use correct default temporality for streams based on the instrument kind
open-telemetry · Mar 18, 2024 · e8c2b2a · e8c2b2a
2 parents d1f3e35 + aaece44
commit e8c2b2a
Show file tree

Hide file tree

Showing 10 changed files with 55 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## Experimental API 0.5.1 - 2024-03-18
+
+### Added
+
+- [instrument kind temporality function for use by the
+  SDK](https://github.com/open-telemetry/opentelemetry-erlang/pull/713)
+
+## Experimental SDK 0.5.1 - 2024-03-18
+
+### Fixes
+
+- [use correct default temporality for streams based on the instrument
+  kind](https://github.com/open-telemetry/opentelemetry-erlang/pull/713)
+
 ## API 1.3.0 - 2024-03-15
 
 ### Changes

diff --git a/apps/opentelemetry_api_experimental/src/opentelemetry_api_experimental.app.src b/apps/opentelemetry_api_experimental/src/opentelemetry_api_experimental.app.src
@@ -1,6 +1,6 @@
 {application, opentelemetry_api_experimental,
  [{description, "API for unstable OpenTelemetry signals"},
-  {vsn, "0.5.0"},
+  {vsn, "0.5.1"},
   {registered, []},
   {applications,
    [kernel,

diff --git a/apps/opentelemetry_api_experimental/src/otel_instrument.erl b/apps/opentelemetry_api_experimental/src/otel_instrument.erl
@@ -20,7 +20,8 @@
 -export([new/5,
          new/7,
          is_monotonic/1,
-         temporality/1]).
+         temporality/1,
+         kind_temporality/1]).
 
 -include("otel_metrics.hrl").
 
@@ -99,15 +100,18 @@ is_monotonic(#instrument{kind=?KIND_HISTOGRAM}) ->
 is_monotonic(_) ->
     false.
 
-temporality(#instrument{kind=?KIND_COUNTER}) ->
+temporality(#instrument{kind=Kind}) ->
+    kind_temporality(Kind).
+
+kind_temporality(?KIND_COUNTER) ->
     ?TEMPORALITY_DELTA;
-temporality(#instrument{kind=?KIND_OBSERVABLE_COUNTER}) ->
+kind_temporality(?KIND_OBSERVABLE_COUNTER) ->
     ?TEMPORALITY_CUMULATIVE;
-temporality(#instrument{kind=?KIND_UPDOWN_COUNTER}) ->
+kind_temporality(?KIND_UPDOWN_COUNTER) ->
     ?TEMPORALITY_DELTA;
-temporality(#instrument{kind=?KIND_OBSERVABLE_UPDOWNCOUNTER}) ->
+kind_temporality(?KIND_OBSERVABLE_UPDOWNCOUNTER) ->
     ?TEMPORALITY_CUMULATIVE;
-temporality(#instrument{kind=?KIND_HISTOGRAM}) ->
+kind_temporality(?KIND_HISTOGRAM) ->
     ?TEMPORALITY_DELTA;
-temporality(#instrument{kind=?KIND_OBSERVABLE_GAUGE}) ->
+kind_temporality(?KIND_OBSERVABLE_GAUGE) ->
     ?TEMPORALITY_CUMULATIVE.
diff --git a/apps/opentelemetry_experimental/rebar.config b/apps/opentelemetry_experimental/rebar.config
@@ -1,7 +1,7 @@
 {erl_opts, [debug_info]}.
 {deps, [{opentelemetry, "~> 1.4"},
         {opentelemetry_api, "~> 1.3"},
-        {opentelemetry_api_experimental, "~> 0.5"}]}.
+        {opentelemetry_api_experimental, "~> 0.5.1"}]}.
 
 {shell, [
   % {config, "config/sys.config"},

diff --git a/apps/opentelemetry_experimental/src/opentelemetry_experimental.app.src b/apps/opentelemetry_experimental/src/opentelemetry_experimental.app.src
@@ -1,6 +1,6 @@
 {application, opentelemetry_experimental,
  [{description, "Implementation of unstable OpenTelemetry signals"},
-  {vsn, "0.5.0"},
+  {vsn, "0.5.1"},
   {registered, []},
   {mod, {opentelemetry_experimental_app, []}},
   {applications,

diff --git a/apps/opentelemetry_experimental/src/otel_aggregation.erl b/apps/opentelemetry_experimental/src/otel_aggregation.erl
@@ -1,7 +1,8 @@
 -module(otel_aggregation).
 
 -export([maybe_init_aggregate/6,
-         default_mapping/0]).
+         default_mapping/0,
+         default_temporality_mapping/0]).
 
 -include_lib("opentelemetry_api_experimental/include/otel_metrics.hrl").
 -include("otel_view.hrl").
@@ -69,6 +70,19 @@ default_mapping() ->
       ?KIND_UPDOWN_COUNTER => otel_aggregation_sum,
       ?KIND_OBSERVABLE_UPDOWNCOUNTER => otel_aggregation_sum}.
 
+%% by default the aggregators use the same temporality as is native to the instrument
+-spec default_temporality_mapping() -> #{otel_instrument:kind() => otel_instrument:temporality()}.
+default_temporality_mapping() ->
+    lists:foldl(fun(Kind, Acc) ->
+                        Acc#{Kind => otel_instrument:kind_temporality(Kind)}
+                end, #{}, [?KIND_COUNTER,
+                           ?KIND_OBSERVABLE_COUNTER,
+                           ?KIND_HISTOGRAM,
+                           ?KIND_OBSERVABLE_GAUGE,
+                           ?KIND_UPDOWN_COUNTER,
+                           ?KIND_OBSERVABLE_UPDOWNCOUNTER
+                          ]).
+
 split(Keys, Map) ->
     lists:foldl(fun(Key, {KeptAcc, DroppedAcc}) ->
                         case maps:take(Key, DroppedAcc) of

diff --git a/apps/opentelemetry_experimental/src/otel_meter_default.erl b/apps/opentelemetry_experimental/src/otel_meter_default.erl
@@ -97,6 +97,9 @@ validate_advisory_param(Name, _Kind, Opt, _Value) ->
     ?LOG_WARNING("[instrument '~s'] '~s' advisory parameter is not supported, ignoring", [Name, Opt]),
     false.
 
+%% empty list denotes a single bucket histogram that can be used for things like summaries
+validate_explicit_bucket_boundaries(_Name, []) ->
+    {true, {explicit_bucket_boundaries, []}};
 validate_explicit_bucket_boundaries(Name, [_ | _] = Value) ->
     case lists:all(fun is_number/1, Value) and (lists:sort(Value) == Value) of
         true ->

diff --git a/apps/opentelemetry_experimental/src/otel_metric_reader.erl b/apps/opentelemetry_experimental/src/otel_metric_reader.erl
@@ -87,7 +87,7 @@ init([ReaderId, ProviderSup, Config]) ->
     Exporter = otel_exporter:init(ExporterModuleConfig),
 
     DefaultAggregationMapping = maps:get(default_aggregation_mapping, Config, otel_aggregation:default_mapping()),
-    Temporality = maps:get(default_temporality_mapping, Config, #{}),
+    Temporality = maps:get(default_temporality_mapping, Config, otel_aggregation:default_temporality_mapping()),
 
     %% if a periodic reader is needed then this value is set
     %% somehow need to do a default of 10000 MILLIS, but only if this is a periodic reader

diff --git a/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl b/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl
@@ -169,8 +169,7 @@ init_per_testcase(delta_counter, Config) ->
     %% delta is the default for a counter with sum aggregation
     %% so no need to set any temporality mapping in the reader
     ok = application:set_env(opentelemetry_experimental, readers, [#{module => otel_metric_reader,
-                                                                     config => #{exporter => {otel_metric_exporter_pid, self()},
-                                                                                 default_temporality_mapping => default_temporality_mapping()}}]),
+                                                                     config => #{exporter => {otel_metric_exporter_pid, self()}}}]),
 
     {ok, _} = application:ensure_all_started(opentelemetry_experimental),
 
@@ -1383,6 +1382,11 @@ advisory_params(_Config) ->
                                   #{advisory_params => #{explicit_bucket_boundaries => [10, 20, 30]}}),
     ?assertEqual(Histogram#instrument.advisory_params, #{explicit_bucket_boundaries => [10, 20, 30]}),
 
+    %% an empty boundaries list can be used to get a single bucket histogram `(-Inf, +Inf)'
+    BHistogram = otel_histogram:create(Meter, b_histogram,
+                                       #{advisory_params => #{explicit_bucket_boundaries => []}}),
+    ?assertEqual(BHistogram#instrument.advisory_params, #{explicit_bucket_boundaries => []}),
+
     Ctx = otel_ctx:new(),
 
     ?assertEqual(ok, otel_histogram:record(Ctx, Histogram, 15, #{<<"a">> => <<"1">>})),

diff --git a/docs.sh b/docs.sh
@@ -11,8 +11,8 @@ rebar3 compile
 rebar3 edoc
 sdk_version=1.4.0
 api_version=1.3.0
-exp_sdk_version=0.5.0
-exp_api_version=0.5.0
+exp_sdk_version=0.5.1
+exp_api_version=0.5.1
 otlp_version=1.7.0
 zipkin_version=1.1.0
 semconv_version=0.2.0