diff --git a/apps/opentelemetry_experimental/src/otel_meter_server.erl b/apps/opentelemetry_experimental/src/otel_meter_server.erl index e99cd82b..cd8a4f6d 100644 --- a/apps/opentelemetry_experimental/src/otel_meter_server.erl +++ b/apps/opentelemetry_experimental/src/otel_meter_server.erl @@ -177,9 +177,7 @@ init([Name, RegName, Resource, Config]) -> %% TODO: don't do this if its already set? opentelemetry_experimental:set_default_meter(Name, {otel_meter_default, Meter}), - %% TODO: drop View if Criteria is a wildcard instrument name and View - %% name is not undefined - Views = [new_view(V) || V <- maps:get(views, Config, [])], + Views = lists:filtermap(fun new_view/1, maps:get(views, Config, [])), {ok, #state{shared_meter=Meter, instruments_tab=InstrumentsTab, @@ -235,10 +233,13 @@ handle_call({add_view, Name, Criteria, Config}, _From, State=#state{views=Views, callbacks_tab=CallbacksTab, view_aggregations_tab=ViewAggregationsTab, readers=Readers}) -> - %% TODO: drop View if Criteria is a wildcard instrument name and View name is not undefined - NewView = otel_view:new(Name, Criteria, Config), - _ = update_view_aggregations(InstrumentsTab, CallbacksTab, ViewAggregationsTab, [NewView], Readers), - {reply, true, State#state{views=[NewView | Views]}}; + case otel_view:new(Name, Criteria, Config) of + {ok, NewView} -> + _ = update_view_aggregations(InstrumentsTab, CallbacksTab, ViewAggregationsTab, [NewView], Readers), + {reply, true, State#state{views=[NewView | Views]}}; + error -> + {reply, false, State} + end; handle_call(force_flush, _From, State=#state{readers=Readers}) -> [otel_metric_reader:collect(Pid) || #reader{pid=Pid} <- Readers], {reply, ok, State}. @@ -290,11 +291,14 @@ new_view(ViewConfig) -> AttributeKeys = maps:get(attribute_keys, ViewConfig, undefined), AggregationModule = maps:get(aggregation_module, ViewConfig, undefined), AggregationOptions = maps:get(aggregation_options, ViewConfig, #{}), - otel_view:new(Name, Selector, #{description => Description, + case otel_view:new(Name, Selector, #{description => Description, attribute_keys => AttributeKeys, aggregation_module => AggregationModule, aggregation_options => AggregationOptions - }). + }) of + {ok, View} -> {true, View}; + error -> false + end. %% Match the Instrument to views and then store a per-Reader aggregation for the View add_instrument_(InstrumentsTab, CallbacksTab, ViewAggregationsTab, Instrument=#instrument{meter=Meter, diff --git a/apps/opentelemetry_experimental/src/otel_view.erl b/apps/opentelemetry_experimental/src/otel_view.erl index c2e9e960..fd328a0f 100644 --- a/apps/opentelemetry_experimental/src/otel_view.erl +++ b/apps/opentelemetry_experimental/src/otel_view.erl @@ -21,6 +21,7 @@ new/3, match_instrument_to_views/2]). +-include_lib("kernel/include/logger.hrl"). -include_lib("opentelemetry_api_experimental/include/otel_metrics.hrl"). -include_lib("opentelemetry_api/include/opentelemetry.hrl"). -include("otel_metrics.hrl"). @@ -50,11 +51,29 @@ -include_lib("opentelemetry_api/include/gradualizer.hrl"). -%% 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(). +-spec new(criteria() | undefined, config()) -> {ok, t()} | error. new(Criteria, Config) -> + new(undefined, Criteria, Config). + +-dialyzer({nowarn_function,new/3}). +-spec new(name(), criteria() | undefined, config()) -> {ok, t()} | error. +new(undefined, Criteria, Config) -> + {ok, do_new(Criteria, Config)}; +new(Name, Criteria, Config) -> + case do_new(Criteria, Config) of + #view{name='*'} -> + ?LOG_INFO("Wildacrd Views can not have a name, discarding view ~s", [Name]), + error; + View -> + {ok, View#view{name=Name}} + end. + +%% no name means Instrument name is used +%% must reject wildcard Criteria in this case +-dialyzer({nowarn_function,do_new/2}). +do_new(Criteria, Config) -> CriteriaInstrumentName = view_name_from_criteria(Criteria), Matchspec = criteria_to_instrument_matchspec(Criteria), %% no name given so use the name of the instrument in the selection @@ -66,14 +85,6 @@ new(Criteria, 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); -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(), [t()]) -> [{t() | undefined, #view_aggregation{}}]. match_instrument_to_views(Instrument=#instrument{name=InstrumentName, @@ -128,10 +139,12 @@ value_or(Value, _Other) -> Value. -dialyzer({nowarn_function,criteria_to_instrument_matchspec/1}). --spec criteria_to_instrument_matchspec(map() | undefined) -> ets:compiled_match_spec(). +-spec criteria_to_instrument_matchspec(map() | undefined) -> ets:comp_match_spec(). criteria_to_instrument_matchspec(Criteria) when is_map(Criteria) -> Instrument = - maps:fold(fun(instrument_name, InstrumentName, InstrumentAcc) -> + maps:fold(fun(instrument_name, '*', InstrumentAcc) -> + InstrumentAcc; + (instrument_name, InstrumentName, InstrumentAcc) -> InstrumentAcc#instrument{name=InstrumentName}; (instrument_kind, Kind, InstrumentAcc) -> InstrumentAcc#instrument{kind=Kind}; @@ -174,6 +187,9 @@ update_meter_schema_url(SchemaUrl, {_, Meter=#meter{instrumentation_scope=Scope} {'_', Meter#meter{instrumentation_scope=Scope#instrumentation_scope{schema_url=SchemaUrl}}}. view_name_from_criteria(Criteria) when is_map(Criteria) -> - maps:get(instrument_name, Criteria, undefined); + case maps:get(instrument_name, Criteria, undefined) of + '*' -> undefined; + Name -> Name + end; view_name_from_criteria(_) -> undefined. diff --git a/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl b/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl index 73638f13..28ab441b 100644 --- a/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl +++ b/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl @@ -64,21 +64,21 @@ -define(assertNotReceive(Name, Description, Unit), (fun() -> receive - M=#metric{name=MetricName, - description=MetricDescription, - unit=MetricUnit} + {otel_metric, M=#metric{name=MetricName, + description=MetricDescription, + unit=MetricUnit}} when MetricName =:= Name, MetricDescription =:= Description, MetricUnit =:= Unit -> ct:fail({metric_received, M}) after - 0 -> + 50 -> ok end end)()). all() -> - [default_view, provider_test, view_creation_test, counter_add, multiple_readers, + [default_view, provider_test, view_creation_test, wildcard_view, counter_add, multiple_readers, 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, @@ -433,17 +433,17 @@ view_creation_test(_Config) -> ?assert(otel_meter_server:add_view(view_a, #{instrument_name => a_counter}, #{aggregation_module => otel_aggregation_sum})), - View = otel_view:new(#{instrument_name => a_counter}, #{aggregation_module => otel_aggregation_sum}), + {ok, View} = otel_view:new(#{instrument_name => a_counter}, #{aggregation_module => otel_aggregation_sum}), %% view name becomes the instrument name ?assertEqual(a_counter, View#view.name), Matches = otel_view:match_instrument_to_views(Counter, [View]), ?assertMatch([_], Matches), - ViewUnitMatch = otel_view:new(#{instrument_name => CounterName, instrument_unit => CounterUnit}, #{aggregation_module => otel_aggregation_sum}), + {ok, ViewUnitMatch} = otel_view:new(#{instrument_name => CounterName, instrument_unit => CounterUnit}, #{aggregation_module => otel_aggregation_sum}), ?assertMatch([{#view{}, _}], otel_view:match_instrument_to_views(Counter, [ViewUnitMatch])), - ViewUnitNotMatch = otel_view:new(#{instrument_name => CounterName, instrument_unit => not_matching}, #{aggregation_module => otel_aggregation_sum}), + {ok, ViewUnitNotMatch} = otel_view:new(#{instrument_name => CounterName, instrument_unit => not_matching}, #{aggregation_module => otel_aggregation_sum}), ?assertMatch([{undefined, _}], otel_view:match_instrument_to_views(Counter, [ViewUnitNotMatch])), %% views require a unique name @@ -459,6 +459,36 @@ view_creation_test(_Config) -> ok. +wildcard_view(_Config) -> + Meter = opentelemetry_experimental:get_meter(), + + ViewCriteria = #{instrument_name => '*'}, + ViewConfig = #{aggregation_module => otel_aggregation_drop}, + + ?assert(otel_meter_server:add_view(ViewCriteria, ViewConfig)), + + CounterName = a_counter, + CounterDesc = <<"counter description">>, + CounterUnit = kb, + + Counter = otel_counter:create(Meter, CounterName, + #{description => CounterDesc, + unit => CounterUnit}), + + ?assertEqual(ok, otel_counter:add(Counter, 1, #{})), + + otel_meter_server:force_flush(), + + ?assertNotReceive(CounterName, CounterDesc, CounterUnit), + + {ok, View} = otel_view:new(ViewCriteria, ViewConfig), + ?assertMatch([{#view{}, _}], otel_view:match_instrument_to_views(Counter, [View])), + + %% not possible to create wildcard views with a name + error = otel_view:new(view_name, ViewCriteria, ViewConfig), + + ok. + counter_add(_Config) -> Meter = opentelemetry_experimental:get_meter(), @@ -825,7 +855,7 @@ kill_server(_Config) -> otel_meter_server:force_flush(), %% at this time a crashed meter server will mean losing the recorded metrics up to that point - ?assertNotReceive(a_counter, <<"counter description">>, kb), + ?assertSumReceive(a_counter, <<"counter description">>, kb, []), ?assertSumReceive(z_counter, <<"counter description">>, kb, [{9, #{<<"c">> => <<"b">>}}]), ok.