Enhance sampling logic to record discarded transactions

Author: solnic

lib/sentry/opentelemetry/sampler.ex

@@ -3,6 +3,7 @@ if Code.ensure_loaded?(:otel_sampler) do
     @moduledoc false
 
     alias OpenTelemetry.{Span, Tracer}
+    alias Sentry.{ClientReport, Transaction}
 
     @behaviour :otel_sampler
 
@@ -30,19 +31,30 @@ if Code.ensure_loaded?(:otel_sampler) do
           _attributes,
           config
         ) do
-      if span_name in config[:drop] do
-        {:drop, [], []}
-      else
-        sample_rate = Sentry.Config.traces_sample_rate()
-
-        case get_parent_sampling_decision(ctx, trace_id) do
-          {:inherit, parent_sampled, tracestate} ->
-            decision = if parent_sampled, do: :record_and_sample, else: :drop
-            {decision, [], tracestate}
-
-          :no_parent ->
-            make_sampling_decision(sample_rate)
+      result =
+        if span_name in config[:drop] do
+          {:drop, [], []}
+        else
+          sample_rate = Sentry.Config.traces_sample_rate()
+
+          case get_parent_sampling_decision(ctx, trace_id) do
+            {:inherit, parent_sampled, tracestate} ->
+              decision = if parent_sampled, do: :record_and_sample, else: :drop
+
+              {decision, [], tracestate}
+
+            :no_parent ->
+              make_sampling_decision(sample_rate)
+          end
         end
+
+      case result do
+        {:drop, _, _} ->
+          record_discarded_transaction(ctx)
+          result
+
+        _ ->
+          result
       end
     end
 
@@ -107,5 +119,22 @@ if Code.ensure_loaded?(:otel_sampler) do
         nil -> nil
       end
     end
+
+    defp record_discarded_transaction(ctx) do
+      span_id =
+        case Tracer.current_span_ctx(ctx) do
+          :undefined -> Sentry.UUID.uuid4_hex()
+          span_ctx -> Span.span_id(span_ctx)
+        end
+
+      transaction =
+        Transaction.new(%{
+          span_id: span_id,
+          start_timestamp: System.system_time(:second),
+          timestamp: System.system_time(:second)
+        })
+
+      ClientReport.Sender.record_discarded_events(:sample_rate, [transaction])
+    end
   end
 end

test/sentry/opentelemetry/sampler_test.exs

@@ -2,6 +2,21 @@ defmodule Sentry.Opentelemetry.SamplerTest do
   use Sentry.Case, async: true
 
   alias Sentry.OpenTelemetry.Sampler
+  alias Sentry.ClientReport
+
+  defp create_test_span_context(span_id \\ 123_456_789) do
+    {
+      :span_ctx,
+      12_345_678_901_234_567_890_123_456_789_012,
+      span_id,
+      1,
+      [],
+      true,
+      false,
+      true,
+      nil
+    }
+  end
 
   setup do
     original_rate = Sentry.Config.traces_sample_rate()
@@ -14,18 +29,29 @@ defmodule Sentry.Opentelemetry.SamplerTest do
   end
 
   describe "span name dropping" do
-    test "drops spans with the given name" do
+    test "drops spans with the given name and records discarded event" do
+      :sys.replace_state(ClientReport.Sender, fn _ -> %{} end)
+
+      test_ctx = create_test_span_context()
+
       assert {:drop, [], []} =
