From 28cfcc58f333ff29c15dfee576c0ca4712d3c63f Mon Sep 17 00:00:00 2001 From: dvic Date: Thu, 5 Aug 2021 22:21:43 +0200 Subject: [PATCH 1/5] Fix root span sampling --- apps/opentelemetry/src/otel_span_utils.erl | 16 ++++----------- .../test/opentelemetry_SUITE.erl | 20 ++++++++++++++++--- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/apps/opentelemetry/src/otel_span_utils.erl b/apps/opentelemetry/src/otel_span_utils.erl index 5ed9deed..3b53ca68 100644 --- a/apps/opentelemetry/src/otel_span_utils.erl +++ b/apps/opentelemetry/src/otel_span_utils.erl @@ -36,14 +36,14 @@ start_span(Ctx, Name, Opts) -> new_span(Ctx, Name, Sampler, StartTime, Kind, Attributes, Links). new_span(Ctx, Name, Sampler, StartTime, Kind, Attributes, Links) -> - ParentTraceId = trace_id(Ctx), - {TraceFlags, IsRecording, SamplerAttributes, TraceState} = - sample(Ctx, Sampler, ParentTraceId, Links, Name, Kind, Attributes), - {NewSpanCtx, ParentSpanId} = new_span_ctx(Ctx), + TraceId = NewSpanCtx#span_ctx.trace_id, SpanId = NewSpanCtx#span_ctx.span_id, + {TraceFlags, IsRecording, SamplerAttributes, TraceState} = + sample(Ctx, Sampler, TraceId, Links, Name, Kind, Attributes), + Span = #span{trace_id=TraceId, span_id=SpanId, tracestate=TraceState, @@ -60,14 +60,6 @@ new_span(Ctx, Name, Sampler, StartTime, Kind, Attributes, Links) -> is_valid=true, is_recording=IsRecording}, Span}. -trace_id(Ctx) -> - case otel_tracer:current_span_ctx(Ctx) of - #span_ctx{trace_id=TraceId} -> - TraceId; - _ -> - undefined - end. - -spec new_span_ctx(otel_ctx:t()) -> {opentelemetry:span_ctx(), opentelemetry:span_id()}. new_span_ctx(Ctx) -> case otel_tracer:current_span_ctx(Ctx) of diff --git a/apps/opentelemetry/test/opentelemetry_SUITE.erl b/apps/opentelemetry/test/opentelemetry_SUITE.erl index fb6b783d..04c94768 100644 --- a/apps/opentelemetry/test/opentelemetry_SUITE.erl +++ b/apps/opentelemetry/test/opentelemetry_SUITE.erl @@ -19,9 +19,9 @@ all() -> all_testcases() -> [disable_auto_registration, registered_tracers, with_span, macros, child_spans, - update_span_data, tracer_instrumentation_library, - tracer_previous_ctx, stop_temporary_app, reset_after, attach_ctx, default_sampler, - record_but_not_sample, record_exception_works, record_exception_with_message_works]. + update_span_data, tracer_instrumentation_library, tracer_previous_ctx, stop_temporary_app, + reset_after, attach_ctx, default_sampler, root_span_sampling, record_but_not_sample, + record_exception_works, record_exception_with_message_works]. groups() -> [{w3c, [], [propagation]}, @@ -457,6 +457,20 @@ default_sampler(_Config) -> ok. +root_span_sampling(_Config) -> + Tracer = opentelemetry:get_tracer(), + + Sampler = otel_sampler:setup({trace_id_ratio_based, 1.0}), + + SpanCtx1 = otel_tracer:start_span(Tracer, <<"span-1">>, #{sampler => Sampler}), + ?assertMatch(true, SpanCtx1#span_ctx.is_recording), + + otel_tracer:set_current_span(SpanCtx1), + SpanCtx2 = otel_tracer:start_span(Tracer, <<"span-2">>, #{sampler => Sampler}), + ?assertMatch(true, SpanCtx2#span_ctx.is_recording), + + ok. + record_but_not_sample(Config) -> ct:comment("Test that a Span that the sampler returns RECORD_ONLY for gets created" "as a valid recorded span but is not sent to the exporter."), From 08e2b572b7a6d10857a948e422c3f1458c1c6e67 Mon Sep 17 00:00:00 2001 From: dvic Date: Fri, 6 Aug 2021 14:23:49 +0200 Subject: [PATCH 2/5] Add tests for root_span_sampling always_on and always_off --- .../test/opentelemetry_SUITE.erl | 29 +++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/apps/opentelemetry/test/opentelemetry_SUITE.erl b/apps/opentelemetry/test/opentelemetry_SUITE.erl index 04c94768..76a436b7 100644 --- a/apps/opentelemetry/test/opentelemetry_SUITE.erl +++ b/apps/opentelemetry/test/opentelemetry_SUITE.erl @@ -20,8 +20,8 @@ all() -> all_testcases() -> [disable_auto_registration, registered_tracers, with_span, macros, child_spans, update_span_data, tracer_instrumentation_library, tracer_previous_ctx, stop_temporary_app, - reset_after, attach_ctx, default_sampler, root_span_sampling, record_but_not_sample, - record_exception_works, record_exception_with_message_works]. + reset_after, attach_ctx, default_sampler, root_span_sampling_always_on, root_span_sampling_always_on, + record_but_not_sample, record_exception_works, record_exception_with_message_works]. groups() -> [{w3c, [], [propagation]}, @@ -457,17 +457,36 @@ default_sampler(_Config) -> ok. -root_span_sampling(_Config) -> +root_span_sampling_always_off(_Config) -> Tracer = opentelemetry:get_tracer(), - Sampler = otel_sampler:setup({trace_id_ratio_based, 1.0}), + Sampler = otel_sampler:setup(always_off), + + SpanCtx1 = otel_tracer:start_span(Tracer, <<"span-1">>, #{sampler => Sampler}), + ?assertMatch(false, SpanCtx1#span_ctx.is_recording), + ?assertMatch(0, SpanCtx1#span_ctx.trace_flags), + + otel_tracer:set_current_span(SpanCtx1), + % if i pass sampler sampler => Sampler then the test succeeds + SpanCtx2 = otel_tracer:start_span(Tracer, <<"span-2">>, #{}), + ?assertMatch(false, SpanCtx2#span_ctx.is_recording), + ?assertMatch(0, SpanCtx2#span_ctx.trace_flags), + + ok. + +root_span_sampling_always_on(_Config) -> + Tracer = opentelemetry:get_tracer(), + + Sampler = otel_sampler:setup(always_on), SpanCtx1 = otel_tracer:start_span(Tracer, <<"span-1">>, #{sampler => Sampler}), ?assertMatch(true, SpanCtx1#span_ctx.is_recording), + ?assertMatch(1, SpanCtx1#span_ctx.trace_flags), otel_tracer:set_current_span(SpanCtx1), - SpanCtx2 = otel_tracer:start_span(Tracer, <<"span-2">>, #{sampler => Sampler}), + SpanCtx2 = otel_tracer:start_span(Tracer, <<"span-2">>, #{}), ?assertMatch(true, SpanCtx2#span_ctx.is_recording), + ?assertMatch(1, SpanCtx1#span_ctx.trace_flags), ok. From 92b592c9011472659132898a2c74a42dba5d19e6 Mon Sep 17 00:00:00 2001 From: dvic Date: Fri, 6 Aug 2021 19:57:59 +0200 Subject: [PATCH 3/5] Remove comment in test code --- apps/opentelemetry/test/opentelemetry_SUITE.erl | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/opentelemetry/test/opentelemetry_SUITE.erl b/apps/opentelemetry/test/opentelemetry_SUITE.erl index 76a436b7..94630e6c 100644 --- a/apps/opentelemetry/test/opentelemetry_SUITE.erl +++ b/apps/opentelemetry/test/opentelemetry_SUITE.erl @@ -467,7 +467,6 @@ root_span_sampling_always_off(_Config) -> ?assertMatch(0, SpanCtx1#span_ctx.trace_flags), otel_tracer:set_current_span(SpanCtx1), - % if i pass sampler sampler => Sampler then the test succeeds SpanCtx2 = otel_tracer:start_span(Tracer, <<"span-2">>, #{}), ?assertMatch(false, SpanCtx2#span_ctx.is_recording), ?assertMatch(0, SpanCtx2#span_ctx.trace_flags), From f3bf8a4dbab5e973bdf769c6be59e2e9778b76f8 Mon Sep 17 00:00:00 2001 From: dvic Date: Fri, 6 Aug 2021 20:48:41 +0200 Subject: [PATCH 4/5] Add missing test case to all_testcases() --- apps/opentelemetry/test/opentelemetry_SUITE.erl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/opentelemetry/test/opentelemetry_SUITE.erl b/apps/opentelemetry/test/opentelemetry_SUITE.erl index 94630e6c..73366a3b 100644 --- a/apps/opentelemetry/test/opentelemetry_SUITE.erl +++ b/apps/opentelemetry/test/opentelemetry_SUITE.erl @@ -20,8 +20,9 @@ all() -> all_testcases() -> [disable_auto_registration, registered_tracers, with_span, macros, child_spans, update_span_data, tracer_instrumentation_library, tracer_previous_ctx, stop_temporary_app, - reset_after, attach_ctx, default_sampler, root_span_sampling_always_on, root_span_sampling_always_on, - record_but_not_sample, record_exception_works, record_exception_with_message_works]. + reset_after, attach_ctx, default_sampler, root_span_sampling_always_on, + root_span_sampling_always_off,record_but_not_sample, record_exception_works, + record_exception_with_message_works]. groups() -> [{w3c, [], [propagation]}, From 7862aa52791cf1030d2366aa7ece2cff42814895 Mon Sep 17 00:00:00 2001 From: dvic Date: Fri, 20 Aug 2021 00:00:43 +0200 Subject: [PATCH 5/5] Update for refactored otel_sampler:new function --- apps/opentelemetry/test/opentelemetry_SUITE.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/opentelemetry/test/opentelemetry_SUITE.erl b/apps/opentelemetry/test/opentelemetry_SUITE.erl index 69a6aae5..d1a1a0c9 100644 --- a/apps/opentelemetry/test/opentelemetry_SUITE.erl +++ b/apps/opentelemetry/test/opentelemetry_SUITE.erl @@ -461,7 +461,7 @@ default_sampler(_Config) -> root_span_sampling_always_off(_Config) -> Tracer = opentelemetry:get_tracer(), - Sampler = otel_sampler:setup(always_off), + Sampler = otel_sampler:new(always_off), SpanCtx1 = otel_tracer:start_span(Tracer, <<"span-1">>, #{sampler => Sampler}), ?assertMatch(false, SpanCtx1#span_ctx.is_recording), @@ -477,7 +477,7 @@ root_span_sampling_always_off(_Config) -> root_span_sampling_always_on(_Config) -> Tracer = opentelemetry:get_tracer(), - Sampler = otel_sampler:setup(always_on), + Sampler = otel_sampler:new(always_on), SpanCtx1 = otel_tracer:start_span(Tracer, <<"span-1">>, #{sampler => Sampler}), ?assertMatch(true, SpanCtx1#span_ctx.is_recording),