Merge pull request #613 from albertored/temporality

Tristan Sloughter · web-flow · commit 6d081861aef0 · 2023-08-28T11:01:46.000-06:00
Fix temporality handling
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixes
 
  - [Correctly record histogram values greater than last boundary](https://github.com/open-telemetry/opentelemetry-erlang/pull/614)
+ - [Readers should use a default cumulative temporality if not specified](https://github.com/open-telemetry/opentelemetry-erlang/pull/613)
 
 ## SDK 1.3.1 - 2023-08-15
 
diff --git a/apps/opentelemetry_api_experimental/include/otel_metrics.hrl b/apps/opentelemetry_api_experimental/include/otel_metrics.hrl
@@ -10,7 +10,6 @@
 
 -define(TEMPORALITY_DELTA, temporality_delta).
 -define(TEMPORALITY_CUMULATIVE, temporality_cumulative).
--define(TEMPORALITY_UNSPECIFIED, temporality_unspecified).
 
 -define(KIND_COUNTER, counter).
 -define(KIND_OBSERVABLE_COUNTER, observable_counter).
diff --git a/apps/opentelemetry_api_experimental/src/otel_instrument.erl b/apps/opentelemetry_api_experimental/src/otel_instrument.erl
@@ -19,7 +19,8 @@
 
 -export([new/6,
          new/8,
-         is_monotonic/1]).
+         is_monotonic/1,
+         temporality/1]).
 
 -include("otel_metrics.hrl").
 
@@ -35,9 +36,7 @@
                            [named_observations()].
 -type callback() :: fun((callback_args()) -> callback_result()).
 
--type temporality() :: ?TEMPORALITY_UNSPECIFIED |
-                       ?TEMPORALITY_DELTA |
-                       ?TEMPORALITY_CUMULATIVE.
+-type temporality() :: ?TEMPORALITY_DELTA | ?TEMPORALITY_CUMULATIVE.
 
 -type t() :: #instrument{}.
 
