Merge pull request #263 from qdentity/dvic-root-span-sampling

Fix root span sampling
open-telemetry · Aug 20, 2021 · c0487cf · c0487cf
2 parents ea75b2a + 38b964c
commit c0487cf
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 14 deletions.
diff --git 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
@@ -21,8 +21,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, non_recording_ets_table, record_but_not_sample,
-     record_exception_works, record_exception_with_message_works].
+     reset_after, attach_ctx, default_sampler, non_recording_ets_table, 
+     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]},
@@ -471,6 +472,38 @@ non_recording_ets_table(_Config) ->
     ?assertMatch([#span{name = <<"span-1">>}], ets:tab2list(?SPAN_TAB)),
     ok.
 
+root_span_sampling_always_off(_Config) ->
+    Tracer = opentelemetry:get_tracer(),
+
+    Sampler = otel_sampler:new(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),
+    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:new(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">>, #{}),
+    ?assertMatch(true, SpanCtx2#span_ctx.is_recording),
+    ?assertMatch(1, SpanCtx1#span_ctx.trace_flags),
+
+    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."),