diff --git a/apps/opentelemetry/test/opentelemetry_SUITE.erl b/apps/opentelemetry/test/opentelemetry_SUITE.erl index 87bcb1739..b2a7c2c39 100644 --- a/apps/opentelemetry/test/opentelemetry_SUITE.erl +++ b/apps/opentelemetry/test/opentelemetry_SUITE.erl @@ -214,21 +214,27 @@ propagation(Config) -> ?assertMatch(#span_ctx{trace_flags=1}, ?current_span_ctx), ?assertMatch(#span_ctx{is_recording=true}, ?current_span_ctx), - otel_baggage:set(<<"key-1">>, <<"value=1">>), - otel_baggage:set(<<"key-2">>, <<"value-2">>), + + otel_baggage:set(<<"key-1">>, <<"value=1">>, []), + %% TODO: should the whole baggage entry be dropped if metadata is bad? + %% drop bad metadata (the `1'). + otel_baggage:set(<<"key-2">>, <<"value-2">>, [<<"metadata">>, 1]), + %% drop baggage with bad value + otel_baggage:set(<<"key-3">>, value3), + Headers = otel_propagator:text_map_inject([{<<"existing-header">>, <<"I exist">>}]), EncodedTraceId = io_lib:format("~32.16.0b", [TraceId]), EncodedSpanId = io_lib:format("~16.16.0b", [SpanId]), - ?assertListsEqual([{<<"baggage">>, <<"key-2=value-2,key-1=value%3D1">>}, + ?assertListsEqual([{<<"baggage">>, <<"key-2=value-2;metadata,key-1=value%3D1">>}, {<<"existing-header">>, <<"I exist">>} | trace_context(Propagator, EncodedTraceId, EncodedSpanId)], Headers), ?end_span(SpanCtx), - ?assertEqual(#{<<"key-1">> => <<"value=1">>, - <<"key-2">> => <<"value-2">>}, otel_baggage:get_all()), + ?assertEqual(#{<<"key-1">> => {<<"value=1">>, []}, + <<"key-2">> => {<<"value-2">>, [<<"metadata">>]}}, otel_baggage:get_all()), %% ?end_span doesn't remove the span from the context ?assertEqual(SpanCtx, ?current_span_ctx), @@ -243,8 +249,8 @@ propagation(Config) -> BinaryHeaders = [{string:uppercase(Key), iolist_to_binary(Value)} || {Key, Value} <- Headers], otel_propagator:text_map_extract(BinaryHeaders), - ?assertEqual(#{<<"key-1">> => <<"value=1">>, - <<"key-2">> => <<"value-2">>}, otel_baggage:get_all()), + ?assertEqual(#{<<"key-1">> => {<<"value=1">>, []}, + <<"key-2">> => {<<"value-2">>, [<<"metadata">>]}}, otel_baggage:get_all()), %% extracted remote spans are set to the active span %% but with `is_recording' false diff --git a/apps/opentelemetry_api/lib/open_telemetry/baggage.ex b/apps/opentelemetry_api/lib/open_telemetry/baggage.ex index 60470af40..877d183bd 100644 --- a/apps/opentelemetry_api/lib/open_telemetry/baggage.ex +++ b/apps/opentelemetry_api/lib/open_telemetry/baggage.ex @@ -7,7 +7,8 @@ defmodule OpenTelemetry.Baggage do defdelegate set(keyvalues), to: :otel_baggage defdelegate set(ctx_or_key, keyvalues), to: :otel_baggage - defdelegate set(ctx, key, values), to: :otel_baggage + defdelegate set(ctx, key, value), to: :otel_baggage + defdelegate set(ctx, key, values, metadata), to: :otel_baggage defdelegate get_all(), to: :otel_baggage defdelegate get_all(ctx), to: :otel_baggage defdelegate clear(), to: :otel_baggage diff --git a/apps/opentelemetry_api/src/otel_baggage.erl b/apps/opentelemetry_api/src/otel_baggage.erl index ab89012f3..a064ec0b5 100644 --- a/apps/opentelemetry_api/src/otel_baggage.erl +++ b/apps/opentelemetry_api/src/otel_baggage.erl @@ -22,6 +22,7 @@ -export([set/1, set/2, set/3, + set/4, get_all/0, get_all/1, clear/0, @@ -31,8 +32,9 @@ %% keys and values are UTF-8 binaries -type key() :: unicode:unicode_binary(). -type value() :: unicode:unicode_binary(). +-type metadata() :: [unicode:unicode_binary() | {unicode:unicode_binary(), unicode:unicode_binary()}]. --type t() :: #{key() => value()}. +-type t() :: #{key() => {value(), metadata()}}. -export_type([t/0, key/0, @@ -57,23 +59,31 @@ set(KeyValues) when is_list(KeyValues) -> set(maps:from_list(KeyValues)); set(KeyValues) when is_map(KeyValues) -> Baggage = otel_ctx:get_value(?BAGGAGE_KEY, #{}), - otel_ctx:set_value(?BAGGAGE_KEY, maps:merge(Baggage, KeyValues)). + otel_ctx:set_value(?BAGGAGE_KEY, maps:merge(Baggage, verify_baggage(KeyValues))); +set(_) -> + ok. %% Ctx will never be a list or binary so we can tell if a context is passed by checking that -spec set(otel_ctx:t() | key(), #{key() => value()} | [{key(), value()}] | value()) -> otel_ctx:t(). set(Key, Value) when is_list(Key) ; is_binary(Key) -> - Baggage = otel_ctx:get_value(?BAGGAGE_KEY, #{}), - otel_ctx:set_value(?BAGGAGE_KEY, Baggage#{Key => Value}); + set(Key, Value, []); set(Ctx, KeyValues) when is_list(KeyValues) -> set(Ctx, maps:from_list(KeyValues)); set(Ctx, KeyValues) when is_map(KeyValues) -> Baggage = otel_ctx:get_value(Ctx, ?BAGGAGE_KEY, #{}), - otel_ctx:set_value(Ctx, ?BAGGAGE_KEY, maps:merge(Baggage, KeyValues)). + otel_ctx:set_value(Ctx, ?BAGGAGE_KEY, maps:merge(Baggage, verify_baggage(KeyValues))). --spec set(otel_ctx:t(), key(), value()) -> otel_ctx:t(). +-spec set(otel_ctx:t(), key(), value()) -> ok | otel_ctx:t(). +set(Key, Value, Metadata) when is_list(Key) ; is_binary(Key) -> + Baggage = otel_ctx:get_value(?BAGGAGE_KEY, #{}), + otel_ctx:set_value(?BAGGAGE_KEY, maps:merge(Baggage, verify_baggage(#{Key => {Value, Metadata}}))); set(Ctx, Key, Value) -> + set(Ctx, Key, Value, []). + +-spec set(otel_ctx:t(), key(), value(), metadata()) -> otel_ctx:t(). +set(Ctx, Key, Value, Metadata) -> Baggage = otel_ctx:get_value(Ctx, ?BAGGAGE_KEY, #{}), - otel_ctx:set_value(Ctx, ?BAGGAGE_KEY, Baggage#{Key => Value}). + otel_ctx:set_value(Ctx, ?BAGGAGE_KEY, maps:merge(Baggage, verify_baggage(#{Key => {Value, Metadata}}))). -spec get_all() -> t(). get_all() -> @@ -122,6 +132,34 @@ get_text_map_propagators() -> Extract = otel_ctx:text_map_extractor(?BAGGAGE_KEY, FromText), {Extract, Inject}. +%% + +verify_baggage(KeyValues) -> + maps:fold(fun(K, V, Acc) -> + %% TODO: filter out bad keys here + case update_metadata(V) of + {true, ValueMetadata} -> + Acc#{K => ValueMetadata}; + _ -> + Acc + end + end, #{}, KeyValues). + +update_metadata({Value, Metadata}) when (is_binary(Value) orelse is_list(Value)) andalso is_list(Metadata) -> + {true, {Value, lists:filtermap(fun verify_metadata/1, Metadata)}}; +update_metadata(Value) when is_binary(Value) ; is_list(Value) -> + {true, {Value, []}}; +update_metadata(_) -> + false. + + +verify_metadata({MK, MV}) when is_binary(MK) , is_binary(MV) -> + true; +verify_metadata(M) when is_binary(M) -> + true; +verify_metadata(_) -> + false. + %% find a header in a list, ignoring case lookup(_, []) -> undefined; @@ -136,14 +174,41 @@ lookup(Header, [{H, Value} | Rest]) -> encode_key(Key) -> form_urlencode(Key, [{encoding, utf8}]). -encode_value(Value) -> - form_urlencode(Value, [{encoding, utf8}]). +encode_value({Value, Metadata}) -> + EncodedMetadata = encode_metadata(Metadata), + EncodedValue = form_urlencode(Value, [{encoding, utf8}]), + unicode:characters_to_binary(lists:join(<<";">>, [EncodedValue | EncodedMetadata])). + +encode_metadata(Metadata) when is_list(Metadata) -> + lists:filtermap(fun({MK, MV}) when is_binary(MK) , is_binary(MV) -> + {true, [MK, <<"=">>, MV]}; + (M) when is_binary(M) -> + {true, M}; + (_) -> + false + end, Metadata); +encode_metadata(_) -> + []. decode_key(Key) -> percent_decode(string:trim(unicode:characters_to_binary(Key))). -decode_value(Value) -> - percent_decode(string:trim(unicode:characters_to_binary(Value))). +decode_value(ValueAndMetadata) -> + [Value | MetadataList] = string:lexemes(ValueAndMetadata, [$;]), + {string_decode(Value), lists:filtermap(fun metadata_decode/1, MetadataList)}. + +metadata_decode(Metadata) -> + case string:split(Metadata, "=") of + [MetadataKey] -> + {true, string_decode(MetadataKey)}; + [MetadataKey, MetadataValue] -> + {true, {string_decode(MetadataKey), string_decode(MetadataValue)}}; + _ -> + false + end. + +string_decode(S) -> + percent_decode(string:trim(unicode:characters_to_binary(S))). %% TODO: call `uri_string:percent_decode' and remove this when OTP-23 is %% the oldest version we maintain support for