@@ -81,3 +80,16 @@ is_monotonic(#instrument{kind=?KIND_HISTOGRAM}) ->
     true;
 is_monotonic(_) ->
     false.
+
+temporality(#instrument{kind=?KIND_COUNTER}) ->
+    ?TEMPORALITY_DELTA;
+temporality(#instrument{kind=?KIND_OBSERVABLE_COUNTER}) ->
+    ?TEMPORALITY_CUMULATIVE;
+temporality(#instrument{kind=?KIND_UPDOWN_COUNTER}) ->
+    ?TEMPORALITY_DELTA;
+temporality(#instrument{kind=?KIND_OBSERVABLE_UPDOWNCOUNTER}) ->
+    ?TEMPORALITY_CUMULATIVE;
+temporality(#instrument{kind=?KIND_HISTOGRAM}) ->
+    ?TEMPORALITY_DELTA;
+temporality(#instrument{kind=?KIND_OBSERVABLE_GAUGE}) ->
+    ?TEMPORALITY_CUMULATIVE.
diff --git a/apps/opentelemetry_experimental/src/otel_aggregation.erl b/apps/opentelemetry_experimental/src/otel_aggregation.erl
@@ -1,9 +1,7 @@
 -module(otel_aggregation).
 
 -export([maybe_init_aggregate/4,
-         default_mapping/0,
-         temporality_mapping/0,
-         instrument_temporality/1]).
+         default_mapping/0]).
 
 -include_lib("opentelemetry_api_experimental/include/otel_metrics.hrl").
 -include("otel_metrics.hrl").
@@ -71,25 +69,4 @@ default_mapping() ->
       ?KIND_HISTOGRAM => otel_aggregation_histogram_explicit,
       ?KIND_OBSERVABLE_GAUGE => otel_aggregation_last_value,
       ?KIND_UPDOWN_COUNTER => otel_aggregation_sum,
-      ?KIND_OBSERVABLE_UPDOWNCOUNTER => otel_aggregation_sum}.
-
-temporality_mapping() ->
-    #{?KIND_COUNTER =>?TEMPORALITY_DELTA,
-      ?KIND_OBSERVABLE_COUNTER => ?TEMPORALITY_CUMULATIVE,
-      ?KIND_UPDOWN_COUNTER => ?TEMPORALITY_DELTA,
-      ?KIND_OBSERVABLE_UPDOWNCOUNTER => ?TEMPORALITY_CUMULATIVE,
-      ?KIND_HISTOGRAM => ?TEMPORALITY_UNSPECIFIED,
-      ?KIND_OBSERVABLE_GAUGE => ?TEMPORALITY_UNSPECIFIED}.
-
-instrument_temporality(#instrument{kind=?KIND_COUNTER}) ->
-    ?TEMPORALITY_DELTA;
-instrument_temporality(#instrument{kind=?KIND_OBSERVABLE_COUNTER}) ->
-    ?TEMPORALITY_CUMULATIVE;
-instrument_temporality(#instrument{kind=?KIND_UPDOWN_COUNTER}) ->
-    ?TEMPORALITY_DELTA;
-instrument_temporality(#instrument{kind=?KIND_OBSERVABLE_UPDOWNCOUNTER}) ->
-    ?TEMPORALITY_CUMULATIVE;
-instrument_temporality(#instrument{kind=?KIND_HISTOGRAM}) ->
-    ?TEMPORALITY_UNSPECIFIED;
-instrument_temporality(#instrument{kind=?KIND_OBSERVABLE_GAUGE}) ->
-    ?TEMPORALITY_UNSPECIFIED.
+      ?KIND_OBSERVABLE_UPDOWNCOUNTER => otel_aggregation_sum}.
diff --git a/apps/opentelemetry_experimental/src/otel_meter_server.erl b/apps/opentelemetry_experimental/src/otel_meter_server.erl
@@ -342,14 +342,12 @@ metric_reader(ReaderId, ReaderPid, DefaultAggregationMapping, Temporality) ->
 
     ReaderAggregationMapping = maps:merge(otel_aggregation:default_mapping(),
                                           DefaultAggregationMapping),
-    ReaderTemporalityMapping = maps:merge(otel_aggregation:temporality_mapping(),
-                                          Temporality),
 
     #reader{id=ReaderId,
             pid=ReaderPid,
             monitor_ref=Ref,
             default_aggregation_mapping=ReaderAggregationMapping,
-            default_temporality_mapping=ReaderTemporalityMapping}.
+            default_temporality_mapping=Temporality}.
 
 
 %% a Measurement's Instrument is matched against Views
@@ -388,7 +386,7 @@ view_aggregation_for_reader(Instrument=#instrument{kind=Kind}, ViewAggregation,
                             Reader=#reader{id=Id,
                                            default_temporality_mapping=ReaderTemporalityMapping}) ->
     AggregationModule = aggregation_module(Instrument, View, Reader),
-    Temporality = maps:get(Kind, ReaderTemporalityMapping, ?TEMPORALITY_UNSPECIFIED),
+    Temporality = maps:get(Kind, ReaderTemporalityMapping, ?TEMPORALITY_CUMULATIVE),
 
     ViewAggregation#view_aggregation{
       reader=Id,
@@ -400,7 +398,7 @@ view_aggregation_for_reader(Instrument=#instrument{kind=Kind}, ViewAggregation,
                             Reader=#reader{id=Id,
                                            default_temporality_mapping=ReaderTemporalityMapping}) ->
     AggregationModule = aggregation_module(Instrument, View, Reader),
-    Temporality = maps:get(Kind, ReaderTemporalityMapping, ?TEMPORALITY_UNSPECIFIED),
+    Temporality = maps:get(Kind, ReaderTemporalityMapping, ?TEMPORALITY_CUMULATIVE),
 
     ViewAggregation#view_aggregation{
       reader=Id,
diff --git a/apps/opentelemetry_experimental/src/otel_otlp_metrics.erl b/apps/opentelemetry_experimental/src/otel_otlp_metrics.erl
@@ -21,6 +21,7 @@
 
 -include_lib("kernel/include/logger.hrl").
 -include_lib("opentelemetry_api/include/opentelemetry.hrl").
+-include_lib("opentelemetry_api_experimental/include/otel_metrics.hrl").
 -include_lib("opentelemetry/include/otel_span.hrl").
 -include("otel_metrics.hrl").
 
@@ -125,9 +126,7 @@ to_datapoint_value(Value) when is_integer(Value) ->
 to_datapoint_value(Value) when is_float(Value) ->
     {as_double, Value}.
 
-to_otlp_temporality(temporality_delta) ->
+to_otlp_temporality(?TEMPORALITY_DELTA) ->
     'AGGREGATION_TEMPORALITY_DELTA';
-to_otlp_temporality(temporality_cumulative) ->
-    'AGGREGATION_TEMPORALITY_CUMULATIVE';
-to_otlp_temporality(_) ->
-    'AGGREGATION_TEMPORALITY_UNSPECIFIED'.
+to_otlp_temporality(?TEMPORALITY_CUMULATIVE) ->
+    'AGGREGATION_TEMPORALITY_CUMULATIVE'.
diff --git a/apps/opentelemetry_experimental/src/otel_view.erl b/apps/opentelemetry_experimental/src/otel_view.erl
@@ -80,7 +80,7 @@ match_instrument_to_views(Instrument=#instrument{name=InstrumentName,
                                                  meter=Meter,
                                                  description=Description}, Views) ->
     IsMonotonic = otel_instrument:is_monotonic(Instrument),
-    Temporality = otel_aggregation:instrument_temporality(Instrument),
+    Temporality = otel_instrument:temporality(Instrument),
     Scope = otel_meter:scope(Meter),
     case lists:filtermap(fun(View=#view{name=ViewName,
                                         description=ViewDescription,
diff --git a/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl b/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl
@@ -97,7 +97,8 @@ init_per_testcase(provider_test, Config) ->
     application:load(opentelemetry_experimental),
     ok = application:set_env(opentelemetry_experimental, readers,
                              [#{module => otel_metric_reader,
-                                config => #{exporter => {otel_metric_exporter_pid, self()}}}]),
+                                config => #{exporter => {otel_metric_exporter_pid, self()},
+                                            default_temporality_mapping => default_temporality_mapping()}}]),
     ok = application:set_env(opentelemetry_experimental, views,
                              [#{selector => #{instrument_name => a_counter},
                                 aggregation_module => otel_aggregation_sum},
@@ -117,9 +118,11 @@ init_per_testcase(multiple_readers, Config) ->
     application:load(opentelemetry_experimental),
     ok = application:set_env(opentelemetry_experimental, readers,
                              [#{module => otel_metric_reader,
-                                config => #{exporter => {otel_metric_exporter_pid, self()}}},
+                                config => #{exporter => {otel_metric_exporter_pid, self()},
+                                            default_temporality_mapping => default_temporality_mapping()}},
                               #{module => otel_metric_reader,
                                 config => #{exporter => {otel_metric_exporter_pid, self()},
+                                            default_temporality_mapping => default_temporality_mapping(),
                                             default_aggregation_mapping =>
                                                 #{?KIND_COUNTER => otel_aggregation_drop}}}]),
 
@@ -132,13 +135,14 @@ 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()}}}]),
+                                                                     config => #{exporter => {otel_metric_exporter_pid, self()},
+                                                                                 default_temporality_mapping => default_temporality_mapping()}}]),
 
     {ok, _} = application:ensure_all_started(opentelemetry_experimental),
 
     Config;
 init_per_testcase(cumulative_counter, Config) ->
