From 6f71e91570c267495e12a51007cc6db099fa3037 Mon Sep 17 00:00:00 2001 From: Aleksey Kashapov Date: Fri, 26 Jul 2024 16:22:22 +0300 Subject: [PATCH 1/3] TD-949: Fixes otel ctx attachment with sampled ctx --- apps/mg_core/src/mg_core_machine.erl | 2 +- apps/mg_core/src/mg_core_otel.erl | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/apps/mg_core/src/mg_core_machine.erl b/apps/mg_core/src/mg_core_machine.erl index decc391..9108ace 100644 --- a/apps/mg_core/src/mg_core_machine.erl +++ b/apps/mg_core/src/mg_core_machine.erl @@ -513,7 +513,7 @@ handle_load(ID, Options, ReqCtx) -> -spec handle_call(_Call, mg_core_worker:call_context(), maybe(request_context()), deadline(), state()) -> {{reply, _Resp} | noreply, state()}. handle_call(Call, CallContext, ReqCtx, Deadline, S) -> - %% FIXME Consider adding new pulse beats to wrap 'processing calls' here. + %% NOTE Consider adding new pulse beats to wrap 'processing calls' here. ok = attach_otel_ctx(ReqCtx), ParentSpanId = mg_core_otel:current_span_id(otel_ctx:get_current()), ?with_span(?SPAN_NAME(call), ?SPAN_OPTS, fun(SpanCtx) -> diff --git a/apps/mg_core/src/mg_core_otel.erl b/apps/mg_core/src/mg_core_otel.erl index 087422a..a81ecfc 100644 --- a/apps/mg_core/src/mg_core_otel.erl +++ b/apps/mg_core/src/mg_core_otel.erl @@ -67,12 +67,15 @@ maybe_attach_otel_ctx(NewCtx) -> _ = otel_ctx:attach(choose_viable_otel_ctx(NewCtx, otel_ctx:get_current())), ok. +%% lowest bit is if it is sampled +-define(SPAN_IS_SAMPLED(SpanCtx), SpanCtx#span_ctx.trace_flags band 2#1 =:= 1). + -spec choose_viable_otel_ctx(T, T) -> T when T :: otel_ctx:t(). choose_viable_otel_ctx(NewCtx, CurrentCtx) -> case {otel_tracer:current_span_ctx(NewCtx), otel_tracer:current_span_ctx(CurrentCtx)} of - %% Don't attach if new context is without recording span - %% and old context has span defined - {#span_ctx{is_recording = false}, #span_ctx{}} -> CurrentCtx; + %% Don't attach if new context is without sampled span and old + %% context has span defined + {SpanCtx = #span_ctx{}, #span_ctx{}} when ?SPAN_IS_SAMPLED(SpanCtx) -> CurrentCtx; {undefined, #span_ctx{}} -> CurrentCtx; {_, _} -> NewCtx end. From 0c463cd2268b6645b089b58d9368e24aa357c395 Mon Sep 17 00:00:00 2001 From: Aleksey Kashapov Date: Fri, 26 Jul 2024 16:37:23 +0300 Subject: [PATCH 2/3] Updates unit tests --- apps/mg_core/src/mg_core_otel.erl | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/apps/mg_core/src/mg_core_otel.erl b/apps/mg_core/src/mg_core_otel.erl index a81ecfc..36e15a5 100644 --- a/apps/mg_core/src/mg_core_otel.erl +++ b/apps/mg_core/src/mg_core_otel.erl @@ -68,14 +68,14 @@ maybe_attach_otel_ctx(NewCtx) -> ok. %% lowest bit is if it is sampled --define(SPAN_IS_SAMPLED(SpanCtx), SpanCtx#span_ctx.trace_flags band 2#1 =:= 1). +-define(IS_NOT_SAMPLED(SpanCtx), SpanCtx#span_ctx.trace_flags band 2#1 =/= 1). -spec choose_viable_otel_ctx(T, T) -> T when T :: otel_ctx:t(). choose_viable_otel_ctx(NewCtx, CurrentCtx) -> case {otel_tracer:current_span_ctx(NewCtx), otel_tracer:current_span_ctx(CurrentCtx)} of %% Don't attach if new context is without sampled span and old %% context has span defined - {SpanCtx = #span_ctx{}, #span_ctx{}} when ?SPAN_IS_SAMPLED(SpanCtx) -> CurrentCtx; + {SpanCtx = #span_ctx{}, #span_ctx{}} when ?IS_NOT_SAMPLED(SpanCtx) -> CurrentCtx; {undefined, #span_ctx{}} -> CurrentCtx; {_, _} -> NewCtx end. @@ -187,7 +187,7 @@ binary_to_id(Opaque) when is_binary(Opaque) -> -type testgen() :: {_ID, fun(() -> _)}. -spec test() -> _. --define(OTEL_CTX(IsRecording), +-define(OTEL_CTX(IsSampled), otel_tracer:set_current_span( otel_ctx:new(), (otel_tracer_noop:noop_span_ctx())#span_ctx{ @@ -195,7 +195,12 @@ binary_to_id(Opaque) when is_binary(Opaque) -> span_id = otel_id_generator:generate_span_id(), is_valid = true, is_remote = true, - is_recording = IsRecording + is_recording = false, + trace_flags = + case IsSampled of + true -> 1; + false -> 0 + end } ) ). From cd9974d4ca0e5845fe0a9fcd234ec6d91adc7ddc Mon Sep 17 00:00:00 2001 From: Aleksey Kashapov Date: Tue, 30 Jul 2024 10:32:49 +0300 Subject: [PATCH 3/3] Simplify test helper macro --- apps/mg_core/src/mg_core_otel.erl | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/apps/mg_core/src/mg_core_otel.erl b/apps/mg_core/src/mg_core_otel.erl index 36e15a5..aa69541 100644 --- a/apps/mg_core/src/mg_core_otel.erl +++ b/apps/mg_core/src/mg_core_otel.erl @@ -187,6 +187,8 @@ binary_to_id(Opaque) when is_binary(Opaque) -> -type testgen() :: {_ID, fun(() -> _)}. -spec test() -> _. +-define(IS_SAMPLED, 1). +-define(NOT_SAMPLED, 0). -define(OTEL_CTX(IsSampled), otel_tracer:set_current_span( otel_ctx:new(), @@ -196,19 +198,15 @@ binary_to_id(Opaque) when is_binary(Opaque) -> is_valid = true, is_remote = true, is_recording = false, - trace_flags = - case IsSampled of - true -> 1; - false -> 0 - end + trace_flags = IsSampled } ) ). -spec choose_viable_otel_ctx_test_() -> [testgen()]. choose_viable_otel_ctx_test_() -> - A = ?OTEL_CTX(true), - B = ?OTEL_CTX(false), + A = ?OTEL_CTX(?IS_SAMPLED), + B = ?OTEL_CTX(?NOT_SAMPLED), [ ?_assertEqual(A, choose_viable_otel_ctx(A, B)), ?_assertEqual(A, choose_viable_otel_ctx(B, A)),