From 27bfc447e5a3f4fd2e29a492452f0f3d2ea4d155 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Tue, 10 Jun 2025 09:03:36 +0000 Subject: [PATCH 1/7] Add support for `traces_sampler` config option --- .dialyzer_ignore.exs | 3 +- lib/sentry/config.ex | 60 +++++++- lib/sentry/opentelemetry/sampler.ex | 109 ++++++++++++- lib/sentry/sampling_context.ex | 57 +++++++ test/sentry/config_traces_sampler_test.exs | 81 ++++++++++ test/sentry/opentelemetry/sampler_test.exs | 170 ++++++++++++++++++++- 6 files changed, 469 insertions(+), 11 deletions(-) create mode 100644 lib/sentry/sampling_context.ex create mode 100644 test/sentry/config_traces_sampler_test.exs diff --git a/.dialyzer_ignore.exs b/.dialyzer_ignore.exs index 537452162..f0a3b9e36 100644 --- a/.dialyzer_ignore.exs +++ b/.dialyzer_ignore.exs @@ -1,4 +1,5 @@ [ {"test/support/example_plug_application.ex"}, - {"test/support/test_helpers.ex"} + {"test/support/test_helpers.ex"}, + {"lib/sentry/opentelemetry/sampler.ex", :pattern_match, 1} ] diff --git a/lib/sentry/config.ex b/lib/sentry/config.ex index 7d21873dd..bb340307d 100644 --- a/lib/sentry/config.ex +++ b/lib/sentry/config.ex @@ -1,6 +1,13 @@ defmodule Sentry.Config do @moduledoc false + @typedoc """ + A function that determines the sample rate for transaction events. + + The function receives a sampling context map and should return a boolean or a float between `0.0` and `1.0`. + """ + @type traces_sampler_function :: (map() -> boolean() | float()) | {module(), atom()} + integrations_schema = [ max_expected_check_in_time: [ type: :integer, @@ -158,6 +165,34 @@ defmodule Sentry.Config do for guides on how to set it up. """ ], + traces_sampler: [ + type: {:custom, __MODULE__, :__validate_traces_sampler__, []}, + default: nil, + type_doc: "`t:traces_sampler_function/0` or `nil`", + doc: """ + A function that determines the sample rate for transaction events. This function + receives a sampling context map and should return a boolean or a float between `0.0` and `1.0`. + + The sampling context contains: + - `:parent_sampled` - boolean indicating if the parent trace was sampled (nil if no parent) + - `:transaction_context` - map with transaction information (name, op, etc.) + + If both `:traces_sampler` and `:traces_sample_rate` are configured, `:traces_sampler` takes precedence. + + Example: + ```elixir + traces_sampler: fn sampling_context -> + case sampling_context[:transaction_context][:op] do + "http.server" -> 0.1 # Sample 10% of HTTP requests + "db.query" -> 0.01 # Sample 1% of database queries + _ -> false # Don't sample other operations + end + end + ``` + + This value is also used to determine if tracing is enabled: if it's not `nil`, tracing is enabled. + """ + ], included_environments: [ type: {:or, [{:in, [:all]}, {:list, {:or, [:atom, :string]}}]}, deprecated: "Use :dsn to control whether to send events to Sentry.", @@ -625,6 +660,9 @@ defmodule Sentry.Config do @spec traces_sample_rate() :: nil | float() def traces_sample_rate, do: fetch!(:traces_sample_rate) + @spec traces_sampler() :: traces_sampler_function() | nil + def traces_sampler, do: get(:traces_sampler) + @spec hackney_opts() :: keyword() def hackney_opts, do: fetch!(:hackney_opts) @@ -663,7 +701,7 @@ defmodule Sentry.Config do def integrations, do: fetch!(:integrations) @spec tracing?() :: boolean() - def tracing?, do: not is_nil(fetch!(:traces_sample_rate)) + def tracing?, do: not is_nil(fetch!(:traces_sample_rate)) or not is_nil(get(:traces_sampler)) @spec put_config(atom(), term()) :: :ok def put_config(key, value) when is_atom(key) do @@ -773,6 +811,26 @@ defmodule Sentry.Config do end end + def __validate_traces_sampler__(nil), do: {:ok, nil} + + def __validate_traces_sampler__(fun) when is_function(fun, 1) do + {:ok, fun} + end + + def __validate_traces_sampler__({module, function}) + when is_atom(module) and is_atom(function) do + if function_exported?(module, function, 1) do + {:ok, {module, function}} + else + {:error, "function #{module}.#{function}/1 is not exported"} + end + end + + def __validate_traces_sampler__(other) do + {:error, + "expected :traces_sampler to be nil, a function with arity 1, or a {module, function} tuple, got: #{inspect(other)}"} + end + def __validate_json_library__(nil) do {:error, "nil is not a valid value for the :json_library option"} end diff --git a/lib/sentry/opentelemetry/sampler.ex b/lib/sentry/opentelemetry/sampler.ex index 2c7d6087d..df957b2a3 100644 --- a/lib/sentry/opentelemetry/sampler.ex +++ b/lib/sentry/opentelemetry/sampler.ex @@ -4,6 +4,9 @@ if Code.ensure_loaded?(:otel_sampler) do alias OpenTelemetry.{Span, Tracer} alias Sentry.ClientReport + alias SamplingContext + + require Logger @behaviour :otel_sampler @@ -24,27 +27,47 @@ if Code.ensure_loaded?(:otel_sampler) do @impl true def should_sample( ctx, - _trace_id, + trace_id, _links, span_name, - _span_kind, - _attributes, + span_kind, + attributes, config ) do result = if span_name in config[:drop] do {:drop, [], []} else - sample_rate = Sentry.Config.traces_sample_rate() + traces_sampler = Sentry.Config.traces_sampler() + traces_sample_rate = Sentry.Config.traces_sample_rate() case get_trace_sampling_decision(ctx) do {:inherit, trace_sampled, tracestate} -> - decision = if trace_sampled, do: :record_and_sample, else: :drop - - {decision, [], tracestate} + if traces_sampler do + sampling_context = + build_sampling_context( + trace_sampled, + span_name, + span_kind, + attributes, + trace_id + ) + + make_sampler_decision(traces_sampler, sampling_context, tracestate) + else + decision = if trace_sampled, do: :record_and_sample, else: :drop + {decision, [], tracestate} + end :no_trace -> - make_sampling_decision(sample_rate) + if traces_sampler do + sampling_context = + build_sampling_context(nil, span_name, span_kind, attributes, trace_id) + + make_sampler_decision(traces_sampler, sampling_context, []) + else + make_sampling_decision(traces_sample_rate) + end end end @@ -121,6 +144,76 @@ if Code.ensure_loaded?(:otel_sampler) do end end + defp build_sampling_context(parent_sampled, span_name, _span_kind, attributes, trace_id) do + transaction_context = %{ + name: span_name, + op: span_name, + trace_id: trace_id, + attributes: attributes + } + + sampling_context = %SamplingContext{ + transaction_context: transaction_context, + parent_sampled: parent_sampled + } + + if attributes && map_size(attributes) > 0 do + Map.merge(sampling_context, attributes) + else + sampling_context + end + end + + defp make_sampler_decision(traces_sampler, sampling_context, _existing_tracestate) do + try do + result = call_traces_sampler(traces_sampler, sampling_context) + sample_rate = normalize_sampler_result(result) + + cond do + sample_rate == 0.0 -> + tracestate = build_tracestate(0.0, 1.0, false) + + {:drop, [], tracestate} + + sample_rate == 1.0 -> + tracestate = build_tracestate(1.0, 0.0, true) + + {:record_and_sample, [], tracestate} + + is_float(sample_rate) and sample_rate > 0.0 and sample_rate < 1.0 -> + random_value = :rand.uniform() + sampled = random_value < sample_rate + tracestate = build_tracestate(sample_rate, random_value, sampled) + decision = if sampled, do: :record_and_sample, else: :drop + + {decision, [], tracestate} + + true -> + tracestate = build_tracestate(0.0, 1.0, false) + + {:drop, [], tracestate} + end + rescue + error -> + Logger.warning("traces_sampler function failed: #{inspect(error)}") + + tracestate = build_tracestate(0.0, 1.0, false) + {:drop, [], tracestate} + end + end + + defp call_traces_sampler(fun, sampling_context) when is_function(fun, 1) do + fun.(sampling_context) + end + + defp call_traces_sampler({module, function}, sampling_context) do + apply(module, function, [sampling_context]) + end + + defp normalize_sampler_result(true), do: 1.0 + defp normalize_sampler_result(false), do: 0.0 + defp normalize_sampler_result(rate) when is_float(rate), do: rate + defp record_discarded_transaction() do ClientReport.Sender.record_discarded_events(:sample_rate, "transaction") end diff --git a/lib/sentry/sampling_context.ex b/lib/sentry/sampling_context.ex new file mode 100644 index 000000000..a64f3edfd --- /dev/null +++ b/lib/sentry/sampling_context.ex @@ -0,0 +1,57 @@ +defmodule SamplingContext do + @moduledoc """ + The struct for the **sampling_context** that is passed to `traces_sampler`. + + This is set up via `Sentry.OpenTelemetry.Sampler`. + + See also . + """ + + @moduledoc since: "11.0.0" + + @typedoc """ + The sampling context struct that contains information needed for sampling decisions. + + This matches the structure used in the Python SDK's create_sampling_context function. + """ + @type t :: %__MODULE__{ + transaction_context: %{ + name: String.t() | nil, + op: String.t(), + trace_id: String.t(), + attributes: map() + }, + parent_sampled: boolean() | nil + } + + @enforce_keys [:transaction_context, :parent_sampled] + defstruct [:transaction_context, :parent_sampled] + + @behaviour Access + + @impl Access + def fetch(struct, key) do + case Map.fetch(struct, key) do + {:ok, value} -> {:ok, value} + :error -> :error + end + end + + @impl Access + def get_and_update(struct, key, function) do + current_value = Map.get(struct, key) + + case function.(current_value) do + {get_value, update_value} -> + {get_value, Map.put(struct, key, update_value)} + + :pop -> + {current_value, Map.delete(struct, key)} + end + end + + @impl Access + def pop(struct, key) do + {Map.get(struct, key), Map.delete(struct, key)} + end +end diff --git a/test/sentry/config_traces_sampler_test.exs b/test/sentry/config_traces_sampler_test.exs new file mode 100644 index 000000000..0caf78b5c --- /dev/null +++ b/test/sentry/config_traces_sampler_test.exs @@ -0,0 +1,81 @@ +defmodule Sentry.ConfigTracesSamplerTest do + use ExUnit.Case, async: true + + import Sentry.TestHelpers + + describe "traces_sampler configuration validation" do + defmodule TestSampler do + def sample(_context), do: 0.5 + end + + test "accepts nil" do + assert :ok = put_test_config(traces_sampler: nil) + assert Sentry.Config.traces_sampler() == nil + end + + test "accepts function with arity 1" do + fun = fn _context -> 0.5 end + assert :ok = put_test_config(traces_sampler: fun) + assert Sentry.Config.traces_sampler() == fun + end + + test "accepts MFA tuple with exported function" do + assert :ok = put_test_config(traces_sampler: {TestSampler, :sample}) + assert Sentry.Config.traces_sampler() == {TestSampler, :sample} + end + + test "rejects MFA tuple with non-exported function" do + assert_raise ArgumentError, ~r/function.*is not exported/, fn -> + put_test_config(traces_sampler: {TestSampler, :non_existent}) + end + end + + test "rejects function with wrong arity" do + fun = fn -> 0.5 end + + assert_raise ArgumentError, ~r/expected :traces_sampler to be/, fn -> + put_test_config(traces_sampler: fun) + end + end + + test "rejects invalid types" do + assert_raise ArgumentError, ~r/expected :traces_sampler to be/, fn -> + put_test_config(traces_sampler: "invalid") + end + + assert_raise ArgumentError, ~r/expected :traces_sampler to be/, fn -> + put_test_config(traces_sampler: 123) + end + + assert_raise ArgumentError, ~r/expected :traces_sampler to be/, fn -> + put_test_config(traces_sampler: []) + end + end + end + + describe "tracing? function" do + test "returns true when traces_sample_rate is set" do + put_test_config(traces_sample_rate: 0.5, traces_sampler: nil) + + assert Sentry.Config.tracing?() + end + + test "returns true when traces_sampler is set" do + put_test_config(traces_sample_rate: nil, traces_sampler: fn _ -> 0.5 end) + + assert Sentry.Config.tracing?() + end + + test "returns true when both are set" do + put_test_config(traces_sample_rate: 0.5, traces_sampler: fn _ -> 0.5 end) + + assert Sentry.Config.tracing?() + end + + test "returns false when neither is set" do + put_test_config(traces_sample_rate: nil, traces_sampler: nil) + + refute Sentry.Config.tracing?() + end + end +end diff --git a/test/sentry/opentelemetry/sampler_test.exs b/test/sentry/opentelemetry/sampler_test.exs index 9b9f2ee7b..63b69368a 100644 --- a/test/sentry/opentelemetry/sampler_test.exs +++ b/test/sentry/opentelemetry/sampler_test.exs @@ -182,7 +182,6 @@ defmodule Sentry.Opentelemetry.SamplerTest do test "all spans in trace inherit sampling decision to sample when trace was sampled" do trace_id = 12_345_678_901_234_567_890_123_456_789_012 - # Simulate existing trace context with sample decision trace_tracestate = [ {"sentry-sample_rate", "1.0"}, {"sentry-sample_rand", "0.5"}, @@ -295,4 +294,173 @@ defmodule Sentry.Opentelemetry.SamplerTest do assert sampled_str in ["true", "false"] end end + + describe "traces_sampler functionality" do + test "uses traces_sampler when configured" do + sampler_fun = fn _sampling_context -> 0.5 end + put_test_config(traces_sampler: sampler_fun) + + test_ctx = create_test_span_context() + + {decision, [], tracestate} = + Sampler.should_sample(test_ctx, 123, nil, "test span", :server, %{}, drop: []) + + assert decision in [:record_and_sample, :drop] + assert {"sentry-sample_rate", "0.5"} in tracestate + assert {"sentry-sampled", _} = List.keyfind(tracestate, "sentry-sampled", 0) + end + + test "traces_sampler receives correct sampling context" do + {:ok, received_context} = Agent.start_link(fn -> nil end) + + sampler_fun = fn sampling_context -> + Agent.update(received_context, fn _ -> sampling_context end) + true + end + + put_test_config(traces_sampler: sampler_fun) + + test_ctx = create_test_span_context() + attributes = %{"http.method" => "GET", "http.url" => "http://example.com"} + + Sampler.should_sample(test_ctx, 123, nil, "GET /users", :server, attributes, drop: []) + + context = Agent.get(received_context, & &1) + + assert context[:parent_sampled] == nil + assert context[:transaction_context][:name] == "GET /users" + assert context[:transaction_context][:op] == "GET /users" + assert context[:transaction_context][:trace_id] == 123 + assert context[:transaction_context][:attributes] == attributes + + Agent.stop(received_context) + end + + test "traces_sampler can return boolean values" do + put_test_config(traces_sampler: fn _ -> true end) + test_ctx = create_test_span_context() + + assert {:record_and_sample, [], tracestate} = + Sampler.should_sample(test_ctx, 123, nil, "test span", nil, %{}, drop: []) + + assert {"sentry-sampled", "true"} in tracestate + + put_test_config(traces_sampler: fn _ -> false end) + + assert {:drop, [], tracestate} = + Sampler.should_sample(test_ctx, 123, nil, "test span", nil, %{}, drop: []) + + assert {"sentry-sampled", "false"} in tracestate + end + + test "traces_sampler can return float values" do + put_test_config(traces_sampler: fn _ -> 0.75 end) + + test_ctx = create_test_span_context() + + {decision, [], tracestate} = + Sampler.should_sample(test_ctx, 123, nil, "test span", nil, %{}, drop: []) + + assert decision in [:record_and_sample, :drop] + assert {"sentry-sample_rate", "0.75"} in tracestate + end + + test "traces_sampler takes precedence over traces_sample_rate" do + put_test_config(traces_sample_rate: 1.0, traces_sampler: fn _ -> false end) + + test_ctx = create_test_span_context() + + assert {:drop, [], _tracestate} = + Sampler.should_sample(test_ctx, 123, nil, "test span", nil, %{}, drop: []) + end + + test "traces_sampler can override parent sampling decision" do + trace_tracestate = [ + {"sentry-sample_rate", "1.0"}, + {"sentry-sample_rand", "0.5"}, + {"sentry-sampled", "true"} + ] + + existing_span_ctx = create_span_context_with_tracestate(123, trace_tracestate) + + ctx = :otel_ctx.new() + ctx_with_span = :otel_tracer.set_current_span(ctx, existing_span_ctx) + token = :otel_ctx.attach(ctx_with_span) + + try do + put_test_config(traces_sampler: fn _ -> false end) + + result = + Sampler.should_sample(ctx_with_span, 123, nil, "child span", nil, %{}, drop: []) + + assert {:drop, [], _tracestate} = result + after + :otel_ctx.detach(token) + end + end + + test "handles traces_sampler errors gracefully" do + put_test_config(traces_sampler: fn _ -> raise "sampler error" end) + + test_ctx = create_test_span_context() + + assert {:drop, [], _tracestate} = + Sampler.should_sample(test_ctx, 123, nil, "test span", nil, %{}, drop: []) + end + + test "supports MFA tuple for traces_sampler" do + defmodule TestSampler do + def sample(_sampling_context), do: 0.25 + end + + put_test_config(traces_sampler: {TestSampler, :sample}) + + test_ctx = create_test_span_context() + + {decision, [], tracestate} = + Sampler.should_sample(test_ctx, 123, nil, "test span", nil, %{}, drop: []) + + assert decision in [:record_and_sample, :drop] + assert {"sentry-sample_rate", "0.25"} in tracestate + end + + test "uses span name as operation and passes attributes" do + {:ok, received_context} = Agent.start_link(fn -> nil end) + + sampler_fun = fn sampling_context -> + Agent.update(received_context, fn _ -> sampling_context end) + true + end + + put_test_config(traces_sampler: sampler_fun) + + test_ctx = create_test_span_context() + + http_attributes = %{"http.method" => "POST"} + + Sampler.should_sample(test_ctx, 123, nil, "POST /api", :server, http_attributes, drop: []) + + context = Agent.get(received_context, & &1) + assert context[:transaction_context][:op] == "POST /api" + assert context[:transaction_context][:attributes] == http_attributes + + db_attributes = %{"db.system" => "postgresql"} + + Sampler.should_sample(test_ctx, 124, nil, "SELECT users", :client, db_attributes, drop: []) + + context = Agent.get(received_context, & &1) + assert context[:transaction_context][:op] == "SELECT users" + assert context[:transaction_context][:attributes] == db_attributes + + oban_attributes = %{"messaging.system" => :oban} + + Sampler.should_sample(test_ctx, 125, nil, "MyWorker", :consumer, oban_attributes, drop: []) + + context = Agent.get(received_context, & &1) + assert context[:transaction_context][:op] == "MyWorker" + assert context[:transaction_context][:attributes] == oban_attributes + + Agent.stop(received_context) + end + end end From fb3bced462ead331c8afe62646ac307abc1f8060 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Wed, 11 Jun 2025 13:16:06 +0000 Subject: [PATCH 2/7] Update docs --- lib/sentry/config.ex | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/sentry/config.ex b/lib/sentry/config.ex index bb340307d..8987be4ea 100644 --- a/lib/sentry/config.ex +++ b/lib/sentry/config.ex @@ -171,10 +171,10 @@ defmodule Sentry.Config do type_doc: "`t:traces_sampler_function/0` or `nil`", doc: """ A function that determines the sample rate for transaction events. This function - receives a sampling context map and should return a boolean or a float between `0.0` and `1.0`. + receives a sampling context struct and should return a boolean or a float between `0.0` and `1.0`. The sampling context contains: - - `:parent_sampled` - boolean indicating if the parent trace was sampled (nil if no parent) + - `:parent_sampled` - boolean indicating if the parent trace span was sampled (nil if no parent) - `:transaction_context` - map with transaction information (name, op, etc.) If both `:traces_sampler` and `:traces_sample_rate` are configured, `:traces_sampler` takes precedence. @@ -182,7 +182,7 @@ defmodule Sentry.Config do Example: ```elixir traces_sampler: fn sampling_context -> - case sampling_context[:transaction_context][:op] do + case sampling_context.transaction_context.op do "http.server" -> 0.1 # Sample 10% of HTTP requests "db.query" -> 0.01 # Sample 1% of database queries _ -> false # Don't sample other operations From 8b597f20ee62f9d647332696a5cb90b221790415 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Tue, 17 Jun 2025 05:49:15 +0000 Subject: [PATCH 3/7] Remove unnecessary attribute merging --- lib/sentry/opentelemetry/sampler.ex | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/sentry/opentelemetry/sampler.ex b/lib/sentry/opentelemetry/sampler.ex index df957b2a3..12ec44385 100644 --- a/lib/sentry/opentelemetry/sampler.ex +++ b/lib/sentry/opentelemetry/sampler.ex @@ -157,11 +157,7 @@ if Code.ensure_loaded?(:otel_sampler) do parent_sampled: parent_sampled } - if attributes && map_size(attributes) > 0 do - Map.merge(sampling_context, attributes) - else - sampling_context - end + sampling_context end defp make_sampler_decision(traces_sampler, sampling_context, _existing_tracestate) do From bf32449eded59efb025a0be6fb3e7fb3399a772c Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Tue, 17 Jun 2025 05:57:19 +0000 Subject: [PATCH 4/7] Call traces_sampler only for root spans --- lib/sentry/opentelemetry/sampler.ex | 17 ++------- test/sentry/opentelemetry/sampler_test.exs | 42 +++++++++++++++++++--- 2 files changed, 40 insertions(+), 19 deletions(-) diff --git a/lib/sentry/opentelemetry/sampler.ex b/lib/sentry/opentelemetry/sampler.ex index 12ec44385..23287afcf 100644 --- a/lib/sentry/opentelemetry/sampler.ex +++ b/lib/sentry/opentelemetry/sampler.ex @@ -43,21 +43,8 @@ if Code.ensure_loaded?(:otel_sampler) do case get_trace_sampling_decision(ctx) do {:inherit, trace_sampled, tracestate} -> - if traces_sampler do - sampling_context = - build_sampling_context( - trace_sampled, - span_name, - span_kind, - attributes, - trace_id - ) - - make_sampler_decision(traces_sampler, sampling_context, tracestate) - else - decision = if trace_sampled, do: :record_and_sample, else: :drop - {decision, [], tracestate} - end + decision = if trace_sampled, do: :record_and_sample, else: :drop + {decision, [], tracestate} :no_trace -> if traces_sampler do diff --git a/test/sentry/opentelemetry/sampler_test.exs b/test/sentry/opentelemetry/sampler_test.exs index 63b69368a..f37594a0d 100644 --- a/test/sentry/opentelemetry/sampler_test.exs +++ b/test/sentry/opentelemetry/sampler_test.exs @@ -374,7 +374,16 @@ defmodule Sentry.Opentelemetry.SamplerTest do Sampler.should_sample(test_ctx, 123, nil, "test span", nil, %{}, drop: []) end - test "traces_sampler can override parent sampling decision" do + test "child spans inherit parent sampling decision without calling traces_sampler" do + {:ok, sampler_call_count} = Agent.start_link(fn -> 0 end) + + sampler_fun = fn _sampling_context -> + Agent.update(sampler_call_count, &(&1 + 1)) + false + end + + put_test_config(traces_sampler: sampler_fun) + trace_tracestate = [ {"sentry-sample_rate", "1.0"}, {"sentry-sample_rand", "0.5"}, @@ -388,15 +397,40 @@ defmodule Sentry.Opentelemetry.SamplerTest do token = :otel_ctx.attach(ctx_with_span) try do - put_test_config(traces_sampler: fn _ -> false end) - result = Sampler.should_sample(ctx_with_span, 123, nil, "child span", nil, %{}, drop: []) - assert {:drop, [], _tracestate} = result + assert {:record_and_sample, [], returned_tracestate} = result + assert returned_tracestate == trace_tracestate + + call_count = Agent.get(sampler_call_count, & &1) + assert call_count == 0 after :otel_ctx.detach(token) + Agent.stop(sampler_call_count) + end + end + + test "traces_sampler is only called for root spans" do + {:ok, sampler_call_count} = Agent.start_link(fn -> 0 end) + + sampler_fun = fn _sampling_context -> + Agent.update(sampler_call_count, &(&1 + 1)) + true end + + put_test_config(traces_sampler: sampler_fun) + + test_ctx = create_test_span_context() + + result = Sampler.should_sample(test_ctx, 123, nil, "root span", nil, %{}, drop: []) + + assert {:record_and_sample, [], _tracestate} = result + + call_count = Agent.get(sampler_call_count, & &1) + assert call_count == 1 + + Agent.stop(sampler_call_count) end test "handles traces_sampler errors gracefully" do From e616c212967fac9ef965f714d9ab8ab3ebe5047b Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Tue, 17 Jun 2025 06:14:55 +0000 Subject: [PATCH 5/7] Reuse sampling decision logic --- lib/sentry/opentelemetry/sampler.ex | 38 +++++++++-------------------- 1 file changed, 11 insertions(+), 27 deletions(-) diff --git a/lib/sentry/opentelemetry/sampler.ex b/lib/sentry/opentelemetry/sampler.ex index 23287afcf..f37de5c35 100644 --- a/lib/sentry/opentelemetry/sampler.ex +++ b/lib/sentry/opentelemetry/sampler.ex @@ -51,7 +51,7 @@ if Code.ensure_loaded?(:otel_sampler) do sampling_context = build_sampling_context(nil, span_name, span_kind, attributes, trace_id) - make_sampler_decision(traces_sampler, sampling_context, []) + make_sampler_decision(traces_sampler, sampling_context) else make_sampling_decision(traces_sample_rate) end @@ -147,41 +147,25 @@ if Code.ensure_loaded?(:otel_sampler) do sampling_context end - defp make_sampler_decision(traces_sampler, sampling_context, _existing_tracestate) do + defp make_sampler_decision(traces_sampler, sampling_context) do try do result = call_traces_sampler(traces_sampler, sampling_context) sample_rate = normalize_sampler_result(result) - cond do - sample_rate == 0.0 -> - tracestate = build_tracestate(0.0, 1.0, false) - - {:drop, [], tracestate} - - sample_rate == 1.0 -> - tracestate = build_tracestate(1.0, 0.0, true) - - {:record_and_sample, [], tracestate} - - is_float(sample_rate) and sample_rate > 0.0 and sample_rate < 1.0 -> - random_value = :rand.uniform() - sampled = random_value < sample_rate - tracestate = build_tracestate(sample_rate, random_value, sampled) - decision = if sampled, do: :record_and_sample, else: :drop - - {decision, [], tracestate} - - true -> - tracestate = build_tracestate(0.0, 1.0, false) + if is_float(sample_rate) and sample_rate >= 0.0 and sample_rate <= 1.0 do + make_sampling_decision(sample_rate) + else + Logger.warning( + "traces_sampler function returned an invalid sample rate: #{inspect(sample_rate)}" + ) - {:drop, [], tracestate} + make_sampling_decision(0.0) end rescue error -> Logger.warning("traces_sampler function failed: #{inspect(error)}") - tracestate = build_tracestate(0.0, 1.0, false) - {:drop, [], tracestate} + make_sampling_decision(0.0) end end @@ -195,7 +179,7 @@ if Code.ensure_loaded?(:otel_sampler) do defp normalize_sampler_result(true), do: 1.0 defp normalize_sampler_result(false), do: 0.0 - defp normalize_sampler_result(rate) when is_float(rate), do: rate + defp normalize_sampler_result(rate), do: rate defp record_discarded_transaction() do ClientReport.Sender.record_discarded_events(:sample_rate, "transaction") From d3f70487b1fd72c835b1a59eca84dcfdce5df713 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Tue, 17 Jun 2025 06:39:32 +0000 Subject: [PATCH 6/7] Add test for handling invalid traces_sampler return values --- test/sentry/opentelemetry/sampler_test.exs | 25 ++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/sentry/opentelemetry/sampler_test.exs b/test/sentry/opentelemetry/sampler_test.exs index f37594a0d..7cdd8dc6d 100644 --- a/test/sentry/opentelemetry/sampler_test.exs +++ b/test/sentry/opentelemetry/sampler_test.exs @@ -442,6 +442,31 @@ defmodule Sentry.Opentelemetry.SamplerTest do Sampler.should_sample(test_ctx, 123, nil, "test span", nil, %{}, drop: []) end + test "handles invalid traces_sampler return values gracefully" do + test_cases = [ + -0.5, + 1.5, + 2.0, + "invalid", + :invalid, + %{invalid: true}, + [1, 2, 3], + nil + ] + + Enum.each(test_cases, fn invalid_value -> + put_test_config(traces_sampler: fn _ -> invalid_value end) + + test_ctx = create_test_span_context() + + result = Sampler.should_sample(test_ctx, 123, nil, "test span", nil, %{}, drop: []) + + assert {:drop, [], tracestate} = result + assert {"sentry-sample_rate", "0.0"} in tracestate + assert {"sentry-sampled", "false"} in tracestate + end) + end + test "supports MFA tuple for traces_sampler" do defmodule TestSampler do def sample(_sampling_context), do: 0.25 From 36a6b4d85f4fc4a24106d81792bc0cc8b5c1e66c Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Tue, 17 Jun 2025 07:06:16 +0000 Subject: [PATCH 7/7] Add tests for SamplingContext access functions --- test/sentry/opentelemetry/sampler_test.exs | 1 + test/sentry/sampling_context_test.exs | 187 +++++++++++++++++++++ 2 files changed, 188 insertions(+) create mode 100644 test/sentry/sampling_context_test.exs diff --git a/test/sentry/opentelemetry/sampler_test.exs b/test/sentry/opentelemetry/sampler_test.exs index 7cdd8dc6d..3b37c5b05 100644 --- a/test/sentry/opentelemetry/sampler_test.exs +++ b/test/sentry/opentelemetry/sampler_test.exs @@ -3,6 +3,7 @@ defmodule Sentry.Opentelemetry.SamplerTest do alias Sentry.OpenTelemetry.Sampler alias Sentry.ClientReport + alias SamplingContext import Sentry.TestHelpers diff --git a/test/sentry/sampling_context_test.exs b/test/sentry/sampling_context_test.exs new file mode 100644 index 000000000..62f891d2f --- /dev/null +++ b/test/sentry/sampling_context_test.exs @@ -0,0 +1,187 @@ +defmodule Sentry.Opentelemetry.SamplingContextTest do + use Sentry.Case, async: true + + alias SamplingContext + + describe "Access functions" do + test "fetch/2 returns {:ok, value} for existing keys" do + transaction_context = %{ + name: "GET /users", + op: "http.server", + trace_id: 123, + attributes: %{"http.method" => "GET"} + } + + sampling_context = %SamplingContext{ + transaction_context: transaction_context, + parent_sampled: true + } + + assert {:ok, ^transaction_context} = + SamplingContext.fetch(sampling_context, :transaction_context) + + assert {:ok, true} = SamplingContext.fetch(sampling_context, :parent_sampled) + end + + test "fetch/2 returns :error for non-existing keys" do + sampling_context = %SamplingContext{ + transaction_context: %{name: "test", op: "test", trace_id: 123, attributes: %{}}, + parent_sampled: nil + } + + assert :error = SamplingContext.fetch(sampling_context, :non_existing_key) + assert :error = SamplingContext.fetch(sampling_context, :invalid) + end + + test "get_and_update/3 updates existing keys" do + transaction_context = %{ + name: "GET /users", + op: "http.server", + trace_id: 123, + attributes: %{"http.method" => "GET"} + } + + sampling_context = %SamplingContext{ + transaction_context: transaction_context, + parent_sampled: false + } + + update_fun = fn current_value -> + {current_value, !current_value} + end + + {old_value, updated_context} = + SamplingContext.get_and_update(sampling_context, :parent_sampled, update_fun) + + assert old_value == false + assert updated_context.parent_sampled == true + assert updated_context.transaction_context == transaction_context + end + + test "get_and_update/3 handles :pop operation" do + sampling_context = %SamplingContext{ + transaction_context: %{name: "test", op: "test", trace_id: 123, attributes: %{}}, + parent_sampled: true + } + + pop_fun = fn _current_value -> :pop end + + {old_value, updated_context} = + SamplingContext.get_and_update(sampling_context, :parent_sampled, pop_fun) + + assert old_value == true + refute Map.has_key?(updated_context, :parent_sampled) + end + + test "get_and_update/3 works with non-existing keys" do + sampling_context = %SamplingContext{ + transaction_context: %{name: "test", op: "test", trace_id: 123, attributes: %{}}, + parent_sampled: nil + } + + update_fun = fn current_value -> + {current_value, "new_value"} + end + + {old_value, updated_context} = + SamplingContext.get_and_update(sampling_context, :new_key, update_fun) + + assert old_value == nil + assert Map.get(updated_context, :new_key) == "new_value" + end + + test "pop/2 removes existing keys and returns value" do + transaction_context = %{ + name: "POST /api", + op: "http.server", + trace_id: 456, + attributes: %{"http.method" => "POST"} + } + + sampling_context = %SamplingContext{ + transaction_context: transaction_context, + parent_sampled: true + } + + {popped_value, updated_context} = SamplingContext.pop(sampling_context, :parent_sampled) + + assert popped_value == true + refute Map.has_key?(updated_context, :parent_sampled) + assert updated_context.transaction_context == transaction_context + end + + test "pop/2 returns nil for non-existing keys" do + sampling_context = %SamplingContext{ + transaction_context: %{name: "test", op: "test", trace_id: 123, attributes: %{}}, + parent_sampled: nil + } + + {popped_value, updated_context} = SamplingContext.pop(sampling_context, :non_existing_key) + + assert popped_value == nil + assert updated_context == sampling_context + end + + test "Access behavior works with bracket notation" do + transaction_context = %{ + name: "DELETE /resource", + op: "http.server", + trace_id: 789, + attributes: %{"http.method" => "DELETE"} + } + + sampling_context = %SamplingContext{ + transaction_context: transaction_context, + parent_sampled: false + } + + # Test bracket access for reading + assert sampling_context[:transaction_context] == transaction_context + assert sampling_context[:parent_sampled] == false + assert sampling_context[:non_existing] == nil + + # Test get_in/2 + assert get_in(sampling_context, [:transaction_context, :name]) == "DELETE /resource" + + assert get_in(sampling_context, [:transaction_context, :attributes, "http.method"]) == + "DELETE" + end + + test "Access behavior works with put_in/3" do + sampling_context = %SamplingContext{ + transaction_context: %{name: "test", op: "test", trace_id: 123, attributes: %{}}, + parent_sampled: nil + } + + updated_context = put_in(sampling_context[:parent_sampled], true) + + assert updated_context.parent_sampled == true + assert updated_context.transaction_context == sampling_context.transaction_context + end + + test "Access behavior works with update_in/3" do + transaction_context = %{ + name: "PUT /update", + op: "http.server", + trace_id: 999, + attributes: %{"http.method" => "PUT", "http.status_code" => 200} + } + + sampling_context = %SamplingContext{ + transaction_context: transaction_context, + parent_sampled: false + } + + updated_context = + update_in(sampling_context[:transaction_context][:attributes], fn attrs -> + Map.put(attrs, "http.status_code", 404) + end) + + assert get_in(updated_context, [:transaction_context, :attributes, "http.status_code"]) == + 404 + + assert get_in(updated_context, [:transaction_context, :attributes, "http.method"]) == "PUT" + assert updated_context.parent_sampled == false + end + end +end