From 7f8f743f53931e83ebe3ce217fa5a1772bbf7efa Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Fri, 27 Jan 2023 15:32:47 -0700 Subject: [PATCH 1/6] remove value check and logs from instrument API --- .../src/otel_counter.erl | 15 ++++----------- .../test/otel_metrics_SUITE.erl | 4 ++-- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/apps/opentelemetry_api_experimental/src/otel_counter.erl b/apps/opentelemetry_api_experimental/src/otel_counter.erl index 7c5906d2..730eb639 100644 --- a/apps/opentelemetry_api_experimental/src/otel_counter.erl +++ b/apps/opentelemetry_api_experimental/src/otel_counter.erl @@ -33,16 +33,9 @@ create(Meter, Name, Opts) -> otel_meter:create_counter(Meter, Name, Opts). -spec add(otel_meter:t(), otel_instrument:name(), number(), opentelemetry:attributes_map()) -> ok. -add(Meter, Name, Number, Attributes) when Number >= 0 -> - otel_meter:record(Meter, Name, Number, Attributes); -add(_, Name, _, _) -> - ?LOG_INFO("Counter instrument ~p does not support negative values.", [Name]), - ok. +add(Meter, Name, Number, Attributes) -> + otel_meter:record(Meter, Name, Number, Attributes). -spec add(otel_instrument:t(), number(), opentelemetry:attributes_map()) -> ok. -add(Instrument=#instrument{module=Module}, Number, Attributes) - when Number >= 0 -> - Module:record(Instrument, Number, Attributes); -add(#instrument{name=Name}, _, _) -> - ?LOG_INFO("Counter instrument ~p does not support negative values.", [Name]), - ok. +add(Instrument=#instrument{module=Module}, Number, Attributes) -> + Module:record(Instrument, Number, Attributes). diff --git a/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl b/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl index f04554d7..de7aaf60 100644 --- a/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl +++ b/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl @@ -348,8 +348,8 @@ provider_test(_Config) -> otel_meter_server:force_flush(), - ?assertSumReceive(a_counter, <<"counter description">>, kb, [{16.0, #{<<"c">> => <<"b">>}}]), - ?assertSumReceive(view_c, <<"counter description">>, kb, [{16.0, #{<<"c">> => <<"b">>}}]), + ?assertSumReceive(a_counter, <<"counter description">>, kb, [{6.0, #{<<"c">> => <<"b">>}}]), + ?assertSumReceive(view_c, <<"counter description">>, kb, [{6.0, #{<<"c">> => <<"b">>}}]), %% sum agg is default delta temporality so counter will reset ?assertEqual(ok, otel_counter:add(Counter, 7, #{<<"c">> => <<"b">>})), From b75b68f4292d550b80dc8e98eabdaa78197853c4 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Mon, 30 Jan 2023 15:25:01 -0700 Subject: [PATCH 2/6] fix is_monotonic to true for histogram kind instrument --- apps/opentelemetry_api_experimental/src/otel_instrument.erl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/opentelemetry_api_experimental/src/otel_instrument.erl b/apps/opentelemetry_api_experimental/src/otel_instrument.erl index 9eaedfc1..7eef2a06 100644 --- a/apps/opentelemetry_api_experimental/src/otel_instrument.erl +++ b/apps/opentelemetry_api_experimental/src/otel_instrument.erl @@ -68,5 +68,7 @@ is_monotonic(#instrument{kind=?KIND_COUNTER}) -> true; is_monotonic(#instrument{kind=?KIND_OBSERVABLE_COUNTER}) -> true; +is_monotonic(#instrument{kind=?KIND_HISTOGRAM}) -> + true; is_monotonic(_) -> false. From dfd03d08952bdd98f6305c39c015605762fec47a Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Sun, 5 Feb 2023 10:56:21 -0700 Subject: [PATCH 3/6] handle is_monotonic for views in Sum aggregation This required changing the aggregate API to be passed the view_aggregation record so it has access to is_monotonic. The API feels a little cleaner anyway. --- .../src/otel_aggregation.erl | 18 ++++++++------- .../otel_aggregation_histogram_explicit.erl | 11 +++++++-- .../src/otel_aggregation_last_value.erl | 12 +++++++--- .../src/otel_aggregation_sum.erl | 23 +++++++++++++++---- .../src/otel_meter_server.erl | 7 ++---- .../src/otel_metric_reader.erl | 13 ++++------- .../test/otel_metrics_SUITE.erl | 6 ++--- 7 files changed, 56 insertions(+), 34 deletions(-) diff --git a/apps/opentelemetry_experimental/src/otel_aggregation.erl b/apps/opentelemetry_experimental/src/otel_aggregation.erl index 04c0b709..857eb694 100644 --- a/apps/opentelemetry_experimental/src/otel_aggregation.erl +++ b/apps/opentelemetry_experimental/src/otel_aggregation.erl @@ -1,12 +1,13 @@ -module(otel_aggregation). --export([maybe_init_aggregate/5, +-export([maybe_init_aggregate/4, default_mapping/0, temporality_mapping/0, instrument_temporality/1]). -include_lib("opentelemetry_api_experimental/include/otel_metrics.hrl"). -include("otel_metrics.hrl"). +-include("otel_view.hrl"). -type temporality() :: ?AGGREGATION_TEMPORALITY_UNSPECIFIED | ?AGGREGATION_TEMPORALITY_DELTA | @@ -30,11 +31,11 @@ Options :: options(), Aggregation :: t(). --callback aggregate(Table, Key, Value, Options) -> boolean() when +-callback aggregate(Table, ViewAggregation, Value, Attributes) -> boolean() when Table :: ets:table(), - Key :: key(), + ViewAggregation :: #view_aggregation{}, Value :: number(), - Options :: options(). + Attributes :: opentelemetry:attributes_map(). -callback checkpoint(Table, Name, ReaderId, Temporality, CollectionStartTime) -> ok when Table :: ets:table(), @@ -50,16 +51,17 @@ Temporality :: temporality(), CollectionStartTime :: integer(). -maybe_init_aggregate(MetricsTab, AggregationModule, Key, Value, Options) -> - case AggregationModule:aggregate(MetricsTab, Key, Value, Options) of +maybe_init_aggregate(MetricsTab, ViewAggregation=#view_aggregation{aggregation_module=AggregationModule}, + Value, Attributes) -> + case AggregationModule:aggregate(MetricsTab, ViewAggregation, Value, Attributes) of true -> ok; false -> %% entry doesn't exist, create it and rerun the aggregate function - Metric = AggregationModule:init(Key, Options), + Metric = AggregationModule:init(ViewAggregation, Attributes), %% don't overwrite a possible concurrent measurement doing the same _ = ets:insert_new(MetricsTab, Metric), - AggregationModule:aggregate(MetricsTab, Key, Value, Options) + AggregationModule:aggregate(MetricsTab, ViewAggregation, Value, Attributes) end. -spec default_mapping() -> #{otel_instrument:kind() => module()}. diff --git a/apps/opentelemetry_experimental/src/otel_aggregation_histogram_explicit.erl b/apps/opentelemetry_experimental/src/otel_aggregation_histogram_explicit.erl index af27a1e2..3b5200c4 100644 --- a/apps/opentelemetry_experimental/src/otel_aggregation_histogram_explicit.erl +++ b/apps/opentelemetry_experimental/src/otel_aggregation_histogram_explicit.erl @@ -26,6 +26,7 @@ -include("otel_metrics.hrl"). -include_lib("opentelemetry_api_experimental/include/otel_metrics.hrl"). +-include("otel_view.hrl"). -type t() :: #explicit_histogram_aggregation{}. @@ -37,7 +38,10 @@ -define(MIN_DOUBLE, -9223372036854775807.0). %% the proto representation of size `fixed64' -init(Key, Options) -> +init(#view_aggregation{name=Name, + reader=ReaderId, + aggregation_options=Options}, Attributes) -> + Key = {Name, Attributes, ReaderId}, Boundaries = maps:get(boundaries, Options, ?DEFAULT_BOUNDARIES), RecordMinMax = maps:get(record_min_max, Options, true), #explicit_histogram_aggregation{key=Key, @@ -51,7 +55,10 @@ init(Key, Options) -> sum=0 }. -aggregate(Table, Key, Value, Options) -> +aggregate(Table, #view_aggregation{name=Name, + reader=ReaderId, + aggregation_options=Options}, Value, Attributes) -> + Key = {Name, Attributes, ReaderId}, Boundaries = maps:get(boundaries, Options, ?DEFAULT_BOUNDARIES), try ets:lookup_element(Table, Key, #explicit_histogram_aggregation.bucket_counts) of BucketCounts0 -> diff --git a/apps/opentelemetry_experimental/src/otel_aggregation_last_value.erl b/apps/opentelemetry_experimental/src/otel_aggregation_last_value.erl index caa62631..b77f0f1d 100644 --- a/apps/opentelemetry_experimental/src/otel_aggregation_last_value.erl +++ b/apps/opentelemetry_experimental/src/otel_aggregation_last_value.erl @@ -31,17 +31,23 @@ -export_type([t/0]). -include_lib("opentelemetry_api/include/gradualizer.hrl"). +-include("otel_view.hrl"). -init(Key, _Options) -> +init(#view_aggregation{name=Name, + reader=ReaderId, + aggregation_options=_Options}, Attributes) -> + Key = {Name, Attributes, ReaderId}, #last_value_aggregation{key=Key, value=undefined}. -aggregate(Tab, Key, Value, Options) -> +aggregate(Tab, ViewAggregation=#view_aggregation{name=Name, + reader=ReaderId}, Value, Attributes) -> + Key = {Name, Attributes, ReaderId}, case ets:update_element(Tab, Key, {#last_value_aggregation.value, Value}) of true -> true; false -> - Metric = init(Key, Options), + Metric = init(ViewAggregation, Attributes), ets:insert(Tab, ?assert_type((?assert_type(Metric, #last_value_aggregation{}))#last_value_aggregation{value=Value}, tuple())) end. diff --git a/apps/opentelemetry_experimental/src/otel_aggregation_sum.erl b/apps/opentelemetry_experimental/src/otel_aggregation_sum.erl index 654c2792..f81a0bff 100644 --- a/apps/opentelemetry_experimental/src/otel_aggregation_sum.erl +++ b/apps/opentelemetry_experimental/src/otel_aggregation_sum.erl @@ -26,18 +26,26 @@ -include("otel_metrics.hrl"). -include_lib("opentelemetry_api_experimental/include/otel_metrics.hrl"). +-include("otel_view.hrl"). -type t() :: #sum_aggregation{}. -export_type([t/0]). -init(Key, _) -> +init(#view_aggregation{name=Name, + reader=ReaderId}, Attributes) -> + Key = {Name, Attributes, ReaderId}, #sum_aggregation{key=Key, start_time_unix_nano=erlang:system_time(nanosecond), int_value=0, float_value=0.0}. -aggregate(Tab, Key, Value, _Options) when is_integer(Value) -> +aggregate(Tab, #view_aggregation{name=Name, + reader=ReaderId, + is_monotonic=IsMonotonic}, Value, Attributes) + when is_integer(Value) andalso + ((IsMonotonic andalso Value >= 0) orelse not IsMonotonic) -> + Key = {Name, Attributes, ReaderId}, try _ = ets:update_counter(Tab, Key, {#sum_aggregation.int_value, Value}), true @@ -53,7 +61,11 @@ aggregate(Tab, Key, Value, _Options) when is_integer(Value) -> %% true false end; -aggregate(Tab, Key, Value, _) -> +aggregate(Tab, #view_aggregation{name=Name, + reader=ReaderId, + is_monotonic=IsMonotonic}, Value, Attributes) + when (IsMonotonic andalso Value >= 0.0) orelse not IsMonotonic -> + Key = {Name, Attributes, ReaderId}, MS = [{#sum_aggregation{key=Key, start_time_unix_nano='$1', checkpoint='$2', @@ -65,7 +77,10 @@ aggregate(Tab, Key, Value, _) -> checkpoint='$2', int_value='$3', float_value={'+', '$4', {const, Value}}}}]}], - 1 =:= ets:select_replace(Tab, MS). + 1 =:= ets:select_replace(Tab, MS); +aggregate(_Tab, #view_aggregation{name=_Name, + is_monotonic=_IsMonotonic}, _Value, _) -> + false. -dialyzer({nowarn_function, checkpoint/5}). checkpoint(Tab, Name, ReaderPid, ?AGGREGATION_TEMPORALITY_DELTA, CollectionStartNano) -> diff --git a/apps/opentelemetry_experimental/src/otel_meter_server.erl b/apps/opentelemetry_experimental/src/otel_meter_server.erl index ee6862f7..4a8348fb 100644 --- a/apps/opentelemetry_experimental/src/otel_meter_server.erl +++ b/apps/opentelemetry_experimental/src/otel_meter_server.erl @@ -388,11 +388,8 @@ handle_measurement(Meter, Name, Number, Attributes, ViewAggregationsTab, Metrics update_aggregations(Number, Attributes, Matches, MetricsTab). update_aggregations(Value, Attributes, ViewAggregations, MetricsTab) -> - lists:foreach(fun(#view_aggregation{name=Name, - reader=ReaderId, - aggregation_module=AggregationModule, - aggregation_options=Options}) -> - otel_aggregation:maybe_init_aggregate(MetricsTab, AggregationModule, {Name, Attributes, ReaderId}, Value, Options); + lists:foreach(fun(ViewAggregation=#view_aggregation{}) -> + otel_aggregation:maybe_init_aggregate(MetricsTab, ViewAggregation, Value, Attributes); (_) -> ok end, ViewAggregations). diff --git a/apps/opentelemetry_experimental/src/otel_metric_reader.erl b/apps/opentelemetry_experimental/src/otel_metric_reader.erl index 6de4eae0..722a85fb 100644 --- a/apps/opentelemetry_experimental/src/otel_metric_reader.erl +++ b/apps/opentelemetry_experimental/src/otel_metric_reader.erl @@ -234,12 +234,11 @@ handle_callback_results(Results, ViewAggregationTab, MetricsTab, ReaderId) -> %% single instrument callbacks return a 2-tuple of {number(), opentelemetry:attributes_map()} handle_instrument_observation({Number, Attributes}, [#instrument{meter=Meter, name=Name}], - ViewAggregationTab, MetricsTab, ReaderId) + ViewAggregationTab, MetricsTab, _ReaderId) when is_number(Number) -> try ets:lookup_element(ViewAggregationTab, {Meter, Name}, 2) of ViewAggregations -> - [otel_aggregation:maybe_init_aggregate(MetricsTab, AggregationModule, {Name, Attributes, ReaderId}, Number, AggregationOptions) || #view_aggregation{aggregation_module=AggregationModule, - aggregation_options=AggregationOptions} <- ViewAggregations] + [otel_aggregation:maybe_init_aggregate(MetricsTab, ViewAggregation, Number, Attributes) || #view_aggregation{}=ViewAggregation <- ViewAggregations] catch error:badarg -> %% no Views for this Instrument, so nothing to do @@ -255,11 +254,8 @@ handle_instrument_observation({InstrumentName, Number, Attributes}, Instruments, #instrument{meter=Meter} -> try ets:lookup_element(ViewAggregationTab, {Meter, InstrumentName}, 2) of ViewAggregations -> - [otel_aggregation:maybe_init_aggregate(MetricsTab, AggregationModule, {Name, Attributes, ReaderId}, Number, AggregationOptions) || - #view_aggregation{name=Name, - reader=Id, - aggregation_module=AggregationModule, - aggregation_options=AggregationOptions} <- ViewAggregations, Id =:= ReaderId] + [otel_aggregation:maybe_init_aggregate(MetricsTab, ViewAggregation, Number, Attributes) || + #view_aggregation{reader=Id}=ViewAggregation <- ViewAggregations, Id =:= ReaderId] catch error:badarg -> %% no Views for this Instrument, so nothing to do @@ -315,4 +311,3 @@ data(otel_aggregation_histogram_explicit, Name, Self, Temporality, _IsMonotonic, #histogram{datapoints=Datapoints, aggregation_temporality=Temporality }. - diff --git a/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl b/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl index de7aaf60..6f06eabb 100644 --- a/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl +++ b/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl @@ -274,7 +274,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], 5, 10.3, 31.1}] -- AttributeBuckets, AttributeBuckets) after 5000 -> @@ -348,8 +348,8 @@ provider_test(_Config) -> otel_meter_server:force_flush(), - ?assertSumReceive(a_counter, <<"counter description">>, kb, [{6.0, #{<<"c">> => <<"b">>}}]), - ?assertSumReceive(view_c, <<"counter description">>, kb, [{6.0, #{<<"c">> => <<"b">>}}]), + ?assertSumReceive(a_counter, <<"counter description">>, kb, [{16.0, #{<<"c">> => <<"b">>}}]), + ?assertSumReceive(view_c, <<"counter description">>, kb, [{16.0, #{<<"c">> => <<"b">>}}]), %% sum agg is default delta temporality so counter will reset ?assertEqual(ok, otel_counter:add(Counter, 7, #{<<"c">> => <<"b">>})), From 8e7fc909b96cf0d72e52283832845a316353e9ce Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Sun, 5 Feb 2023 15:45:46 -0700 Subject: [PATCH 4/6] use ViewAggregation to all aggregation functions --- .../src/otel_aggregation.erl | 20 +++++----- .../src/otel_aggregation_drop.erl | 8 ++-- .../otel_aggregation_histogram_explicit.erl | 21 ++++++---- .../src/otel_aggregation_last_value.erl | 17 ++++---- .../src/otel_aggregation_sum.erl | 27 +++++++++---- .../src/otel_metric_reader.erl | 39 +++++-------------- 6 files changed, 65 insertions(+), 67 deletions(-) diff --git a/apps/opentelemetry_experimental/src/otel_aggregation.erl b/apps/opentelemetry_experimental/src/otel_aggregation.erl index 857eb694..1127523f 100644 --- a/apps/opentelemetry_experimental/src/otel_aggregation.erl +++ b/apps/opentelemetry_experimental/src/otel_aggregation.erl @@ -26,9 +26,11 @@ options/0, temporality/0]). --callback init(Key, Options) -> Aggregation when - Key :: key(), - Options :: options(), +%% Returns the aggregation's record as it is seen and updated by +%% the aggregation module in the metrics table. +-callback init(ViewAggregation, Attributes) -> Aggregation when + ViewAggregation :: #view_aggregation{}, + Attributes :: opentelemetry:attributes_map(), Aggregation :: t(). -callback aggregate(Table, ViewAggregation, Value, Attributes) -> boolean() when @@ -37,18 +39,14 @@ Value :: number(), Attributes :: opentelemetry:attributes_map(). --callback checkpoint(Table, Name, ReaderId, Temporality, CollectionStartTime) -> ok when +-callback checkpoint(Table, ViewAggregation, CollectionStartTime) -> ok when Table :: ets:table(), - Name :: atom(), - ReaderId :: reference(), - Temporality :: temporality(), + ViewAggregation :: #view_aggregation{}, CollectionStartTime :: integer(). --callback collect(Table, Name, ReaderId, Temporality, CollectionStartTime) -> [tuple()] when +-callback collect(Table, ViewAggregation, CollectionStartTime) -> [tuple()] when Table :: ets:table(), - Name :: atom(), - ReaderId :: reference(), - Temporality :: temporality(), + ViewAggregation :: #view_aggregation{}, CollectionStartTime :: integer(). maybe_init_aggregate(MetricsTab, ViewAggregation=#view_aggregation{aggregation_module=AggregationModule}, diff --git a/apps/opentelemetry_experimental/src/otel_aggregation_drop.erl b/apps/opentelemetry_experimental/src/otel_aggregation_drop.erl index 873d0609..62a7e42a 100644 --- a/apps/opentelemetry_experimental/src/otel_aggregation_drop.erl +++ b/apps/opentelemetry_experimental/src/otel_aggregation_drop.erl @@ -4,8 +4,8 @@ -export([init/2, aggregate/4, - checkpoint/5, - collect/5]). + checkpoint/3, + collect/3]). -include("otel_metrics.hrl"). @@ -19,8 +19,8 @@ init(_, _) -> aggregate(_, _, _, _) -> true. -checkpoint(_, _, _, _, _) -> +checkpoint(_, _, _) -> ok. -collect(_, _, _, _, _) -> +collect(_, _, _) -> []. diff --git a/apps/opentelemetry_experimental/src/otel_aggregation_histogram_explicit.erl b/apps/opentelemetry_experimental/src/otel_aggregation_histogram_explicit.erl index 3b5200c4..c1886eef 100644 --- a/apps/opentelemetry_experimental/src/otel_aggregation_histogram_explicit.erl +++ b/apps/opentelemetry_experimental/src/otel_aggregation_histogram_explicit.erl @@ -21,8 +21,8 @@ -export([init/2, aggregate/4, - checkpoint/5, - collect/5]). + checkpoint/3, + collect/3]). -include("otel_metrics.hrl"). -include_lib("opentelemetry_api_experimental/include/otel_metrics.hrl"). @@ -130,8 +130,10 @@ aggregate(Table, #view_aggregation{name=Name, false end. --dialyzer({nowarn_function, checkpoint/5}). -checkpoint(Tab, Name, ReaderPid, ?AGGREGATION_TEMPORALITY_DELTA, CollectionStartNano) -> +-dialyzer({nowarn_function, checkpoint/3}). +checkpoint(Tab, #view_aggregation{name=Name, + reader=ReaderId, + temporality=?AGGREGATION_TEMPORALITY_DELTA}, CollectionStartNano) -> MS = [{#explicit_histogram_aggregation{key='$1', start_time_unix_nano='_', boundaries='$2', @@ -143,7 +145,7 @@ checkpoint(Tab, Name, ReaderPid, ?AGGREGATION_TEMPORALITY_DELTA, CollectionStart sum='$8' }, [{'=:=', {element, 1, '$1'}, {const, Name}}, - {'=:=', {element, 3, '$1'}, {const, ReaderPid}}], + {'=:=', {element, 3, '$1'}, {const, ReaderId}}], [{#explicit_histogram_aggregation{key='$1', start_time_unix_nano={const, CollectionStartNano}, boundaries='$2', @@ -159,20 +161,23 @@ checkpoint(Tab, Name, ReaderPid, ?AGGREGATION_TEMPORALITY_DELTA, CollectionStart _ = ets:select_replace(Tab, MS), ok; -checkpoint(_Tab, _Name, _ReaderPid, _, _CollectionStartNano) -> +checkpoint(_Tab, _, _CollectionStartNano) -> %% no good way to checkpoint the `counters' without being out of sync with %% min/max/sum, so may as well just collect them in `collect', which will %% also be out of sync, but best we can do right now ok. -collect(Tab, Name, ReaderPid, _, CollectionStartTime) -> +collect(Tab, #view_aggregation{name=Name, + reader=ReaderPid, + temporality=Temporality}, CollectionStartTime) -> Select = [{'$1', [{'==', Name, {element, 1, {element, 2, '$1'}}}, {'==', ReaderPid, {element, 3, {element, 2, '$1'}}}], ['$1']}], AttributesAggregation = ets:select(Tab, Select), - [datapoint(CollectionStartTime, SumAgg) || SumAgg <- AttributesAggregation]. + #histogram{datapoints=[datapoint(CollectionStartTime, SumAgg) || SumAgg <- AttributesAggregation], + aggregation_temporality=Temporality}. %% diff --git a/apps/opentelemetry_experimental/src/otel_aggregation_last_value.erl b/apps/opentelemetry_experimental/src/otel_aggregation_last_value.erl index b77f0f1d..d8ffd2c5 100644 --- a/apps/opentelemetry_experimental/src/otel_aggregation_last_value.erl +++ b/apps/opentelemetry_experimental/src/otel_aggregation_last_value.erl @@ -21,8 +21,8 @@ -export([init/2, aggregate/4, - checkpoint/5, - collect/5]). + checkpoint/3, + collect/3]). -include("otel_metrics.hrl"). @@ -51,13 +51,14 @@ aggregate(Tab, ViewAggregation=#view_aggregation{name=Name, ets:insert(Tab, ?assert_type((?assert_type(Metric, #last_value_aggregation{}))#last_value_aggregation{value=Value}, tuple())) end. --dialyzer({nowarn_function, checkpoint/5}). -checkpoint(Tab, Name, ReaderPid, _, _CollectionStartNano) -> +-dialyzer({nowarn_function, checkpoint/3}). +checkpoint(Tab, #view_aggregation{name=Name, + reader=ReaderId}, _CollectionStartNano) -> MS = [{#last_value_aggregation{key='$1', checkpoint='_', value='$2'}, [{'=:=', {element, 1, '$1'}, {const, Name}}, - {'=:=', {element, 3, '$1'}, {const, ReaderPid}}], + {'=:=', {element, 3, '$1'}, {const, ReaderId}}], [{#last_value_aggregation{key='$1', checkpoint='$2', value='$2'}}]}], @@ -65,13 +66,15 @@ checkpoint(Tab, Name, ReaderPid, _, _CollectionStartNano) -> ok. -collect(Tab, Name, ReaderPid, _, CollectionStartTime) -> +collect(Tab, #view_aggregation{name=Name, + reader=ReaderPid}, CollectionStartTime) -> Select = [{'$1', [{'=:=', Name, {element, 1, {element, 2, '$1'}}}, {'=:=', ReaderPid, {element, 3, {element, 2, '$1'}}}], ['$1']}], AttributesAggregation = ets:select(Tab, Select), - [datapoint(CollectionStartTime, LastValueAgg) || LastValueAgg <- AttributesAggregation]. + #gauge{datapoints=[datapoint(CollectionStartTime, LastValueAgg) || + LastValueAgg <- AttributesAggregation]}. %% diff --git a/apps/opentelemetry_experimental/src/otel_aggregation_sum.erl b/apps/opentelemetry_experimental/src/otel_aggregation_sum.erl index f81a0bff..b3092b28 100644 --- a/apps/opentelemetry_experimental/src/otel_aggregation_sum.erl +++ b/apps/opentelemetry_experimental/src/otel_aggregation_sum.erl @@ -21,8 +21,8 @@ -export([init/2, aggregate/4, - checkpoint/5, - collect/5]). + checkpoint/3, + collect/3]). -include("otel_metrics.hrl"). -include_lib("opentelemetry_api_experimental/include/otel_metrics.hrl"). @@ -82,8 +82,10 @@ aggregate(_Tab, #view_aggregation{name=_Name, is_monotonic=_IsMonotonic}, _Value, _) -> false. --dialyzer({nowarn_function, checkpoint/5}). -checkpoint(Tab, Name, ReaderPid, ?AGGREGATION_TEMPORALITY_DELTA, CollectionStartNano) -> +-dialyzer({nowarn_function, checkpoint/3}). +checkpoint(Tab, #view_aggregation{name=Name, + reader=ReaderPid, + temporality=?AGGREGATION_TEMPORALITY_DELTA}, CollectionStartNano) -> MS = [{#sum_aggregation{key='$1', start_time_unix_nano='_', checkpoint='_', @@ -111,7 +113,9 @@ checkpoint(Tab, Name, ReaderPid, ?AGGREGATION_TEMPORALITY_DELTA, CollectionStart float_value=0.0}}]}], _ = ets:select_replace(Tab, MS), ok; -checkpoint(Tab, Name, ReaderPid, ?AGGREGATION_TEMPORALITY_CUMULATIVE, _CollectionStartNano) -> +checkpoint(Tab, #view_aggregation{name=Name, + reader=ReaderPid, + temporality=?AGGREGATION_TEMPORALITY_CUMULATIVE}, _CollectionStartNano) -> MS = [{#sum_aggregation{key='$1', start_time_unix_nano='$2', checkpoint='_', @@ -140,13 +144,20 @@ checkpoint(Tab, Name, ReaderPid, ?AGGREGATION_TEMPORALITY_CUMULATIVE, _Collectio _ = ets:select_replace(Tab, MS), ok. -collect(Tab, Name, ReaderPid, _, CollectionStartTime) -> +collect(Tab, #view_aggregation{name=Name, + reader=ReaderId, + temporality=Temporality, + is_monotonic=IsMonotonic}, CollectionStartTime) -> Select = [{'$1', [{'=:=', Name, {element, 1, {element, 2, '$1'}}}, - {'=:=', ReaderPid, {element, 3, {element, 2, '$1'}}}], + {'=:=', ReaderId, {element, 3, {element, 2, '$1'}}}], ['$1']}], AttributesAggregation = ets:select(Tab, Select), - [datapoint(CollectionStartTime, SumAgg) || SumAgg <- AttributesAggregation]. + #sum{aggregation_temporality=Temporality, + is_monotonic=IsMonotonic, + datapoints=[datapoint(CollectionStartTime, SumAgg) || SumAgg <- AttributesAggregation]}. + + datapoint(CollectionStartNano, #sum_aggregation{key={_, Attributes, _}, start_time_unix_nano=StartTimeUnixNano, diff --git a/apps/opentelemetry_experimental/src/otel_metric_reader.erl b/apps/opentelemetry_experimental/src/otel_metric_reader.erl index 722a85fb..f4e6ab27 100644 --- a/apps/opentelemetry_experimental/src/otel_metric_reader.erl +++ b/apps/opentelemetry_experimental/src/otel_metric_reader.erl @@ -269,22 +269,18 @@ handle_instrument_observation(_, _, _, _, _) -> checkpoint_metrics(MetricsTab, CollectionStartTime, Id, ViewAggregations) -> lists:foldl(fun(#view_aggregation{aggregation_module=otel_aggregation_drop}, Acc) -> Acc; - (#view_aggregation{name=Name, - reader=ReaderId, - instrument=Instrument=#instrument{unit=Unit}, - aggregation_module=AggregationModule, - description=Description, - temporality=Temporality, - is_monotonic=IsMonotonic - }, Acc) when Id =:= ReaderId -> + (ViewAggregation=#view_aggregation{name=Name, + reader=ReaderId, + instrument=Instrument=#instrument{unit=Unit}, + aggregation_module=AggregationModule, + description=Description + }, Acc) when Id =:= ReaderId -> AggregationModule:checkpoint(MetricsTab, - Name, - ReaderId, - Temporality, + ViewAggregation, CollectionStartTime), - Data = data(AggregationModule, Name, ReaderId, Temporality, IsMonotonic, - CollectionStartTime, MetricsTab), - + Data = AggregationModule:collect(MetricsTab, + ViewAggregation, + CollectionStartTime), [metric(Instrument, Name, Description, Unit, Data) | Acc]; (_, Acc) -> Acc @@ -296,18 +292,3 @@ metric(#instrument{meter=Meter}, Name, Description, Unit, Data) -> description=Description, unit=Unit, data=Data}. - -data(otel_aggregation_sum, Name, Self, Temporality, IsMonotonic, CollectionStartTime, MetricTab) -> - Datapoints = otel_aggregation_sum:collect(MetricTab, Name, Self, Temporality, CollectionStartTime), - #sum{ - aggregation_temporality=Temporality, - is_monotonic=IsMonotonic, - datapoints=Datapoints}; -data(otel_aggregation_last_value, Name, Self, Temporality, _IsMonotonic, CollectionStartTime, MetricTab) -> - Datapoints = otel_aggregation_last_value:collect(MetricTab, Name, Self, Temporality, CollectionStartTime), - #gauge{datapoints=Datapoints}; -data(otel_aggregation_histogram_explicit, Name, Self, Temporality, _IsMonotonic, CollectionStartTime, MetricTab) -> - Datapoints = otel_aggregation_histogram_explicit:collect(MetricTab, Name, Self, Temporality, CollectionStartTime), - #histogram{datapoints=Datapoints, - aggregation_temporality=Temporality - }. From 82feb8a958c41b597b6ba6c80cc7476b45773729 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Mon, 6 Feb 2023 05:14:20 -0700 Subject: [PATCH 5/6] use atoms for more attribute/event key names --- apps/opentelemetry/test/opentelemetry_SUITE.erl | 16 ++++++++-------- apps/opentelemetry_api/src/otel_span.erl | 4 ++-- .../test/opentelemetry_exporter_SUITE.erl | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/apps/opentelemetry/test/opentelemetry_SUITE.erl b/apps/opentelemetry/test/opentelemetry_SUITE.erl index d67eb0c1..c89d0b4f 100644 --- a/apps/opentelemetry/test/opentelemetry_SUITE.erl +++ b/apps/opentelemetry/test/opentelemetry_SUITE.erl @@ -868,9 +868,9 @@ record_exception_works(Config) -> otel_span:end_span(SpanCtx), [Span] = assert_exported(Tid, SpanCtx), [Event] = otel_events:list(Span#span.events), - ?assertEqual(<<"exception">>, Event#event.name), - ?assertEqual(#{<<"exception.type">> => <<"throw:my_error">>, - <<"exception.stacktrace">> => list_to_binary(io_lib:format("~p", [Stacktrace], [{chars_limit, 50}])), + ?assertEqual(exception, Event#event.name), + ?assertEqual(#{'exception.type' => <<"throw:my_error">>, + 'exception.stacktrace' => list_to_binary(io_lib:format("~p", [Stacktrace], [{chars_limit, 50}])), <<"some-attribute">> => <<"value">>}, otel_attributes:map(Event#event.attributes)), ok @@ -888,10 +888,10 @@ record_exception_with_message_works(Config) -> otel_span:end_span(SpanCtx), [Span] = assert_exported(Tid, SpanCtx), [Event] = otel_events:list(Span#span.events), - ?assertEqual(<<"exception">>, Event#event.name), - ?assertEqual(#{<<"exception.type">> => <<"throw:my_error">>, - <<"exception.stacktrace">> => list_to_binary(io_lib:format("~p", [Stacktrace], [{chars_limit, 50}])), - <<"exception.message">> => <<"My message">>, + ?assertEqual(exception, Event#event.name), + ?assertEqual(#{'exception.type' => <<"throw:my_error">>, + 'exception.stacktrace' => list_to_binary(io_lib:format("~p", [Stacktrace], [{chars_limit, 50}])), + 'exception.message' => <<"My message">>, <<"some-attribute">> => <<"value">>}, otel_attributes:map(Event#event.attributes) ), @@ -916,7 +916,7 @@ dropped_attributes(Config) -> truncated_binary_attributes(_Config) -> InfinityLengthAttributes = otel_attributes:new(#{<<"attr-1">> => <<"abcde">>, - <<"attr-2">> => [<<"a">>, <<"abcde">>, <<"abcde">>]}, + <<"attr-2">> => [<<"a">>, <<"abcde">>, <<"abcde">>]}, 128, infinity), %% when length limit is inifinity diff --git a/apps/opentelemetry_api/src/otel_span.erl b/apps/opentelemetry_api/src/otel_span.erl index 7ce10013..3d10feab 100644 --- a/apps/opentelemetry_api/src/otel_span.erl +++ b/apps/opentelemetry_api/src/otel_span.erl @@ -266,7 +266,7 @@ record_exception(SpanCtx, Class, Term, Stacktrace, Attributes) when is_map(Attri {ok, StacktraceString} = otel_utils:format_binary_string("~0tP", [Stacktrace, 10], [{chars_limit, 50}]), ExceptionAttributes = #{?EXCEPTION_TYPE => ExceptionType, ?EXCEPTION_STACKTRACE => StacktraceString}, - add_event(SpanCtx, <<"exception">>, maps:merge(ExceptionAttributes, Attributes)); + add_event(SpanCtx, 'exception', maps:merge(ExceptionAttributes, Attributes)); record_exception(_, _, _, _, _) -> false. @@ -285,7 +285,7 @@ record_exception(SpanCtx, Class, Term, Message, Stacktrace, Attributes) when is_ ExceptionAttributes = #{?EXCEPTION_TYPE => ExceptionType, ?EXCEPTION_STACKTRACE => StacktraceString, ?EXCEPTION_MESSAGE => Message}, - add_event(SpanCtx, <<"exception">>, maps:merge(ExceptionAttributes, Attributes)); + add_event(SpanCtx, 'exception', maps:merge(ExceptionAttributes, Attributes)); record_exception(_, _, _, _, _, _) -> false. diff --git a/apps/opentelemetry_exporter/test/opentelemetry_exporter_SUITE.erl b/apps/opentelemetry_exporter/test/opentelemetry_exporter_SUITE.erl index ea7b5236..1034c019 100644 --- a/apps/opentelemetry_exporter/test/opentelemetry_exporter_SUITE.erl +++ b/apps/opentelemetry_exporter/test/opentelemetry_exporter_SUITE.erl @@ -509,8 +509,8 @@ verify_export(Config) -> ?assertMatch([#{spans := [_, _]}], otel_otlp_traces:to_proto_by_instrumentation_scope(Tid)), Resource = otel_resource_env_var:get_resource([]), - ?assertEqual(otel_attributes:new([{<<"service.name">>,<<"my-test-service">>}, - {<<"service.version">>,<<"98da75ea6d38724743bf42b45565049238d86b3f">>}], 128, 255), otel_resource:attributes(Resource)), + ?assertEqual(otel_attributes:new([{'service.name',<<"my-test-service">>}, + {'service.version',<<"98da75ea6d38724743bf42b45565049238d86b3f">>}], 128, 255), otel_resource:attributes(Resource)), ?assertMatch(ok, opentelemetry_exporter:export(traces, Tid, Resource, State)), ok. From 4c2e721a886c2e4fa93fcb0460e57ab584dd706c Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Tue, 7 Feb 2023 17:31:23 -0700 Subject: [PATCH 6/6] improve some typing thanks to eqwalizer This has more dialyzer ignores added in order to remove stuff like '$1' and '_' in type specs. I figure this is acceptable because eqwalizer has a more fine grained ignore ability which will let us check these parts of the code better even if the dialyzer warnings are now ignored from those parts of the code. --- .../include/otel_metrics.hrl | 16 ++--- .../src/opentelemetry_experimental.erl | 10 +-- .../src/otel_meter.erl | 4 +- .../include/otel_metrics.hrl | 2 +- .../src/otel_aggregation.erl | 4 +- .../src/otel_aggregation_drop.erl | 2 +- .../src/otel_meter_server.erl | 4 ++ .../src/otel_metric_reader.erl | 2 +- .../src/otel_view.erl | 68 +++++++++++-------- .../src/otel_view.hrl | 8 +-- .../test/otel_metrics_SUITE.erl | 56 ++++++++++++++- 11 files changed, 121 insertions(+), 55 deletions(-) diff --git a/apps/opentelemetry_api_experimental/include/otel_metrics.hrl b/apps/opentelemetry_api_experimental/include/otel_metrics.hrl index d336c789..15fc740f 100644 --- a/apps/opentelemetry_api_experimental/include/otel_metrics.hrl +++ b/apps/opentelemetry_api_experimental/include/otel_metrics.hrl @@ -1,11 +1,11 @@ --record(instrument, {module :: module() | '_', - meter :: otel_meter:t() | '_', - name :: otel_instrument:name() | '_', - description :: otel_instrument:description() | undefined | '_', - kind :: otel_instrument:kind() | '_', - unit :: otel_instrument:unit() | undefined | '_', - callback :: otel_instrument:callback() | undefined | '_', - callback_args :: term() | undefined | '_'}). +-record(instrument, {module :: module(), + meter :: otel_meter:t(), + name :: otel_instrument:name(), + description :: otel_instrument:description(), + kind :: otel_instrument:kind(), + unit :: otel_instrument:unit() | undefined, + callback :: otel_instrument:callback() | undefined, + callback_args :: term() | undefined}). -define(KIND_COUNTER, counter). -define(KIND_OBSERVABLE_COUNTER, observable_counter). diff --git a/apps/opentelemetry_api_experimental/src/opentelemetry_experimental.erl b/apps/opentelemetry_api_experimental/src/opentelemetry_experimental.erl index da8e93c4..ee290e5c 100644 --- a/apps/opentelemetry_api_experimental/src/opentelemetry_experimental.erl +++ b/apps/opentelemetry_api_experimental/src/opentelemetry_experimental.erl @@ -61,7 +61,7 @@ get_meter_(MeterProvider) -> Name :: atom() | {atom(), Vsn, SchemaUrl}, Vsn :: unicode:chardata() | undefined, SchemaUrl :: uri_string:uri_string() | undefined, - Meter:: opentelemetry:meter(). + Meter:: meter(). get_meter('$__default_meter') -> get_meter(); get_meter({Name, Vsn, SchemaUrl}) -> @@ -73,7 +73,7 @@ get_meter(Name) -> Name :: atom(), Vsn :: unicode:chardata() | undefined, SchemaUrl :: uri_string:uri_string() | undefined, - Meter:: opentelemetry:meter(). + Meter:: meter(). get_meter(Name, Vsn, SchemaUrl) -> get_meter(?GLOBAL_METER_PROVIDER_NAME, Name, Vsn, SchemaUrl). @@ -82,7 +82,7 @@ get_meter(Name, Vsn, SchemaUrl) -> Name :: atom(), Vsn :: unicode:chardata() | undefined, SchemaUrl :: uri_string:uri_string() | undefined, - Meter:: opentelemetry:meter(). + Meter:: meter(). get_meter(MeterProvider, Name, Vsn, SchemaUrl) -> %% check cache and then use provider to get the meter if it isn't cached yet case persistent_term:get(?METER_KEY(MeterProvider, {Name, Vsn, SchemaUrl}), undefined) of @@ -106,7 +106,7 @@ set_meter(Name, Meter) -> Name :: atom(), Vsn :: unicode:chardata() | undefined, SchemaUrl :: uri_string:uri_string() | undefined, - Meter:: opentelemetry:meter(). + Meter:: meter(). set_meter(Name, Vsn, SchemaUrl, Meter) -> set_meter(?GLOBAL_METER_PROVIDER_NAME, Name, Vsn, SchemaUrl, Meter). @@ -115,6 +115,6 @@ set_meter(Name, Vsn, SchemaUrl, Meter) -> Name :: atom(), Vsn :: unicode:chardata() | undefined, SchemaUrl :: uri_string:uri_string() | undefined, - Meter:: opentelemetry:meter(). + Meter:: meter(). set_meter(MeterProvider, Name, Vsn, SchemaUrl, Meter) -> opentelemetry:verify_and_set_term(Meter, ?METER_KEY(MeterProvider, {Name, Vsn, SchemaUrl}), otel_meter). diff --git a/apps/opentelemetry_api_experimental/src/otel_meter.erl b/apps/opentelemetry_api_experimental/src/otel_meter.erl index 4da4f902..3b463eb7 100644 --- a/apps/opentelemetry_api_experimental/src/otel_meter.erl +++ b/apps/opentelemetry_api_experimental/src/otel_meter.erl @@ -40,7 +40,7 @@ Meter :: t(), Name :: otel_instrument:name(), Kind :: otel_instrument:kind(), - Opts :: otel_meter:opts(). + Opts :: opts(). -callback create_instrument(Meter, Name, Kind, Callback, CallbackArgs, Opts) -> otel_instrument:t() when Meter :: t(), @@ -48,7 +48,7 @@ Kind :: otel_instrument:kind(), Callback :: otel_instrument:callback(), CallbackArgs :: term(), - Opts :: otel_meter:opts(). + Opts :: opts(). -callback register_callback(Meter, Instruments, Callback, CallbackArgs) -> ok when Meter :: t(), diff --git a/apps/opentelemetry_experimental/include/otel_metrics.hrl b/apps/opentelemetry_experimental/include/otel_metrics.hrl index 424b39c9..edf030a0 100644 --- a/apps/opentelemetry_experimental/include/otel_metrics.hrl +++ b/apps/opentelemetry_experimental/include/otel_metrics.hrl @@ -95,7 +95,7 @@ time_unix_nano :: integer(), count :: number(), sum :: float(), - bucket_counts :: tuple(), + bucket_counts :: list(), explicit_bounds :: [float()], exemplars :: list(), flags :: integer(), diff --git a/apps/opentelemetry_experimental/src/otel_aggregation.erl b/apps/opentelemetry_experimental/src/otel_aggregation.erl index 1127523f..4d6632f6 100644 --- a/apps/opentelemetry_experimental/src/otel_aggregation.erl +++ b/apps/opentelemetry_experimental/src/otel_aggregation.erl @@ -17,7 +17,7 @@ -type t() :: otel_aggregation_drop:t() | otel_aggregation_sum:t() | otel_aggregation_last_value:t() | otel_aggregation_histogram_explicit:t(). --type key() :: {atom(), opentelemetry:attributes_maps(), reference()}. +-type key() :: {atom(), opentelemetry:attributes_map(), reference()}. -type options() :: map(). @@ -44,7 +44,7 @@ ViewAggregation :: #view_aggregation{}, CollectionStartTime :: integer(). --callback collect(Table, ViewAggregation, CollectionStartTime) -> [tuple()] when +-callback collect(Table, ViewAggregation, CollectionStartTime) -> tuple() when Table :: ets:table(), ViewAggregation :: #view_aggregation{}, CollectionStartTime :: integer(). diff --git a/apps/opentelemetry_experimental/src/otel_aggregation_drop.erl b/apps/opentelemetry_experimental/src/otel_aggregation_drop.erl index 62a7e42a..9565ab12 100644 --- a/apps/opentelemetry_experimental/src/otel_aggregation_drop.erl +++ b/apps/opentelemetry_experimental/src/otel_aggregation_drop.erl @@ -23,4 +23,4 @@ checkpoint(_, _, _) -> ok. collect(_, _, _) -> - []. + {}. diff --git a/apps/opentelemetry_experimental/src/otel_meter_server.erl b/apps/opentelemetry_experimental/src/otel_meter_server.erl index 4a8348fb..1cb3afc0 100644 --- a/apps/opentelemetry_experimental/src/otel_meter_server.erl +++ b/apps/opentelemetry_experimental/src/otel_meter_server.erl @@ -285,6 +285,7 @@ metrics_tab(Name) -> {keypos, 2}, public]). +-dialyzer({nowarn_function,new_view/1}). new_view(ViewConfig) -> Name = maps:get(name, ViewConfig, undefined), Description = maps:get(description, ViewConfig, undefined), @@ -416,6 +417,9 @@ view_aggregation_for_reader(Instrument=#instrument{kind=Kind}, ViewAggregation, %% global default, so any missing Kind entries are filled in from the global %% mapping in `otel_aggregation' -spec aggregation_module(otel_instrument:t(), otel_view:t(), reader()) -> module(). +aggregation_module(#instrument{kind=Kind}, undefined, + #reader{default_aggregation_mapping=ReaderAggregationMapping}) -> + maps:get(Kind, ReaderAggregationMapping); aggregation_module(#instrument{kind=Kind}, #view{aggregation_module=undefined}, #reader{default_aggregation_mapping=ReaderAggregationMapping}) -> maps:get(Kind, ReaderAggregationMapping); diff --git a/apps/opentelemetry_experimental/src/otel_metric_reader.erl b/apps/opentelemetry_experimental/src/otel_metric_reader.erl index 598f1421..307e63d1 100644 --- a/apps/opentelemetry_experimental/src/otel_metric_reader.erl +++ b/apps/opentelemetry_experimental/src/otel_metric_reader.erl @@ -72,7 +72,7 @@ init([ReaderId, ProviderSup, Config]) -> ExporterModuleConfig = maps:get(exporter, Config, undefined), Exporter = otel_exporter:init(ExporterModuleConfig), - DefaultAggregationMapping = maps:get(default_aggregation_mapping, Config, #{}), + DefaultAggregationMapping = maps:get(default_aggregation_mapping, Config, otel_aggregation:default_mapping()), Temporality = maps:get(default_temporality_mapping, Config, #{}), %% if a periodic reader is needed then this value is set diff --git a/apps/opentelemetry_experimental/src/otel_view.erl b/apps/opentelemetry_experimental/src/otel_view.erl index a006295e..d2a98834 100644 --- a/apps/opentelemetry_experimental/src/otel_view.erl +++ b/apps/opentelemetry_experimental/src/otel_view.erl @@ -51,6 +51,7 @@ %% no name means Instrument name is used %% must reject wildcard Criteria in this case +-dialyzer({nowarn_function,new/2}). -spec new(criteria() | undefined, config()) -> t(). new(Criteria, Config) -> CriteriaInstrumentName = view_name_from_criteria(Criteria), @@ -60,10 +61,11 @@ new(Criteria, Config) -> #view{name=CriteriaInstrumentName, instrument_matchspec=Matchspec, description=maps:get(description, Config, undefined), - attribute_keys=maps:get(attribute_keys, Config, undefined), + attribute_keys=maps:get(attribute_keys, Config, []), aggregation_module=maps:get(aggregation_module, Config, undefined), aggregation_options=maps:get(aggregation_options, Config, #{})}. +-dialyzer({nowarn_function,new/3}). -spec new(name(), criteria() | undefined, config()) -> t(). new(undefined, Criteria, Config) -> new(Criteria, Config); @@ -71,8 +73,9 @@ new(Name, Criteria, Config) -> View = new(Criteria, Config), View#view{name=Name}. +-dialyzer({nowarn_function,match_instrument_to_views/2}). -spec match_instrument_to_views(otel_instrument:t(), [otel_view:t()]) -> - [{otel_view:t(), #view_aggregation{}}]. + [{otel_view:t() | undefined, #view_aggregation{}}]. match_instrument_to_views(Instrument=#instrument{name=InstrumentName, meter=Meter, description=Description}, Views) -> @@ -83,33 +86,33 @@ match_instrument_to_views(Instrument=#instrument{name=InstrumentName, description=ViewDescription, aggregation_options=AggregationOptions, instrument_matchspec=Matchspec}) -> - case ets:match_spec_run([Instrument], Matchspec) of - [] -> - false; - _ -> - {true, {View, #view_aggregation{name=value_or(ViewName, - InstrumentName), - scope=Scope, - instrument=Instrument, - temporality=Temporality, - is_monotonic=IsMonotonic, - aggregation_options=AggregationOptions, - description=value_or(ViewDescription, - Description) - }}} - end - end, Views) of - [] -> - [{#view{}, #view_aggregation{name=InstrumentName, - scope=Scope, - instrument=Instrument, - temporality=Temporality, - is_monotonic=IsMonotonic, - aggregation_options=#{}, - description=Description}}]; - Aggs -> - Aggs - end. + case ets:match_spec_run([Instrument], Matchspec) of + [] -> + false; + _ -> + {true, {View, #view_aggregation{name=value_or(ViewName, + InstrumentName), + scope=Scope, + instrument=Instrument, + temporality=Temporality, + is_monotonic=IsMonotonic, + aggregation_options=AggregationOptions, + description=value_or(ViewDescription, + Description) + }}} + end + end, Views) of + [] -> + [{undefined, #view_aggregation{name=InstrumentName, + scope=Scope, + instrument=Instrument, + temporality=Temporality, + is_monotonic=IsMonotonic, + aggregation_options=#{}, + description=Description}}]; + Aggs -> + Aggs + end. %% @@ -118,6 +121,7 @@ value_or(undefined, Other) -> value_or(Value, _Other) -> Value. +-dialyzer({nowarn_function,criteria_to_instrument_matchspec/1}). -spec criteria_to_instrument_matchspec(map() | undefined) -> ets:compiled_match_spec(). criteria_to_instrument_matchspec(Criteria) when is_map(Criteria) -> Instrument = @@ -137,21 +141,27 @@ criteria_to_instrument_matchspec(Criteria) when is_map(Criteria) -> Meter = maybe_init_meter(InstrumentAcc), Meter1 = update_meter_schema_url(SchemaUrl, Meter), InstrumentAcc#instrument{meter=Meter1} + %% eqwalizer:ignore building a matchspec and don't want '_' polluting the type end, #instrument{_='_'}, Criteria), ets:match_spec_compile([{Instrument, [], [true]}]); criteria_to_instrument_matchspec(_) -> + %% eqwalizer:ignore building a matchspec and don't want '_' polluting the type ets:match_spec_compile([{#instrument{_='_'}, [], [true]}]). +-dialyzer({nowarn_function,maybe_init_meter/1}). maybe_init_meter(#instrument{meter='_'}) -> {'_', #meter{instrumentation_scope=#instrumentation_scope{_='_'}, _='_'}}. +-dialyzer({nowarn_function,update_meter_name/2}). update_meter_name(MeterName, {_, Meter=#meter{instrumentation_scope=Scope}}) -> {'_', Meter#meter{instrumentation_scope=Scope#instrumentation_scope{name=MeterName}}}. +-dialyzer({nowarn_function,update_meter_version/2}). update_meter_version(MeterVersion, {_, Meter=#meter{instrumentation_scope=Scope}}) -> {'_', Meter#meter{instrumentation_scope=Scope#instrumentation_scope{version=MeterVersion}}}. +-dialyzer({nowarn_function,update_meter_schema_url/2}). update_meter_schema_url(SchemaUrl, {_, Meter=#meter{instrumentation_scope=Scope}}) -> {'_', Meter#meter{instrumentation_scope=Scope#instrumentation_scope{schema_url=SchemaUrl}}}. diff --git a/apps/opentelemetry_experimental/src/otel_view.hrl b/apps/opentelemetry_experimental/src/otel_view.hrl index 4a66e735..f0b927a7 100644 --- a/apps/opentelemetry_experimental/src/otel_view.hrl +++ b/apps/opentelemetry_experimental/src/otel_view.hrl @@ -1,9 +1,9 @@ -record(view, - {name :: otel_instrument:name() | '_' | undefined, - instrument_matchspec :: ets:compiled_match_spec() | undefined, + {name :: otel_instrument:name(), + instrument_matchspec :: ets:compiled_match_spec(), description :: unicode:unicode_binary() | undefined, - attribute_keys :: [opentelemetry:attribute_key()] | undefined, - aggregation_module :: module() | undefined, + attribute_keys :: [opentelemetry:attribute_key()], + aggregation_module :: module(), aggregation_options=#{} :: map()}). -record(view_aggregation, diff --git a/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl b/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl index 6f06eabb..a08e3960 100644 --- a/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl +++ b/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl @@ -75,8 +75,8 @@ all() -> [default_view, provider_test, view_creation_test, counter_add, multiple_readers, - explicit_histograms, delta_explicit_histograms, cumulative_counter, kill_reader, kill_server, - observable_counter, observable_updown_counter, observable_gauge, + explicit_histograms, delta_explicit_histograms, delta_counter, cumulative_counter, + kill_reader, kill_server, observable_counter, observable_updown_counter, observable_gauge, multi_instrument_callback, using_macros, float_counter, float_updown_counter, float_histogram]. init_per_suite(Config) -> @@ -118,6 +118,17 @@ init_per_testcase(multiple_readers, Config) -> {ok, _} = application:ensure_all_started(opentelemetry_experimental), + Config; +init_per_testcase(delta_counter, Config) -> + application:load(opentelemetry_experimental), + + %% 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()}}}]), + + {ok, _} = application:ensure_all_started(opentelemetry_experimental), + Config; init_per_testcase(cumulative_counter, Config) -> CumulativeCounterTemporality = #{?KIND_COUNTER =>?AGGREGATION_TEMPORALITY_CUMULATIVE}, @@ -571,6 +582,47 @@ delta_explicit_histograms(_Config) -> ok. +delta_counter(_Config) -> + DefaultMeter = otel_meter_default, + + Meter = opentelemetry_experimental:get_meter(), + ?assertMatch({DefaultMeter, _}, Meter), + + CounterName = a_counter, + CounterDesc = <<"counter description">>, + CounterUnit = kb, + + Counter = otel_counter:create(Meter, CounterName, + #{description => CounterDesc, + unit => CounterUnit}), + ?assertMatch(#instrument{meter = {DefaultMeter,_}, + module = DefaultMeter, + name = CounterName, + description = CounterDesc, + kind = counter, + unit = CounterUnit}, Counter), + + otel_meter_server:add_view(#{instrument_name => a_counter}, #{}), + + ?assertEqual(ok, otel_counter:add(Counter, 2, #{<<"c">> => <<"b">>})), + ?assertEqual(ok, otel_counter:add(Counter, 3, #{<<"a">> => <<"b">>, <<"d">> => <<"e">>})), + ?assertEqual(ok, otel_counter:add(Counter, 4, #{<<"c">> => <<"b">>})), + + otel_meter_server:force_flush(), + + ?assertSumReceive(a_counter, <<"counter description">>, kb, [{6, #{<<"c">> => <<"b">>}}]), + + ?assertEqual(ok, otel_counter:add(Counter, 5, #{<<"c">> => <<"b">>})), + + ?assertEqual(ok, otel_counter:add(Counter, 3, #{<<"a">> => <<"b">>, <<"d">> => <<"e">>})), + ?assertEqual(ok, otel_counter:add(Counter, 7, #{<<"c">> => <<"b">>})), + + otel_meter_server:force_flush(), + + ?assertSumReceive(a_counter, <<"counter description">>, kb, [{12, #{<<"c">> => <<"b">>}}]), + + ok. + cumulative_counter(_Config) -> DefaultMeter = otel_meter_default,