From 6ba380b70d601292192fc2345491094fe9d109ab Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Wed, 30 Aug 2023 04:40:57 -0600 Subject: [PATCH 1/3] move attribute processing functions to otel_attributes module --- apps/opentelemetry_api/src/opentelemetry.erl | 10 +-- .../opentelemetry_api/src/otel_attributes.erl | 65 ++++++++++++++++- apps/opentelemetry_api/src/otel_span.erl | 71 ++----------------- .../test/opentelemetry_api_SUITE.erl | 4 +- 4 files changed, 76 insertions(+), 74 deletions(-) diff --git a/apps/opentelemetry_api/src/opentelemetry.erl b/apps/opentelemetry_api/src/opentelemetry.erl index a7513f52..d7a48e2f 100644 --- a/apps/opentelemetry_api/src/opentelemetry.erl +++ b/apps/opentelemetry_api/src/opentelemetry.erl @@ -340,12 +340,12 @@ convert_timestamp(Timestamp, Unit) -> links(List) when is_list(List) -> lists:filtermap(fun({TraceId, SpanId, Attributes, TraceState}) when is_integer(TraceId) , is_integer(SpanId) -> - link_or_false(TraceId, SpanId, otel_span:process_attributes(Attributes), TraceState); + link_or_false(TraceId, SpanId, otel_attributes:process_attributes(Attributes), TraceState); ({#span_ctx{trace_id=TraceId, span_id=SpanId, tracestate=TraceState}, Attributes}) when is_integer(TraceId) , is_integer(SpanId) -> - link_or_false(TraceId, SpanId, otel_span:process_attributes(Attributes), TraceState); + link_or_false(TraceId, SpanId, otel_attributes:process_attributes(Attributes), TraceState); (#span_ctx{trace_id=TraceId, span_id=SpanId, tracestate=TraceState}) when is_integer(TraceId) , @@ -365,7 +365,7 @@ link(SpanCtx) -> link(#span_ctx{trace_id=TraceId, span_id=SpanId, tracestate=TraceState}, Attributes) -> - ?MODULE:link(TraceId, SpanId, otel_span:process_attributes(Attributes), TraceState); + ?MODULE:link(TraceId, SpanId, otel_attributes:process_attributes(Attributes), TraceState); link(_, _) -> undefined. @@ -379,7 +379,7 @@ link(TraceId, SpanId, Attributes, TraceState) when is_integer(TraceId), (is_list(Attributes) orelse is_map(Attributes)) -> #{trace_id => TraceId, span_id => SpanId, - attributes => otel_span:process_attributes(Attributes), + attributes => otel_attributes:process_attributes(Attributes), tracestate => TraceState}; link(_, _, _, _) -> undefined. @@ -401,7 +401,7 @@ event(Timestamp, Name, Attributes) when is_integer(Timestamp), true -> #{system_time_native => Timestamp, name => Name, - attributes => otel_span:process_attributes(Attributes)}; + attributes => otel_attributes:process_attributes(Attributes)}; false -> undefined end; diff --git a/apps/opentelemetry_api/src/otel_attributes.erl b/apps/opentelemetry_api/src/otel_attributes.erl index e0a520d9..c5dd21d2 100644 --- a/apps/opentelemetry_api/src/otel_attributes.erl +++ b/apps/opentelemetry_api/src/otel_attributes.erl @@ -21,8 +21,16 @@ set/2, set/3, dropped/1, - map/1]). + map/1, + is_valid_attribute/2, + process_attributes/1]). +-define(is_allowed_key(Key), (is_atom(Key) orelse (is_binary(Key) andalso Key =/= <<"">>))). +-define(is_allowed_value(Value), (is_atom(Value) orelse + is_boolean(Value) orelse + is_number(Value) orelse + is_binary(Value) orelse + is_list(Value))). -record(attributes, { count_limit :: integer(), value_length_limit :: integer() | infinity, @@ -106,3 +114,58 @@ maybe_truncate_binary(Value, ValueLengthLimit) -> false -> Value end. + +-spec is_valid_attribute(opentelemetry:attribute_key(), opentelemetry:attribute_value()) -> boolean(). +is_valid_attribute(Key, Value) when is_tuple(Value) , ?is_allowed_key(Key) -> + is_valid_attribute(Key, tuple_to_list(Value)); +%% lists as attribute values must be primitive types and homogeneous +is_valid_attribute(Key, [Value1 | _Rest] = Values) when is_binary(Value1) , ?is_allowed_key(Key) -> + lists:all(fun is_binary/1, Values); +is_valid_attribute(Key, [Value1 | _Rest] = Values) when is_boolean(Value1) , ?is_allowed_key(Key) -> + lists:all(fun is_boolean/1, Values); +is_valid_attribute(Key, [Value1 | _Rest] = Values) when is_atom(Value1) , ?is_allowed_key(Key) -> + lists:all(fun is_valid_atom_value/1, Values); +is_valid_attribute(Key, [Value1 | _Rest] = Values) when is_integer(Value1) , ?is_allowed_key(Key) -> + lists:all(fun is_integer/1, Values); +is_valid_attribute(Key, [Value1 | _Rest] = Values) when is_float(Value1) , ?is_allowed_key(Key) -> + lists:all(fun is_float/1, Values); +is_valid_attribute(_Key, Value) when is_list(Value) -> + false; +is_valid_attribute(Key, []) when ?is_allowed_key(Key) -> + true; +is_valid_attribute(Key, Value) when ?is_allowed_key(Key) , ?is_allowed_value(Value) -> + true; +is_valid_attribute(_, _) -> + false. + +is_valid_atom_value(undefined) -> + false; +is_valid_atom_value(nil) -> + false; +is_valid_atom_value(Value) -> + is_atom(Value) andalso (is_boolean(Value) == false). + +-spec process_attributes(eqwalizer:dynamic()) -> opentelemetry:attributes_map(). +process_attributes(Attributes) when is_map(Attributes) -> + maps:fold(fun process_attribute/3, #{}, Attributes); +process_attributes([]) -> #{}; +process_attributes(Attributes) when is_list(Attributes) -> + process_attributes(maps:from_list(Attributes)); +process_attributes(_) -> + #{}. + +process_attribute(Key, Value, Map) when is_tuple(Value) -> + List = tuple_to_list(Value), + case is_valid_attribute(Key, List) of + true -> + maps:put(Key, Value, Map); + false -> + Map + end; +process_attribute(Key, Value, Map) -> + case is_valid_attribute(Key, Value) of + true -> + maps:put(Key, Value, Map); + false -> + Map + end. diff --git a/apps/opentelemetry_api/src/otel_span.erl b/apps/opentelemetry_api/src/otel_span.erl index 1210ac20..5b3a3c4a 100644 --- a/apps/opentelemetry_api/src/otel_span.erl +++ b/apps/opentelemetry_api/src/otel_span.erl @@ -27,7 +27,6 @@ is_recording/1, is_valid/1, is_valid_name/1, - process_attributes/1, validate_start_opts/1, set_attribute/3, set_attributes/2, @@ -45,12 +44,6 @@ -include_lib("opentelemetry_semantic_conventions/include/trace.hrl"). -define(is_recording(SpanCtx), SpanCtx =/= undefined andalso SpanCtx#span_ctx.is_recording =:= true). --define(is_allowed_key(Key), is_atom(Key) orelse (is_binary(Key) andalso Key =/= <<"">>)). --define(is_allowed_value(Value), is_atom(Value) orelse - is_boolean(Value) orelse - is_number(Value) orelse - is_binary(Value) orelse - is_list(Value)). -type start_opts() :: #{attributes := opentelemetry:attributes_map(), links := [opentelemetry:link()], @@ -68,7 +61,7 @@ validate_start_opts(Opts) when is_map(Opts) -> StartTime = maps:get(start_time, Opts, opentelemetry:timestamp()), IsRecording = maps:get(is_recording, Opts, true), #{ - attributes => process_attributes(Attributes), + attributes => otel_attributes:process_attributes(Attributes), links => Links, kind => Kind, start_time => StartTime, @@ -89,60 +82,6 @@ is_valid(#span_ctx{trace_id=TraceId, is_valid(_) -> false. --spec is_valid_attribute(opentelemetry:attribute_key(), opentelemetry:attribute_value()) -> boolean(). -is_valid_attribute(Key, Value) when is_tuple(Value) , ?is_allowed_key(Key) -> - is_valid_attribute(Key, tuple_to_list(Value)); -%% lists as attribute values must be primitive types and homogeneous -is_valid_attribute(Key, [Value1 | _Rest] = Values) when is_binary(Value1) , ?is_allowed_key(Key) -> - lists:all(fun is_binary/1, Values); -is_valid_attribute(Key, [Value1 | _Rest] = Values) when is_boolean(Value1) , ?is_allowed_key(Key) -> - lists:all(fun is_boolean/1, Values); -is_valid_attribute(Key, [Value1 | _Rest] = Values) when is_atom(Value1) , ?is_allowed_key(Key) -> - lists:all(fun is_valid_atom_value/1, Values); -is_valid_attribute(Key, [Value1 | _Rest] = Values) when is_integer(Value1) , ?is_allowed_key(Key) -> - lists:all(fun is_integer/1, Values); -is_valid_attribute(Key, [Value1 | _Rest] = Values) when is_float(Value1) , ?is_allowed_key(Key) -> - lists:all(fun is_float/1, Values); -is_valid_attribute(_Key, Value) when is_list(Value) -> - false; -is_valid_attribute(Key, []) when ?is_allowed_key(Key) -> - true; -is_valid_attribute(Key, Value) when ?is_allowed_key(Key) , ?is_allowed_value(Value) -> - true; -is_valid_attribute(_, _) -> - false. - -is_valid_atom_value(undefined) -> - false; -is_valid_atom_value(nil) -> - false; -is_valid_atom_value(Value) -> - is_atom(Value) andalso (is_boolean(Value) == false). - --spec process_attributes(eqwalizer:dynamic()) -> opentelemetry:attributes_map(). -process_attributes(Attributes) when is_map(Attributes) -> - maps:fold(fun process_attribute/3, #{}, Attributes); -process_attributes([]) -> #{}; -process_attributes(Attributes) when is_list(Attributes) -> - process_attributes(maps:from_list(Attributes)); -process_attributes(_) -> - #{}. - -process_attribute(Key, Value, Map) when is_tuple(Value) -> - List = tuple_to_list(Value), - case is_valid_attribute(Key, List) of - true -> - maps:put(Key, Value, Map); - false -> - Map - end; -process_attribute(Key, Value, Map) -> - case is_valid_attribute(Key, Value) of - true -> - maps:put(Key, Value, Map); - false -> - Map - end. -spec is_valid_name(any()) -> boolean(). is_valid_name(undefined) -> false; @@ -203,14 +142,14 @@ tracestate(_) -> SpanCtx :: opentelemetry:span_ctx(). set_attribute(SpanCtx=#span_ctx{span_sdk={Module, _}}, Key, Value) when ?is_recording(SpanCtx) , is_tuple(Value) -> List = tuple_to_list(Value), - case is_valid_attribute(Key, List) of + case otel_attributes:is_valid_attribute(Key, List) of true -> Module:set_attribute(SpanCtx, Key, List); false -> false end; set_attribute(SpanCtx=#span_ctx{span_sdk={Module, _}}, Key, Value) when ?is_recording(SpanCtx) -> - case is_valid_attribute(Key, Value) of + case otel_attributes:is_valid_attribute(Key, Value) of true -> Module:set_attribute(SpanCtx, Key, Value); false -> @@ -224,7 +163,7 @@ set_attribute(_, _, _) -> SpanCtx :: opentelemetry:span_ctx(). set_attributes(SpanCtx=#span_ctx{span_sdk={Module, _}}, Attributes) when ?is_recording(SpanCtx), (is_list(Attributes) orelse is_map(Attributes)) -> - Module:set_attributes(SpanCtx, process_attributes(Attributes)); + Module:set_attributes(SpanCtx, otel_attributes:process_attributes(Attributes)); set_attributes(_, _) -> false. @@ -237,7 +176,7 @@ add_event(SpanCtx=#span_ctx{span_sdk={Module, _}}, Name, Attributes) (is_list(Attributes) orelse is_map(Attributes)) -> case is_valid_name(Name) of true -> - Module:add_event(SpanCtx, Name, process_attributes(Attributes)); + Module:add_event(SpanCtx, Name, otel_attributes:process_attributes(Attributes)); false -> false end; diff --git a/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl b/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl index 65108e0a..5a6fb9ed 100644 --- a/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl +++ b/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl @@ -76,7 +76,7 @@ can_create_link_from_span(_Config) -> validations(_Config) -> InvalidAttributesArg = undefined, - ?assertMatch(#{}, otel_span:process_attributes(InvalidAttributesArg)), + ?assertMatch(#{}, otel_attributes:process_attributes(InvalidAttributesArg)), Attributes = [ {<<"key-1">>, <<"value-1">>}, @@ -102,7 +102,7 @@ validations(_Config) -> {untimed_event, Attributes}, {<<"">>, Attributes}, {123, Attributes}], - ProcessedAttributes = otel_span:process_attributes(Attributes), + ProcessedAttributes = otel_attributes:process_attributes(Attributes), ?assertMatch(#{key2 := 1, key3 := true, From 74c07f8de34e650ce5536e78b01e94e39d366f7c Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Wed, 30 Aug 2023 10:46:16 -0600 Subject: [PATCH 2/3] fix changelog entry for moving otel_attributes to the api --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50c1db91..d2e504bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,14 +14,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [Add `otel_tracestate` module for creating and updating tracestate](https://github.com/open-telemetry/opentelemetry-erlang/pull/607) - [Attributes module `otel_attributes` moved to - SDK](https://github.com/open-telemetry/opentelemetry-erlang/pull/618) + API](https://github.com/open-telemetry/opentelemetry-erlang/pull/618) ## SDK ### Changes - [Attributes module `otel_attributes` moved to - SDK](https://github.com/open-telemetry/opentelemetry-erlang/pull/618) + API](https://github.com/open-telemetry/opentelemetry-erlang/pull/618) ## Experimental API From 69573438a331f9e409d75fbb5da5e0428ac8403a Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Wed, 30 Aug 2023 10:53:04 -0600 Subject: [PATCH 3/3] add changelog entry for moving attribute processing to otel_attributes --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2e504bb..d0cd8b70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 tracestate](https://github.com/open-telemetry/opentelemetry-erlang/pull/607) - [Attributes module `otel_attributes` moved to API](https://github.com/open-telemetry/opentelemetry-erlang/pull/618) +- [Moved attribute processing functions to `otel_attributes` from + `otel_span`](https://github.com/open-telemetry/opentelemetry-erlang/pull/620) ## SDK