-    CumulativeCounterTemporality = #{?KIND_COUNTER =>?TEMPORALITY_CUMULATIVE},
+    CumulativeCounterTemporality = maps:put(?KIND_COUNTER, ?TEMPORALITY_CUMULATIVE, default_temporality_mapping()),
     application:load(opentelemetry_experimental),
     ok = application:set_env(opentelemetry_experimental, readers, [#{module => otel_metric_reader,
                                                                      config => #{exporter => {otel_metric_exporter_pid, self()},
@@ -149,7 +153,7 @@ init_per_testcase(cumulative_counter, Config) ->
 
     Config;
 init_per_testcase(delta_explicit_histograms, Config) ->
-    DeltaHistogramTemporality = #{?KIND_HISTOGRAM =>?TEMPORALITY_DELTA},
+    DeltaHistogramTemporality = maps:put(?KIND_HISTOGRAM, ?TEMPORALITY_DELTA, default_temporality_mapping()),
     application:load(opentelemetry_experimental),
     ok = application:set_env(opentelemetry_experimental, readers, [#{module => otel_metric_reader,
                                                                      config => #{exporter => {otel_metric_exporter_pid, self()},
@@ -160,7 +164,7 @@ init_per_testcase(delta_explicit_histograms, Config) ->
 
     Config;
 init_per_testcase(delta_observable_counter, Config) ->
-    DeltaObservableCounterTemporality = #{?KIND_OBSERVABLE_COUNTER =>?TEMPORALITY_DELTA},
+    DeltaObservableCounterTemporality = maps:put(?KIND_OBSERVABLE_COUNTER, ?TEMPORALITY_DELTA, default_temporality_mapping()),
     application:load(opentelemetry_experimental),
     ok = application:set_env(opentelemetry_experimental, readers, [#{module => otel_metric_reader,
                                                                      config => #{exporter => {otel_metric_exporter_pid, self()},
@@ -173,7 +177,8 @@ init_per_testcase(delta_observable_counter, Config) ->
 init_per_testcase(_, Config) ->
     application:load(opentelemetry_experimental),
     ok = application:set_env(opentelemetry_experimental, readers, [#{module => otel_metric_reader,
-                                                                     config => #{exporter => {otel_metric_exporter_pid, self()}}}]),
+                                                                     config => #{exporter => {otel_metric_exporter_pid, self()},
+                                                                                 default_temporality_mapping => default_temporality_mapping()}}]),
 
     {ok, _} = application:ensure_all_started(opentelemetry_experimental),
 
@@ -193,6 +198,16 @@ default_resource(_Config) ->
 
     ok.
 
+default_temporality_mapping() ->
+    #{
+        ?KIND_COUNTER => ?TEMPORALITY_DELTA,
+        ?KIND_OBSERVABLE_COUNTER => ?TEMPORALITY_CUMULATIVE,
+        ?KIND_UPDOWN_COUNTER => ?TEMPORALITY_DELTA,
+        ?KIND_OBSERVABLE_UPDOWNCOUNTER => ?TEMPORALITY_CUMULATIVE,
+        ?KIND_HISTOGRAM => ?TEMPORALITY_DELTA,
+        ?KIND_OBSERVABLE_GAUGE => ?TEMPORALITY_CUMULATIVE
+    }.
+
 using_macros(_Config) ->
     DefaultMeter = otel_meter_default,
 
diff --git a/apps/opentelemetry_exporter/test/opentelemetry_exporter_SUITE.erl b/apps/opentelemetry_exporter/test/opentelemetry_exporter_SUITE.erl
@@ -76,7 +76,7 @@ verify_metrics_export(Config) ->
                        name = <<"sum name">>,
                        description = <<"some sum description">>,
                        unit = kb,
-                       data = #sum{aggregation_temporality = 'AGGREGATION_TEMPORALITY_CUMULATIVE',
+                       data = #sum{aggregation_temporality = temporality_cumulative,
                                    is_monotonic=true,
                                    datapoints=[#datapoint{
                                                   attributes=otel_attributes:new(#{<<"key-1">> => <<"value-1">>},
@@ -126,7 +126,7 @@ verify_metrics_export(Config) ->
                        name = <<"histogram name">>,
                        description = <<"some histogram description">>,
                        unit = kb,
-                       data = #histogram{aggregation_temporality = 'AGGREGATION_TEMPORALITY_CUMULATIVE',
+                       data = #histogram{aggregation_temporality = temporality_cumulative,
                                          datapoints=[#histogram_datapoint{
                                                         attributes=otel_attributes:new(#{<<"key-1">> => <<"value-1">>},
                                                                                        128, 128),
diff --git a/test/otel_metric_tests.exs b/test/otel_metric_tests.exs
@@ -28,7 +28,15 @@ defmodule OtelMetricTests do
       %{
         module: :otel_metric_reader,
         config: %{
-          exporter: {:otel_metric_exporter_pid, {:metric, self()}}
+          exporter: {:otel_metric_exporter_pid, {:metric, self()}},
+          default_temporality_mapping: %{
+            counter: :temporality_delta,
+            observable_counter: :temporality_cumulative,
+            updown_counter: :temporality_delta,
+            observable_updowncounter: :temporality_cumulative,
+            histogram: :temporality_delta,
+            observable_gauge: :temporality_cumulative
+          }
         }
       }
     ])
