Add support for :skip_retries to Oban.ErrorReporter

getsentry · Dec 11, 2024 · 22ea6bf · 22ea6bf
1 parent 7414c9d
commit 22ea6bf
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 17 deletions.
diff --git a/lib/sentry/application.ex b/lib/sentry/application.ex
@@ -64,7 +64,9 @@ defmodule Sentry.Application do
     end
 
     if config[:oban][:capture_errors] do
-      Sentry.Integrations.Oban.ErrorReporter.attach()
+      Sentry.Integrations.Oban.ErrorReporter.attach_telemetry_handler(
+        Keyword.get(config[:oban], :error_reporter, [])
+      )
     end
 
     if config[:quantum][:cron][:enabled] do

diff --git a/lib/sentry/integrations/oban/error_reporter.ex b/lib/sentry/integrations/oban/error_reporter.ex
@@ -4,14 +4,14 @@ defmodule Sentry.Integrations.Oban.ErrorReporter do
   # See this blog post:
   # https://getoban.pro/articles/enhancing-error-reporting
 
-  @spec attach() :: :ok
-  def attach do
+  @spec attach_telemetry_handler(keyword()) :: :ok
+  def attach_telemetry_handler(config) do
     _ =
       :telemetry.attach(
         __MODULE__,
         [:oban, :job, :exception],
         &__MODULE__.handle_event/4,
-        :no_config
+        config
       )
 
     :ok
@@ -21,9 +21,24 @@ defmodule Sentry.Integrations.Oban.ErrorReporter do
           [atom(), ...],
           term(),
           %{required(:job) => struct(), optional(term()) => term()},
-          :no_config
+          keyword()
         ) :: :ok
-  def handle_event([:oban, :job, :exception], _measurements, %{job: job} = _metadata, :no_config) do
+  def handle_event([:oban, :job, :exception], _measurements, %{job: job} = _metadata, config) do
+    if should_report?(job, config) do
+      do_report(job)
+    else
+      :ok
+    end
+  end
+
+  defp should_report?(job, config) do
+    case Keyword.get(config, :skip_retries) do
+      true -> job.attempt == job.max_attempts
+      _ -> true
+    end
+  end
+
+  defp do_report(job) do
     %{reason: reason, stacktrace: stacktrace} = job.unsaved_error
 
     stacktrace =
@@ -39,15 +54,13 @@ defmodule Sentry.Integrations.Oban.ErrorReporter do
         [inspect(reason)]
       end
 
-    opts =
-      [
-        stacktrace: stacktrace,
-        tags: %{oban_worker: job.worker, oban_queue: job.queue, oban_state: job.state},
-        fingerprint: [inspect(job.worker)] ++ fingerprint_opts,
-        extra:
-          Map.take(job, [:args, :attempt, :id, :max_attempts, :meta, :queue, :tags, :worker]),
-        integration_meta: %{oban: %{job: job}}
-      ]
+    opts = [
+      stacktrace: stacktrace,
+      tags: %{oban_worker: job.worker, oban_queue: job.queue, oban_state: job.state},
+      fingerprint: [inspect(job.worker)] ++ fingerprint_opts,
+      extra: Map.take(job, [:args, :attempt, :id, :max_attempts, :meta, :queue, :tags, :worker]),
+      integration_meta: %{oban: %{job: job}}
+    ]
 
     _ =
       if is_exception(reason) do

diff --git a/test/sentry/integrations/oban/error_reporter_test.exs b/test/sentry/integrations/oban/error_reporter_test.exs
@@ -30,7 +30,7 @@ defmodule Sentry.Integrations.Oban.ErrorReporterTest do
                  [:oban, :job, :exception],
                  %{},
                  %{job: job},
-                 :no_config
+                 []
                )
 
       assert [event] = Sentry.Test.pop_sentry_reports()
@@ -69,7 +69,7 @@ defmodule Sentry.Integrations.Oban.ErrorReporterTest do
                  [:oban, :job, :exception],
                  %{},
                  %{job: job},
-                 :no_config
+                 []
                )
 
       assert [event] = Sentry.Test.pop_sentry_reports()
@@ -94,5 +94,45 @@ defmodule Sentry.Integrations.Oban.ErrorReporterTest do
       assert event.tags.oban_state == "available"
       assert event.tags.oban_worker == "Sentry.Integrations.Oban.ErrorReporterTest.MyWorker"
     end
+
+    test "with skip_retries: true, only reports final attempt failures" do
+      job =
+        %{"id" => "123", "entity" => "user", "type" => "delete"}
+        |> MyWorker.new()
+        |> Ecto.Changeset.apply_action!(:validate)
+        |> Map.replace!(:unsaved_error, %{
+          reason: %RuntimeError{message: "oops"},
+          kind: :error,
+          stacktrace: []
+        })
+
+      Sentry.Test.start_collecting()
+
+      job_attempt_1 = Map.merge(job, %{attempt: 1, max_attempts: 3})
+
+      assert :ok =
+               ErrorReporter.handle_event(
+                 [:oban, :job, :exception],
+                 %{},
+                 %{job: job_attempt_1},
+                 skip_retries: true
+               )
+
+      assert [] = Sentry.Test.pop_sentry_reports()
+
+      job_attempt_3 = Map.merge(job, %{attempt: 3, max_attempts: 3})
+
+      assert :ok =
+               ErrorReporter.handle_event(
+                 [:oban, :job, :exception],
+                 %{},
+                 %{job: job_attempt_3},
+                 skip_retries: true
+               )
+
+      assert [event] = Sentry.Test.pop_sentry_reports()
+      assert event.original_exception == %RuntimeError{message: "oops"}
+      assert event.tags.oban_worker == "Sentry.Integrations.Oban.ErrorReporterTest.MyWorker"
+    end
   end
 end