From 337fea93ab470f2c77ca9a975303bce99b7aa5de Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Fri, 14 Aug 2026 13:31:09 +0000 Subject: [PATCH 1/2] feat(oban): add :should_report_error_check_in_callback for cron check-ins (#963) --- lib/sentry/config.ex | 20 +++ lib/sentry/integrations/oban/cron.ex | 76 +++++++++--- .../config_oban_tags_to_sentry_tags_test.exs | 50 ++++++++ test/sentry/integrations/oban/cron_test.exs | 116 ++++++++++++++++++ .../test/phoenix_app/oban_test.exs | 44 +++++++ 5 files changed, 289 insertions(+), 17 deletions(-) diff --git a/lib/sentry/config.ex b/lib/sentry/config.ex index 646fac293..78ec8d435 100644 --- a/lib/sentry/config.ex +++ b/lib/sentry/config.ex @@ -120,6 +120,26 @@ defmodule Sentry.Config do The function is called with the `Oban.Job` as its arguments and must return a string. This can be used to customize monitor slugs. *Available since v10.8.0*. """ + ], + should_report_error_check_in_callback: [ + type: {:or, [nil, {:fun, 2}]}, + default: nil, + type_doc: "`(Oban.Worker.t() | nil, Oban.Job.t() -> boolean())` or `nil`", + doc: """ + A function that determines whether to report a failed check-in for an Oban cron + job. The function receives the worker module and the `Oban.Job` struct and should + return `true` to report the failed check-in or `false` to skip it. + + ```elixir + should_report_error_check_in_callback: fn _worker, job -> + job.attempt >= job.max_attempts + end + ``` + + This example only reports a failed check-in once all retries are exhausted. While + retries remain the check-in is left open, so the retry that eventually succeeds + closes the same check-in. *Available since v13.5.0*. + """ ] ] ] diff --git a/lib/sentry/integrations/oban/cron.ex b/lib/sentry/integrations/oban/cron.ex index 4825112cc..700161b13 100644 --- a/lib/sentry/integrations/oban/cron.ex +++ b/lib/sentry/integrations/oban/cron.ex @@ -6,6 +6,7 @@ defmodule Sentry.Integrations.Oban.Cron do @moduledoc since: "10.9.0" alias Sentry.Integrations.CheckInIDMappings + alias Sentry.LoggerUtils @doc """ The Oban integration calls this callback (if present) to customize @@ -63,27 +64,68 @@ defmodule Sentry.Integrations.Oban.Cron do end defp handle_oban_job_event(:stop, measurements, metadata, config) do - if opts = job_to_check_in_opts(metadata.job, config) do - status = - case metadata.state do - :success -> :ok - :failure -> :error - :cancelled -> :ok - :discard -> :ok - :snoozed -> :ok - end + status = + case metadata.state do + :success -> :ok + :failure -> :error + :cancelled -> :ok + :discard -> :ok + :snoozed -> :ok + end - opts - |> Keyword.merge(status: status, duration: duration_in_seconds(measurements)) - |> Sentry.capture_check_in() - end + maybe_capture_check_in(status, measurements, metadata, config) end defp handle_oban_job_event(:exception, measurements, metadata, config) do - if opts = job_to_check_in_opts(metadata.job, config) do - opts - |> Keyword.merge(status: :error, duration: duration_in_seconds(measurements)) - |> Sentry.capture_check_in() + maybe_capture_check_in(:error, measurements, metadata, config) + end + + defp maybe_capture_check_in(status, measurements, metadata, config) do + if status != :error or should_report_error_check_in?(metadata.job, config) do + if opts = job_to_check_in_opts(metadata.job, config) do + opts + |> Keyword.merge(status: status, duration: duration_in_seconds(measurements)) + |> Sentry.capture_check_in() + end + end + end + + defp should_report_error_check_in?(job, config) do + case Keyword.get(config, :should_report_error_check_in_callback) do + callback when is_function(callback, 2) -> + call_should_report_error_check_in_callback(callback, job) + + _ -> + true + end + end + + defp call_should_report_error_check_in_callback(callback, job) do + worker = + case apply(Oban.Worker, :from_string, [job.worker]) do + {:ok, mod} -> + mod + + {:error, _} -> + LoggerUtils.warning( + "Could not resolve Oban worker module from string: #{inspect(job.worker)}" + ) + + nil + end + + try do + callback.(worker, job) == true + rescue + error -> + LoggerUtils.warning(""" + :should_report_error_check_in_callback failed for worker #{inspect(worker)} \ + (job ID #{job.id}): + + #{Exception.format(:error, error, __STACKTRACE__)}\ + """) + + true end end diff --git a/test/sentry/config_oban_tags_to_sentry_tags_test.exs b/test/sentry/config_oban_tags_to_sentry_tags_test.exs index e9bcdb755..2ea22e438 100644 --- a/test/sentry/config_oban_tags_to_sentry_tags_test.exs +++ b/test/sentry/config_oban_tags_to_sentry_tags_test.exs @@ -94,4 +94,54 @@ defmodule Sentry.ConfigObanTagsToSentryTagsTest do end end end + + describe "should_report_error_check_in_callback configuration validation" do + test "accepts nil" do + assert :ok = + put_test_config( + integrations: [oban: [cron: [should_report_error_check_in_callback: nil]]] + ) + + assert Sentry.Config.integrations()[:oban][:cron][:should_report_error_check_in_callback] == + nil + end + + test "accepts function with arity 2" do + fun = fn _worker, _job -> true end + + assert :ok = + put_test_config( + integrations: [oban: [cron: [should_report_error_check_in_callback: fun]]] + ) + + assert Sentry.Config.integrations()[:oban][:cron][:should_report_error_check_in_callback] == + fun + end + + test "rejects function with wrong arity" do + fun = fn _job -> true end + + assert_raise ArgumentError, + ~r/invalid value for :should_report_error_check_in_callback/, + fn -> + put_test_config( + integrations: [oban: [cron: [should_report_error_check_in_callback: fun]]] + ) + end + end + + test "rejects invalid types" do + for invalid <- ["invalid", 123, []] do + assert_raise ArgumentError, + ~r/invalid value for :should_report_error_check_in_callback/, + fn -> + put_test_config( + integrations: [ + oban: [cron: [should_report_error_check_in_callback: invalid]] + ] + ) + end + end + end + end end diff --git a/test/sentry/integrations/oban/cron_test.exs b/test/sentry/integrations/oban/cron_test.exs index d3e369e5b..974648545 100644 --- a/test/sentry/integrations/oban/cron_test.exs +++ b/test/sentry/integrations/oban/cron_test.exs @@ -2,11 +2,19 @@ defmodule Sentry.Integrations.Oban.CronTest do alias Sentry.Integrations.CheckInIDMappings use Sentry.Case, async: false + import ExUnit.CaptureLog import Sentry.Test.Assertions import Sentry.TestHelpers alias Sentry.Test, as: SentryTest + defmodule MyCronWorker do + use Oban.Worker + + @impl Oban.Worker + def perform(_job), do: :ok + end + setup context do opts = context[:attach_opts] || [] @@ -284,6 +292,114 @@ defmodule Sentry.Integrations.Oban.CronTest do ) end + describe "should_report_error_check_in_callback" do + test "should not report a failed check-in when the callback returns false", %{ref: ref} do + attach_with_callback(fn _worker, _job -> false end) + + execute_exception_event() + + refute_sentry_check_in(ref) + end + + test "should report a failed check-in when the callback returns true", %{ref: ref} do + attach_with_callback(fn _worker, _job -> true end) + + execute_exception_event() + + [check_in_body] = SentryTest.collect_sentry_check_ins(ref, 1) + assert_sentry_report(check_in_body, status: "error") + end + + test "should pass the worker module and the job to the callback", %{ref: ref} do + test_pid = self() + + attach_with_callback(fn worker, job -> + send(test_pid, {:callback_args, worker, job}) + false + end) + + execute_exception_event() + + assert_receive {:callback_args, MyCronWorker, %Oban.Job{id: 942}} + refute_sentry_check_in(ref) + end + + test "should report the failed check-in when the callback raises", %{ref: ref} do + log = + capture_log([metadata: [:domain]], fn -> + attach_with_callback(fn _worker, _job -> raise "callback error" end) + + execute_exception_event() + + [check_in_body] = SentryTest.collect_sentry_check_ins(ref, 1) + assert_sentry_report(check_in_body, status: "error") + end) + + assert log =~ ":should_report_error_check_in_callback failed" + assert log =~ "callback error" + assert log =~ ~r/domain=(\w+\.)*sentry/ + end + + test "should pass a nil worker to the callback when the worker cannot be resolved", %{ + ref: ref + } do + test_pid = self() + + log = + capture_log([metadata: [:domain]], fn -> + attach_with_callback(fn worker, _job -> + send(test_pid, {:callback_worker, worker}) + false + end) + + execute_exception_event(worker: "NotA.Real.Worker") + + refute_sentry_check_in(ref) + end) + + assert_receive {:callback_worker, nil} + assert log =~ ~s(Could not resolve Oban worker module from string: "NotA.Real.Worker") + assert log =~ ~r/domain=(\w+\.)*sentry/ + end + + test "should still report the in-progress check-in when the callback returns false", %{ + ref: ref + } do + attach_with_callback(fn _worker, _job -> false end) + + :telemetry.execute([:oban, :job, :start], %{}, %{job: cron_job()}) + + [check_in_body] = SentryTest.collect_sentry_check_ins(ref, 1) + assert_sentry_report(check_in_body, status: "in_progress") + end + end + + defp attach_with_callback(callback) do + :telemetry.detach(Sentry.Integrations.Oban.Cron) + + Sentry.Integrations.Oban.Cron.attach_telemetry_handler( + should_report_error_check_in_callback: callback + ) + end + + defp execute_exception_event(job_overrides \\ []) do + :telemetry.execute([:oban, :job, :exception], %{duration: 0}, %{ + state: :failure, + job: cron_job(job_overrides) + }) + end + + defp cron_job(overrides \\ []) do + struct!( + %Oban.Job{ + worker: inspect(MyCronWorker), + id: 942, + meta: %{"cron" => true, "cron_expr" => "@daily"} + }, + overrides + ) + end + def custom_name_generator(%Oban.Job{worker: "Sentry.ClientWorker", args: %{"client" => client}}) do "Sentry.ClientWorker.#{client}" end diff --git a/test_integrations/phoenix_app/test/phoenix_app/oban_test.exs b/test_integrations/phoenix_app/test/phoenix_app/oban_test.exs index e4333edc5..7ac41c55f 100644 --- a/test_integrations/phoenix_app/test/phoenix_app/oban_test.exs +++ b/test_integrations/phoenix_app/test/phoenix_app/oban_test.exs @@ -8,6 +8,7 @@ defmodule Sentry.Integrations.Phoenix.ObanTest do require OpenTelemetry.Tracer + alias Sentry.Integrations.Oban.Cron alias Sentry.Integrations.Oban.ErrorReporter setup do @@ -34,6 +35,20 @@ defmodule Sentry.Integrations.Phoenix.ObanTest do def perform(_job), do: :ok end + defmodule FlakyCronWorker do + use Oban.Worker, max_attempts: 3 + + @impl Oban.Worker + def perform(%Oban.Job{attempt: 1}) do + raise "intentional failure for testing" + end + + def perform(_job), do: :ok + + @impl Oban.Worker + def backoff(_job), do: 0 + end + defmodule WorkerWithDatabaseQuery do use Oban.Worker @@ -379,6 +394,35 @@ defmodule Sentry.Integrations.Phoenix.ObanTest do end end + describe "should_report_error_check_in_callback config" do + setup %{bypass: bypass} do + on_exit(fn -> :telemetry.detach(Cron) end) + + %{ref: Sentry.Test.setup_bypass_envelope_collector(bypass, type: "check_in")} + end + + test "should not report a failed check-in when a cron job succeeds on retry", %{ref: ref} do + Cron.attach_telemetry_handler( + should_report_error_check_in_callback: fn _worker, job -> + job.attempt >= job.max_attempts + end + ) + + {:ok, _job} = + %{} + |> FlakyCronWorker.new(meta: %{"cron" => true, "cron_expr" => "@daily"}) + |> Oban.insert() + + assert %{failure: 1, success: 1} = + Oban.drain_queue(queue: :default, with_recursion: true, with_scheduled: true) + + check_ins = Sentry.Test.collect_sentry_check_ins(ref, 4, timeout: 200) + + assert Enum.map(check_ins, & &1["status"]) == ["in_progress", "in_progress", "ok"] + assert [_] = check_ins |> Enum.map(& &1["check_in_id"]) |> Enum.uniq() + end + end + describe "allow_sentry_reports/2 with a real Oban worker process" do setup do Sentry.Test.setup_sentry() From 7b64134a27e3ec5d17066584b2a7e86b0cb9c1b0 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Fri, 14 Aug 2026 13:33:33 +0000 Subject: [PATCH 2/2] fix(oban): keep the cron check-in open when a job is snoozed --- lib/sentry/integrations/oban/cron.ex | 7 ++++++- test/sentry/integrations/oban/cron_test.exs | 16 ++++++++++++++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/lib/sentry/integrations/oban/cron.ex b/lib/sentry/integrations/oban/cron.ex index 700161b13..4f91e5412 100644 --- a/lib/sentry/integrations/oban/cron.ex +++ b/lib/sentry/integrations/oban/cron.ex @@ -63,6 +63,12 @@ defmodule Sentry.Integrations.Oban.Cron do end end + # A snoozed job is rescheduled rather than finished, so its check-in stays open + # until the run that actually completes closes it. + defp handle_oban_job_event(:stop, _measurements, %{state: :snoozed}, _config) do + :ok + end + defp handle_oban_job_event(:stop, measurements, metadata, config) do status = case metadata.state do @@ -70,7 +76,6 @@ defmodule Sentry.Integrations.Oban.Cron do :failure -> :error :cancelled -> :ok :discard -> :ok - :snoozed -> :ok end maybe_capture_check_in(status, measurements, metadata, config) diff --git a/test/sentry/integrations/oban/cron_test.exs b/test/sentry/integrations/oban/cron_test.exs index 974648545..43d19b311 100644 --- a/test/sentry/integrations/oban/cron_test.exs +++ b/test/sentry/integrations/oban/cron_test.exs @@ -109,8 +109,7 @@ defmodule Sentry.Integrations.Oban.CronTest do success: "ok", failure: "error", cancelled: "ok", - discard: "ok", - snoozed: "ok" + discard: "ok" ], {frequency, expected_unit} <- [ {"@hourly", "hour"}, @@ -154,6 +153,19 @@ defmodule Sentry.Integrations.Oban.CronTest do end end + test "should not report a check-in when a job is snoozed", %{ref: ref} do + :telemetry.execute([:oban, :job, :stop], %{duration: 0}, %{ + state: :snoozed, + job: %Oban.Job{ + worker: "Sentry.MyWorker", + id: 942, + meta: %{"cron" => true, "cron_expr" => "@daily"} + } + }) + + refute_sentry_check_in(ref) + end + test "captures exception events with monitor config", %{ref: ref} do duration = System.convert_time_unit(12_099, :millisecond, :native)