SpanProcessor with better Sampler (#902)

* Revert "Revert "Add SpanProcessor for OpenTelemetry (#875)""

This reverts commit 2ced90e.

* Better span sampler (#903)

* Better span sampler

This updates OpenTelemetry.Sampler so that it uses
`traces_sample_rate` for sampling spans before they
get sent to the span processor.

This way we're not processing spans when they are
dropped during sampling, which was previously the
case as the Client was responsible for sampling
just before attemping to send a transaction.

* Enhance sampling logic to record discarded transactions

* Clarify trace-level sampling decision-making

Previously it incorrectly referred to "parent" where
in reality we're dealing with spans from the same
trace and respect sampling decisions that were
already made for a given trace.

This is not the same thing as parent/child spans
that we're handling in the SpanProcessor as this
type of relationship is established post-sampling.

* Remove unnecessary sleep calls in sampler tests

* Fix flaky test

* Clean up make_sampling_decision

* Simplify client reports for dropped transactions

* Update traces_sample_rate default value to be nil

* Account for `nil` sample_rate when sampling

* Use put_test_config

* Update traces_sample_rate spec to allow nil value

* Support for traces_sampler option (#910)

* Add support for `traces_sampler` config option

* Update docs

* Remove unnecessary attribute merging

* Call traces_sampler only for root spans

* Reuse sampling decision logic

* Add test for handling invalid traces_sampler return values

* Add tests for SamplingContext access functions

* Add basic integration tests for OpenTelemetry (#912)

* Update CHANGELOG

Author: solnic

.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}
 ]

CHANGELOG.md

@@ -1,8 +1,44 @@
 ## Unreleased
 
-- Tweak credit card regex handling for OTP-28 ([#898](https://github.com/getsentry/sentry-elixir/pull/898))
+This release comes with a beta support for Traces using OpenTelemetry - please test it out and report any issues you find.
+
+### New features
+
+- Beta support for Traces using OpenTelemetry ([#902](https://github.com/getsentry/sentry-elixir/pull/902))
+
+  To enable Tracing in your Phoenix application, you need to add the following to your `mix.exs`:
+
+  ```elixir
+  def deps do
+    [
+      # ...
+      {:sentry, "~> 11.0.0"},
+      {:opentelemetry, "~> 1.5"},
+      {:opentelemetry_api, "~> 1.4"},
+      {:opentelemetry_exporter, "~> 1.0"},
+      {:opentelemetry_semantic_conventions, "~> 1.27"},
+      {:opentelemetry_phoenix, "~> 2.0"},
+      {:opentelemetry_ecto, "~> 1.2"},
+      # ...
+    ]
+  ```
+
+  And then configure Tracing in Sentry and OpenTelemetry in your `config.exs`:
+
+  ```elixir
+  config :sentry,
+    # ...
+    traces_sample_rate: 1.0 # any value between 0 and 1.0 enables tracing
+
+  config :opentelemetry, span_processor: {Sentry.OpenTelemetry.SpanProcessor, []}
+  config :opentelemetry, sampler: {Sentry.OpenTelemetry.Sampler, []}
+  ```
 - Add installer (based on Igniter) ([#876](https://github.com/getsentry/sentry-elixir/pull/876))
 
+### Various improvements
+
+- Tweak credit card regex handling for OTP-28 ([#898](https://github.com/getsentry/sentry-elixir/pull/898))
+
 # Changelog
 
 ## 10.10.0

config/config.exs

@@ -10,9 +10,15 @@ if config_env() == :test do
     send_result: :sync,
     send_max_attempts: 1,
     dedup_events: false,
-    test_mode: true
+    test_mode: true,
+    traces_sample_rate: 1.0
 
   config :logger, backends: []
+
+  config :opentelemetry, span_processor: {Sentry.OpenTelemetry.SpanProcessor, []}
+
+  config :opentelemetry,
+    sampler: {Sentry.OpenTelemetry.Sampler, [drop: ["Elixir.Oban.Stager process"]]}
 end
 
 config :phoenix, :json_library, if(Code.ensure_loaded?(JSON), do: JSON, else: Jason)

lib/sentry/application.ex

@@ -26,6 +26,13 @@ defmodule Sentry.Application do
 
     integrations_config = Config.integrations()
 
+    maybe_span_storage =
+      if Config.tracing?() do
+        [Sentry.OpenTelemetry.SpanStorage]
+      else
+        []
+      end
+
     children =
       [
         {Registry, keys: :unique, name: Sentry.Transport.SenderRegistry},
@@ -39,6 +46,7 @@ defmodule Sentry.Application do
          ]}
       ] ++
         maybe_http_client_spec ++
+        maybe_span_storage ++
         [Sentry.Transport.SenderPool]
 
     cache_loaded_applications()

lib/sentry/client.ex

@@ -110,14 +110,12 @@ defmodule Sentry.Client do
   @spec send_transaction(Transaction.t(), keyword()) ::
           {:ok, transaction_id :: String.t()}
           | {:error, ClientError.t()}
-          | :unsampled
           | :excluded
   def send_transaction(%Transaction{} = transaction, opts \\ []) do
     opts = NimbleOptions.validate!(opts, Options.send_transaction_schema())
 
     result_type = Keyword.get_lazy(opts, :result, &Config.send_result/0)
     client = Keyword.get_lazy(opts, :client, &Config.client/0)
-    sample_rate = Keyword.get_lazy(opts, :sample_rate, &Config.sample_rate/0)
     before_send = Keyword.get_lazy(opts, :before_send, &Config.before_send/0)
     after_send_event = Keyword.get_lazy(opts, :after_send_event, &Config.after_send_event/0)
 
@@ -126,16 +124,11 @@ defmodule Sentry.Client do
         Application.get_env(:sentry, :request_retries, Transport.default_retries())
       end)
 
-    with :ok <- sample_event(sample_rate),
-         {:ok, %Transaction{} = transaction} <- maybe_call_before_send(transaction, before_send) do
+    with {:ok, %Transaction{} = transaction} <- maybe_call_before_send(transaction, before_send) do
       send_result = encode_and_send(transaction, result_type, client, request_retries)
       _ignored = maybe_call_after_send(transaction, send_result, after_send_event)
       send_result
     else
-      :unsampled ->
-        ClientReport.Sender.record_discarded_events(:sample_rate, [transaction])
-        :unsampled
-
       :excluded ->
         :excluded
     end

lib/sentry/client_report/sender.ex

@@ -17,14 +17,21 @@ defmodule Sentry.ClientReport.Sender do
     GenServer.start_link(__MODULE__, nil, name: Keyword.get(opts, :name, __MODULE__))
   end
 
+  def record_discarded_events(reason, info, genserver \\ __MODULE__)
+
+  @spec record_discarded_events(atom(), String.t(), GenServer.server()) :: :ok
+  def record_discarded_events(reason, data_category, genserver)
+      when is_binary(data_category) do
+    GenServer.cast(genserver, {:record_discarded_events, reason, data_category})
+  end
+
   @spec record_discarded_events(atom(), [item], GenServer.server()) :: :ok
         when item:
                Sentry.Attachment.t()
                | Sentry.CheckIn.t()
                | ClientReport.t()
                | Sentry.Event.t()
-               | Sentry.Transaction.t()
-  def record_discarded_events(reason, event_items, genserver \\ __MODULE__)
+  def record_discarded_events(reason, event_items, genserver)
       when is_list(event_items) do
     # We silently ignore events whose reasons aren't valid because we have to add it to the allowlist in Snuba
     # https://develop.sentry.dev/sdk/client-reports/

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,
@@ -143,6 +150,49 @@ defmodule Sentry.Config do
       be used as the value for this option.
       """
     ],
+    traces_sample_rate: [
+      type: {:custom, __MODULE__, :__validate_traces_sample_rate__, []},
+      default: nil,
+      doc: """
+      The sample rate for transaction events. A value between `0.0` and `1.0` (inclusive).
+      A value of `0.0` means no transactions will be sampled, while `1.0` means all transactions
+      will be sampled.
+
+      This value is also used to determine if tracing is enabled: if it's not `nil`, tracing is enabled.
+
+      Tracing requires OpenTelemetry packages to work. See [the
+      OpenTelemetry setup documentation](https://opentelemetry.io/docs/languages/erlang/getting-started/)
+      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 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 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.
+
+      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.",
@@ -607,6 +657,12 @@ defmodule Sentry.Config do
   @spec sample_rate() :: float()
   def sample_rate, do: fetch!(:sample_rate)
 
+  @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)
 
@@ -644,6 +700,9 @@ defmodule Sentry.Config do
   @spec integrations() :: keyword()
   def integrations, do: fetch!(:integrations)
 
+  @spec tracing?() :: boolean()
+  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
     unless key in @valid_keys do
@@ -743,6 +802,35 @@ defmodule Sentry.Config do
     end
   end
 
+  def __validate_traces_sample_rate__(value) do
+    if is_nil(value) or (is_float(value) and value >= 0.0 and value <= 1.0) do
+      {:ok, value}
+    else
+      {:error,
+       "expected :traces_sample_rate to be nil or a value between 0.0 and 1.0 (included), got: #{inspect(value)}"}
+    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