From 9e20f343373f01aa7fb600c4a1d0d8c837a3b553 Mon Sep 17 00:00:00 2001 From: Alberto Sartori Date: Fri, 6 Oct 2023 17:12:54 +0200 Subject: [PATCH] Correctly handle attributes span limits --- apps/opentelemetry/include/otel_span.hrl | 14 ++++--- apps/opentelemetry/src/otel_configuration.erl | 14 ++++--- apps/opentelemetry/src/otel_limits.erl | 20 ++++++++++ apps/opentelemetry/src/otel_span_utils.erl | 4 +- .../test/opentelemetry_SUITE.erl | 4 +- .../test/otel_configuration_SUITE.erl | 40 ++++++++++++------- .../src/otel_meter_server.erl | 4 +- .../src/otel_observables.erl | 4 +- .../test/otel_metrics_SUITE.erl | 34 +++++++++++++++- 9 files changed, 105 insertions(+), 33 deletions(-) diff --git a/apps/opentelemetry/include/otel_span.hrl b/apps/opentelemetry/include/otel_span.hrl index c28f3efe..43fa3b39 100644 --- a/apps/opentelemetry/include/otel_span.hrl +++ b/apps/opentelemetry/include/otel_span.hrl @@ -63,12 +63,14 @@ }). -record(limits, { - attribute_count_limit = 128 :: integer(), %% Maximum allowed attribute count per span; - attribute_value_length_limit = infinity :: integer() | infinity, %% Maximum allowed attribute value length - event_count_limit = 128 :: integer(), %% Maximum allowed span event count - link_count_limit = 128 :: integer(), %% Maximum allowed span link count - attribute_per_event_limit = 128 :: integer(), %% Maximum allowed attribute per span event count - attribute_per_link_limit = 128 :: integer() %% Maximum allowed attribute per span link count + attribute_count_limit = 128 :: integer(), %% Maximum allowed attribute count; + attribute_value_length_limit = infinity :: integer() | infinity, %% Maximum allowed attribute value length + span_attribute_count_limit = undefined :: integer() | undefined, %% Maximum allowed attribute count per span; + span_attribute_value_length_limit = undefined :: integer() | infinity | undefined, %% Maximum allowed attribute value length on span attributes + event_count_limit = 128 :: integer(), %% Maximum allowed span event count + link_count_limit = 128 :: integer(), %% Maximum allowed span link count + attribute_per_event_limit = 128 :: integer(), %% Maximum allowed attribute per span event count + attribute_per_link_limit = 128 :: integer() %% Maximum allowed attribute per span link count }). -record(link, { diff --git a/apps/opentelemetry/src/otel_configuration.erl b/apps/opentelemetry/src/otel_configuration.erl index dd9f793c..96396720 100644 --- a/apps/opentelemetry/src/otel_configuration.erl +++ b/apps/opentelemetry/src/otel_configuration.erl @@ -59,6 +59,8 @@ storage_size => integer() | infinity}, attribute_count_limit := integer(), attribute_value_length_limit := integer() | infinity, + span_attribute_count_limit := integer(), + span_attribute_value_length_limit := integer() | infinity, event_count_limit := integer(), link_count_limit := integer(), attribute_per_event_limit := integer(), @@ -97,6 +99,8 @@ new() -> storage_size => infinity}, attribute_count_limit => 128, attribute_value_length_limit => infinity, + span_attribute_count_limit => undefined, + span_attribute_value_length_limit => undefined, event_count_limit => 128, link_count_limit => 128, attribute_per_event_limit => 128, @@ -323,14 +327,14 @@ config_mappings(general_sdk) -> {"OTEL_SSP_EXPORT_TIMEOUT_MILLIS", ssp_exporting_timeout_ms, integer} ]; config_mappings(limits) -> - [{"OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT", attribute_count_limit, integer}, - {"OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT", attribute_value_length_limit, integer_infinity}, + [{"OTEL_ATTRIBUTE_COUNT_LIMIT", attribute_count_limit, integer}, + {"OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT", attribute_value_length_limit, integer_infinity}, + {"OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT", span_attribute_count_limit, integer}, + {"OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT", span_attribute_value_length_limit, integer_infinity}, {"OTEL_SPAN_EVENT_COUNT_LIMIT", event_count_limit, integer}, {"OTEL_SPAN_LINK_COUNT_LIMIT", link_count_limit, integer}, {"OTEL_EVENT_ATTRIBUTE_COUNT_LIMIT", attribute_per_event_limit, integer}, - {"OTEL_LINK_ATTRIBUTE_COUNT_LIMIT", attribute_per_link_limit, integer}%% , - %% {"OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT", attribute_value_length_limit, integer}, - %% {"OTEL_ATTRIBUTE_COUNT_LIMIT", attribute_per_link_limit, integer} + {"OTEL_LINK_ATTRIBUTE_COUNT_LIMIT", attribute_per_link_limit, integer} ]; config_mappings(sweeper) -> [{"OTEL_SPAN_SWEEPER_INTERVAL", interval, integer_infinity}, diff --git a/apps/opentelemetry/src/otel_limits.erl b/apps/opentelemetry/src/otel_limits.erl index 08aace99..50043d3b 100644 --- a/apps/opentelemetry/src/otel_limits.erl +++ b/apps/opentelemetry/src/otel_limits.erl @@ -22,6 +22,8 @@ set/1, attribute_count_limit/0, attribute_value_length_limit/0, + span_attribute_count_limit/0, + span_attribute_value_length_limit/0, event_count_limit/0, link_count_limit/0, attribute_per_event_limit/0, @@ -38,12 +40,16 @@ get() -> -spec set(otel_configuration:t()) -> ok. set(#{attribute_count_limit := AttributeCountLimit, attribute_value_length_limit := AttributeValueLengthLimit, + span_attribute_count_limit := SpanAttributeCountLimit, + span_attribute_value_length_limit := SpanAttributeValueLengthLimit, event_count_limit := EventCountLimit, link_count_limit := LinkCountLimit, attribute_per_event_limit := AttributePerEventLimit, attribute_per_link_limit := AttributePerLinkLimit}) -> Limits = #limits{attribute_count_limit=AttributeCountLimit, attribute_value_length_limit=AttributeValueLengthLimit, + span_attribute_count_limit=SpanAttributeCountLimit, + span_attribute_value_length_limit=SpanAttributeValueLengthLimit, event_count_limit=EventCountLimit, link_count_limit=LinkCountLimit, attribute_per_event_limit=AttributePerEventLimit, @@ -56,6 +62,12 @@ attribute_count_limit() -> attribute_value_length_limit() -> get_limit(attribute_value_length_limit, ?MODULE:get()). +span_attribute_count_limit() -> + get_limit(span_attribute_count_limit, ?MODULE:get()). + +span_attribute_value_length_limit() -> + get_limit(span_attribute_value_length_limit, ?MODULE:get()). + event_count_limit() -> get_limit(event_count_limit, ?MODULE:get()). @@ -72,6 +84,14 @@ get_limit(attribute_count_limit, #limits{attribute_count_limit=AttributeCountLim AttributeCountLimit; get_limit(attribute_value_length_limit, #limits{attribute_value_length_limit=AttributeValueLengthLimit}) -> AttributeValueLengthLimit; +get_limit(span_attribute_count_limit, #limits{span_attribute_count_limit=undefined, attribute_count_limit=AttributeCountLimit}) -> + AttributeCountLimit; +get_limit(span_attribute_count_limit, #limits{span_attribute_count_limit=SpanAttributeCountLimit}) -> + SpanAttributeCountLimit; +get_limit(span_attribute_value_length_limit, #limits{span_attribute_value_length_limit=undefined, attribute_value_length_limit=AttributeValueLengthLimit}) -> + AttributeValueLengthLimit; +get_limit(span_attribute_value_length_limit, #limits{span_attribute_value_length_limit=SpanAttributeValueLengthLimit}) -> + SpanAttributeValueLengthLimit; get_limit(event_count_limit, #limits{event_count_limit=EventCountLimit}) -> EventCountLimit; get_limit(link_count_limit, #limits{link_count_limit=LinkCountLimit}) -> diff --git a/apps/opentelemetry/src/otel_span_utils.erl b/apps/opentelemetry/src/otel_span_utils.erl index 210d92d1..f0a1f011 100644 --- a/apps/opentelemetry/src/otel_span_utils.erl +++ b/apps/opentelemetry/src/otel_span_utils.erl @@ -29,8 +29,8 @@ -spec start_span(otel_ctx:t(), opentelemetry:span_name(), otel_sampler:t(), otel_id_generator:t(), otel_span:start_opts()) -> {opentelemetry:span_ctx(), opentelemetry:span() | undefined}. start_span(Ctx, Name, Sampler, IdGenerator, Opts) -> - SpanAttributeCountLimit = otel_limits:attribute_count_limit(), - SpanAttributeValueLengthLimit= otel_limits:attribute_value_length_limit(), + SpanAttributeCountLimit = otel_limits:span_attribute_count_limit(), + SpanAttributeValueLengthLimit = otel_limits:span_attribute_value_length_limit(), EventCountLimit = otel_limits:event_count_limit(), LinkCountLimit = otel_limits:link_count_limit(), AttributePerEventLimit = otel_limits:attribute_per_event_limit(), diff --git a/apps/opentelemetry/test/opentelemetry_SUITE.erl b/apps/opentelemetry/test/opentelemetry_SUITE.erl index c41db277..d45b7180 100644 --- a/apps/opentelemetry/test/opentelemetry_SUITE.erl +++ b/apps/opentelemetry/test/opentelemetry_SUITE.erl @@ -115,7 +115,9 @@ init_per_testcase(dropped_attributes, Config) -> Config1; init_per_testcase(too_many_attributes, Config) -> Config1 = set_batch_tab_processor(Config), - application:set_env(opentelemetry, attribute_count_limit, 2), + application:set_env(opentelemetry, attribute_count_limit, 5), + %% this will override the previous one + application:set_env(opentelemetry, span_attribute_count_limit, 2), {ok, _} = application:ensure_all_started(opentelemetry), Config1; init_per_testcase(tracer_instrumentation_scope, Config) -> diff --git a/apps/opentelemetry/test/otel_configuration_SUITE.erl b/apps/opentelemetry/test/otel_configuration_SUITE.erl index 9aa06f8b..19ae6153 100644 --- a/apps/opentelemetry/test/otel_configuration_SUITE.erl +++ b/apps/opentelemetry/test/otel_configuration_SUITE.erl @@ -140,7 +140,9 @@ init_per_testcase(resource_detectors, Config) -> [{os_vars, Vars} | Config]; init_per_testcase(limits, Config) -> - Vars = [{"OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT", "111"}, + Vars = [{"OTEL_ATTRIBUTE_COUNT_LIMIT", "123"}, + {"OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT", "4"}, + {"OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT", "111"}, {"OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT", "009"}, {"OTEL_SPAN_EVENT_COUNT_LIMIT", "200"}, {"OTEL_SPAN_LINK_COUNT_LIMIT", "1101"}, @@ -149,22 +151,28 @@ init_per_testcase(limits, Config) -> setup_env(Vars), - ExpectedOpts = #{attribute_count_limit => 111, - attribute_value_length_limit => 9, + ExpectedOpts = #{attribute_count_limit => 123, + attribute_value_length_limit => 4, + span_attribute_count_limit => 111, + span_attribute_value_length_limit => 9, event_count_limit => 200, link_count_limit => 1101, attribute_per_event_limit => 400, attribute_per_link_limit => 500}, - ExpectedRecord = #limits{attribute_count_limit=111, - attribute_value_length_limit=9, - event_count_limit=200, - link_count_limit=1101, - attribute_per_event_limit=400, - attribute_per_link_limit=500}, + ExpectedRecord = #limits{attribute_count_limit=123, + attribute_value_length_limit=4, + span_attribute_count_limit=111, + span_attribute_value_length_limit=9, + event_count_limit=200, + link_count_limit=1101, + attribute_per_event_limit=400, + attribute_per_link_limit=500}, [{expected_opts, ExpectedOpts}, {expected_record, ExpectedRecord}, {os_vars, Vars} | Config]; init_per_testcase(bad_limits, Config) -> - Vars = [{"OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT", "aaa"}, + Vars = [{"OTEL_ATTRIBUTE_COUNT_LIMIT", "foo"}, + {"OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT", "4a"}, + {"OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT", "aaa"}, {"OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT", "bbb"}, {"OTEL_SPAN_EVENT_COUNT_LIMIT", "1d4"}, {"OTEL_SPAN_LINK_COUNT_LIMIT", "eee"}, @@ -175,11 +183,13 @@ init_per_testcase(bad_limits, Config) -> ExpectedOpts = #{}, ExpectedRecord = #limits{attribute_count_limit=128, - attribute_value_length_limit=infinity, - event_count_limit=128, - link_count_limit=128, - attribute_per_event_limit=128, - attribute_per_link_limit=128}, + attribute_value_length_limit=infinity, + span_attribute_count_limit=undefined, + span_attribute_value_length_limit=undefined, + event_count_limit=128, + link_count_limit=128, + attribute_per_event_limit=128, + attribute_per_link_limit=128}, [{expected_opts, ExpectedOpts}, {expected_record, ExpectedRecord}, {os_vars, Vars} | Config]; init_per_testcase(bad_app_config, Config) -> diff --git a/apps/opentelemetry_experimental/src/otel_meter_server.erl b/apps/opentelemetry_experimental/src/otel_meter_server.erl index 5e11f04d..6b59e428 100644 --- a/apps/opentelemetry_experimental/src/otel_meter_server.erl +++ b/apps/opentelemetry_experimental/src/otel_meter_server.erl @@ -360,7 +360,9 @@ metric_reader(ReaderId, ReaderPid, DefaultAggregationMapping, Temporality) -> %% active metrics are indexed by the ViewAggregation name + the Measurement's Attributes handle_measurement(Meter, Name, Number, Attributes, ViewAggregationsTab, MetricsTab) -> - AttributesRecord = otel_attributes:new(Attributes, 128, infinity), + AttributeCountLimit = otel_limits:attribute_count_limit(), + AttributeValueLengthLimit = otel_limits:attribute_value_length_limit(), + AttributesRecord = otel_attributes:new(Attributes, AttributeCountLimit, AttributeValueLengthLimit), Matches = ets:match(ViewAggregationsTab, {{Meter, Name}, '$1'}), update_aggregations(Number, AttributesRecord, Matches, MetricsTab). diff --git a/apps/opentelemetry_experimental/src/otel_observables.erl b/apps/opentelemetry_experimental/src/otel_observables.erl index daa27148..55da1599 100644 --- a/apps/opentelemetry_experimental/src/otel_observables.erl +++ b/apps/opentelemetry_experimental/src/otel_observables.erl @@ -93,7 +93,9 @@ handle_instruments_observations(Results, _Instruments, _ViewAggregationTab, _Met handle_observations(_MetricsTab, _ViewAggregation, []) -> ok; handle_observations(MetricsTab, ViewAggregation, [{Number, Attributes} | Rest]) when is_number(Number), is_map(Attributes) -> - AttributesRecord = otel_attributes:new(Attributes, 128, infinity), + AttributeCountLimit = otel_limits:attribute_count_limit(), + AttributeValueLengthLimit = otel_limits:attribute_value_length_limit(), + AttributesRecord = otel_attributes:new(Attributes, AttributeCountLimit, AttributeValueLengthLimit), _ = otel_aggregation:maybe_init_aggregate(MetricsTab, ViewAggregation, Number, AttributesRecord), handle_observations(MetricsTab, ViewAggregation, Rest); handle_observations(MetricsTab, ViewAggregation, [Result | Rest]) -> diff --git a/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl b/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl index 85ff390d..9b12bcf8 100644 --- a/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl +++ b/apps/opentelemetry_experimental/test/otel_metrics_SUITE.erl @@ -83,7 +83,8 @@ all() -> kill_reader, kill_server, observable_counter, observable_updown_counter, observable_gauge, multi_instrument_callback, using_macros, float_counter, float_updown_counter, float_histogram, sync_filtered_attributes, async_filtered_attributes, delta_observable_counter, - bad_observable_return, default_resource, histogram_aggregation_options, advisory_params + bad_observable_return, default_resource, histogram_aggregation_options, advisory_params, + too_many_attributes ]. init_per_suite(Config) -> @@ -174,6 +175,12 @@ init_per_testcase(delta_observable_counter, Config) -> {ok, _} = application:ensure_all_started(opentelemetry_experimental), Config; +init_per_testcase(too_many_attributes, Config) -> + application:stop(opentelemetry), + application:unload(opentelemetry), + application:load(opentelemetry), + application:set_env(opentelemetry, attribute_count_limit, 2), + init_per_testcase(undefined, Config); init_per_testcase(_, Config) -> application:load(opentelemetry_experimental), ok = application:set_env(opentelemetry_experimental, readers, [#{module => otel_metric_reader, @@ -185,6 +192,10 @@ init_per_testcase(_, Config) -> Config. +end_per_testcase(too_many_attributes, Config) -> + ok = application:stop(opentelemetry), + application:unload(opentelemetry), + end_per_testcase(undefined, Config); end_per_testcase(_, _Config) -> ok = application:stop(opentelemetry_experimental), application:unload(opentelemetry_experimental), @@ -1278,4 +1289,23 @@ histogram_aggregation_options(_Config) -> after 1000 -> ct:fail(histogram_receive_timeout) - end. \ No newline at end of file + end. + +too_many_attributes(_Config) -> + CounterName = counter, + + Counter = ?create_counter(CounterName, #{}), + + ?assertEqual(ok, otel_counter:add(Counter, 2, #{<<"a">> => 1, <<"b">> => 2, <<"c">> => 3})), + ?assertEqual(ok, otel_counter:add(Counter, 5, #{<<"a">> => 4})), + + otel_meter_server:force_flush(), + + receive + {otel_metric, #metric{name=counter, data=#sum{datapoints=Datapoints}}} -> + Data = lists:sort([{MetricValue, otel_attributes:dropped(MetricAttributes)} || #datapoint{value=MetricValue, attributes=MetricAttributes} <- Datapoints]), + ?assertMatch([{2, 1}, {5, 0}], lists:sort(Data), Data) + after + 1000 -> + ct:fail(counter_receive_timeout) + end.