-               Sampler.should_sample(nil, nil, nil, "Elixir.Oban.Stager process", nil, nil,
+               Sampler.should_sample(test_ctx, nil, nil, "Elixir.Oban.Stager process", nil, nil,
                  drop: ["Elixir.Oban.Stager process"]
                )
+
+      Process.sleep(10)
+
+      state = :sys.get_state(ClientReport.Sender)
+      assert state == %{{:sample_rate, "transaction"} => 1}
     end
 
     test "records and samples spans not in drop list" do
       Sentry.Config.put_config(:traces_sample_rate, 1.0)
 
+      test_ctx = create_test_span_context()
+
       assert {:record_and_sample, [], tracestate} =
-               Sampler.should_sample(nil, 123, nil, "Elixir.Oban.Worker process", nil, nil,
+               Sampler.should_sample(test_ctx, 123, nil, "Elixir.Oban.Worker process", nil, nil,
                  drop: []
                )
 
@@ -36,21 +62,32 @@ defmodule Sentry.Opentelemetry.SamplerTest do
   end
 
   describe "sampling based on traces_sample_rate" do
-    test "always drops when sample rate is 0.0" do
+    test "always drops when sample rate is 0.0 and records discarded event" do
+      :sys.replace_state(ClientReport.Sender, fn _ -> %{} end)
+
       Sentry.Config.put_config(:traces_sample_rate, 0.0)
 
+      test_ctx = create_test_span_context()
+
       assert {:drop, [], tracestate} =
-               Sampler.should_sample(nil, 123, nil, "test span", nil, nil, drop: [])
+               Sampler.should_sample(test_ctx, 123, nil, "test span", nil, nil, drop: [])
 
       assert {"sentry-sample_rate", "0.0"} in tracestate
       assert {"sentry-sampled", "false"} in tracestate
+
+      Process.sleep(10)
+
+      state = :sys.get_state(ClientReport.Sender)
+      assert state == %{{:sample_rate, "transaction"} => 1}
     end
 
     test "always samples when sample rate is 1.0" do
       Sentry.Config.put_config(:traces_sample_rate, 1.0)
 
+      test_ctx = create_test_span_context()
+
       assert {:record_and_sample, [], tracestate} =
-               Sampler.should_sample(nil, 123, nil, "test span", nil, nil, drop: [])
+               Sampler.should_sample(test_ctx, 123, nil, "test span", nil, nil, drop: [])
 
       assert {"sentry-sample_rate", "1.0"} in tracestate
       assert {"sentry-sampled", "true"} in tracestate
@@ -63,8 +100,10 @@ defmodule Sentry.Opentelemetry.SamplerTest do
 
       results =
         Enum.map(trace_ids, fn trace_id ->
+          test_ctx = create_test_span_context()
+
           {decision, [], _tracestate} =
-            Sampler.should_sample(nil, trace_id, nil, "test span", nil, nil, drop: [])
+            Sampler.should_sample(test_ctx, trace_id, nil, "test span", nil, nil, drop: [])
 
           decision == :record_and_sample
         end)
@@ -73,23 +112,44 @@ defmodule Sentry.Opentelemetry.SamplerTest do
 
       assert sampled_count > 30 and sampled_count < 70
     end
+
+    test "records discarded events when randomly dropped by sample rate" do
+      :sys.replace_state(ClientReport.Sender, fn _ -> %{} end)
+
+      Sentry.Config.put_config(:traces_sample_rate, 0.001)
+
+      Enum.each(1..50, fn trace_id ->
+        test_ctx = create_test_span_context()
+        Sampler.should_sample(test_ctx, trace_id, nil, "test span", nil, nil, drop: [])
+      end)
+
+      Process.sleep(10)
+
+      state = :sys.get_state(ClientReport.Sender)
+      discarded_count = Map.get(state, {:sample_rate, "transaction"}, 0)
+      assert discarded_count > 0, "Expected some spans to be dropped and recorded"
+    end
   end
 
   describe "parent span inheritance" do
     test "inherits sampling decision from parent span with same trace_id" do
       Sentry.Config.put_config(:traces_sample_rate, 1.0)
 
+      test_ctx = create_test_span_context()
+
       assert {:record_and_sample, [], _tracestate} =
-               Sampler.should_sample(nil, 123, nil, "test span", nil, nil, drop: [])
+               Sampler.should_sample(test_ctx, 123, nil, "test span", nil, nil, drop: [])
     end
   end
 
   describe "tracestate management" do
     test "builds tracestate with correct format" do
       Sentry.Config.put_config(:traces_sample_rate, 0.75)
 
+      test_ctx = create_test_span_context()
+
       {_decision, [], tracestate} =
-        Sampler.should_sample(nil, 123, nil, "test span", nil, nil, drop: [])
+        Sampler.should_sample(test_ctx, 123, nil, "test span", nil, nil, drop: [])
 
       assert List.keyfind(tracestate, "sentry-sample_rate", 0)
       assert List.keyfind(tracestate, "sentry-sample_rand", 0)

test/sentry/opentelemetry/span_processor_test.exs

@@ -136,13 +136,18 @@ defmodule Sentry.Opentelemetry.SpanProcessorTest do
     test "drops entire trace when root span is not sampled" do
       put_test_config(environment_name: "test", traces_sample_rate: 0.0)
 
+      original_sampler = Application.get_env(:opentelemetry, :sampler)
+      Application.put_env(:opentelemetry, :sampler, {Sentry.OpenTelemetry.Sampler, [drop: []]})
+
       Sentry.Test.start_collecting_sentry_reports()
 
       Enum.each(1..5, fn _ ->
         TestEndpoint.instrumented_function()
       end)
 
       assert [] = Sentry.Test.pop_sentry_transactions()
+
+      Application.put_env(:opentelemetry, :sampler, original_sampler)
     end
 
     @tag span_storage: true
@@ -165,6 +170,9 @@ defmodule Sentry.Opentelemetry.SpanProcessorTest do
     test "child spans inherit parent sampling decision" do
       put_test_config(environment_name: "test", traces_sample_rate: 0.5)
 
+      original_sampler = Application.get_env(:opentelemetry, :sampler)
+      Application.put_env(:opentelemetry, :sampler, {Sentry.OpenTelemetry.Sampler, [drop: []]})
+
       Sentry.Test.start_collecting_sentry_reports()
 
       Enum.each(1..10, fn _ ->
@@ -180,6 +188,8 @@ defmodule Sentry.Opentelemetry.SpanProcessorTest do
         assert transaction.contexts.trace.trace_id == child_span_one.trace_id
         assert transaction.contexts.trace.trace_id == child_span_two.trace_id
       end)
+
+      Application.put_env(:opentelemetry, :sampler, original_sampler)
     end
 
     @tag span_storage: true
@@ -239,6 +249,9 @@ defmodule Sentry.Opentelemetry.SpanProcessorTest do
     test "concurrent traces maintain independent sampling decisions" do
       put_test_config(environment_name: "test", traces_sample_rate: 0.5)
 
+      original_sampler = Application.get_env(:opentelemetry, :sampler)
+      Application.put_env(:opentelemetry, :sampler, {Sentry.OpenTelemetry.Sampler, [drop: []]})
+
       Sentry.Test.start_collecting_sentry_reports()
 
       tasks =
@@ -266,6 +279,8 @@ defmodule Sentry.Opentelemetry.SpanProcessorTest do
 
       assert length(transactions) >= 5
       assert length(transactions) <= 20
+
+      Application.put_env(:opentelemetry, :sampler, original_sampler)
     end
 
     @tag span_storage: true