-
-
Notifications
You must be signed in to change notification settings - Fork 207
Support for traces_sampler option #910
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
27bfc44
fb3bced
8b597f2
bf32449
e616c21
d3f7048
36a6b4d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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} | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
| } | ||
solnic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| 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 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 <https://develop.sentry.dev/sdk/telemetry/traces/#sampling-context>. | ||
| """ | ||
|
|
||
| @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 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the
traces_samplerhas to be invoked only for root spans, seehttps://github.com/getsentry/sentry-python/blob/c21525e4252805561d83cd2d726020dd41aa074d/sentry_sdk/opentelemetry/sampler.py#L246
so in this case I think the
:inheritcase shouldn't exist but only the:no_tracecase below should invoke thetraces_sampler