From 2b6d68e7fc2bac5edf534e1fa9917c11fbeb5080 Mon Sep 17 00:00:00 2001 From: Andrea Leopardi Date: Tue, 3 Sep 2024 16:35:06 +0200 Subject: [PATCH] Add :monitor_config_defaults integrations option (#782) --- lib/sentry/application.ex | 2 +- lib/sentry/check_in.ex | 6 ++- lib/sentry/config.ex | 13 ++++++ lib/sentry/integrations/oban/cron.ex | 44 +++++++++++--------- test/sentry/integrations/oban/cron_test.exs | 45 +++++++++++++++++++++ 5 files changed, 88 insertions(+), 22 deletions(-) diff --git a/lib/sentry/application.ex b/lib/sentry/application.ex index bc269c79..ffa321d2 100644 --- a/lib/sentry/application.ex +++ b/lib/sentry/application.ex @@ -19,7 +19,7 @@ defmodule Sentry.Application do [] end - integrations_config = Keyword.fetch!(config, :integrations) + integrations_config = Config.integrations() children = [ diff --git a/lib/sentry/check_in.ex b/lib/sentry/check_in.ex index b9d796ce..f17db67f 100644 --- a/lib/sentry/check_in.ex +++ b/lib/sentry/check_in.ex @@ -115,7 +115,11 @@ defmodule Sentry.CheckIn do """ ], monitor_config: [ - doc: "If you pass this optional option, you **must** pass the nested `:schedule` option.", + doc: """ + If you pass this optional option, you **must** pass the nested `:schedule` option. The + options below are described in detail in the [Sentry + documentation](https://develop.sentry.dev/sdk/telemetry/check-ins/#monitor-upsert-support). + """, type: :keyword_list, keys: [ checkin_margin: number_schema_opts, diff --git a/lib/sentry/config.ex b/lib/sentry/config.ex index ee39b6c7..7a8d89c4 100644 --- a/lib/sentry/config.ex +++ b/lib/sentry/config.ex @@ -17,6 +17,16 @@ defmodule Sentry.Config do *Available since 10.6.3*. """ ], + monitor_config_defaults: [ + type: :keyword_list, + default: [], + doc: """ + Defaults to be used for the `monitor_config` when reporting cron jobs with one of the + integrations. This supports all the keys defined in the [Sentry + documentation](https://develop.sentry.dev/sdk/telemetry/check-ins/#monitor-upsert-support). + See also `Sentry.CheckIn.new/1`. *Available since v10.8.0*. + """ + ], oban: [ type: :keyword_list, doc: """ @@ -555,6 +565,9 @@ defmodule Sentry.Config do @spec test_mode?() :: boolean() def test_mode?, do: fetch!(:test_mode) + @spec integrations() :: keyword() + def integrations, do: fetch!(:integrations) + @spec put_config(atom(), term()) :: :ok def put_config(key, value) when is_atom(key) do unless key in @valid_keys do diff --git a/lib/sentry/integrations/oban/cron.ex b/lib/sentry/integrations/oban/cron.ex index 2ea50e72..4f67c23d 100644 --- a/lib/sentry/integrations/oban/cron.ex +++ b/lib/sentry/integrations/oban/cron.ex @@ -69,31 +69,35 @@ defmodule Sentry.Integrations.Oban.Cron do end defp job_to_check_in_opts(job) when is_struct(job, Oban.Job) do - if schedule_opts = schedule_opts(job) do - id = CheckInIDMappings.lookup_or_insert_new(job.id) - - [ - check_in_id: id, - # This is already a binary. - monitor_slug: slugify(job.worker), - monitor_config: [schedule: schedule_opts] - ] - else - nil + monitor_config_opts = Sentry.Config.integrations()[:monitor_config_defaults] + + case Keyword.merge(monitor_config_opts, schedule_opts(job)) do + [] -> + nil + + monitor_config_opts -> + id = CheckInIDMappings.lookup_or_insert_new(job.id) + + [ + check_in_id: id, + # This is already a binary. + monitor_slug: slugify(job.worker), + monitor_config: monitor_config_opts + ] end end defp schedule_opts(%{meta: meta} = job) when is_struct(job, Oban.Job) do case meta["cron_expr"] do - "@hourly" -> [type: :interval, value: 1, unit: :hour] - "@daily" -> [type: :interval, value: 1, unit: :day] - "@weekly" -> [type: :interval, value: 1, unit: :week] - "@monthly" -> [type: :interval, value: 1, unit: :month] - "@yearly" -> [type: :interval, value: 1, unit: :year] - "@annually" -> [type: :interval, value: 1, unit: :year] - "@reboot" -> nil - cron_expr when is_binary(cron_expr) -> [type: :crontab, value: cron_expr] - _other -> nil + "@hourly" -> [schedule: [type: :interval, value: 1, unit: :hour]] + "@daily" -> [schedule: [type: :interval, value: 1, unit: :day]] + "@weekly" -> [schedule: [type: :interval, value: 1, unit: :week]] + "@monthly" -> [schedule: [type: :interval, value: 1, unit: :month]] + "@yearly" -> [schedule: [type: :interval, value: 1, unit: :year]] + "@annually" -> [schedule: [type: :interval, value: 1, unit: :year]] + "@reboot" -> [] + cron_expr when is_binary(cron_expr) -> [schedule: [type: :crontab, value: cron_expr]] + _other -> [] end end diff --git a/test/sentry/integrations/oban/cron_test.exs b/test/sentry/integrations/oban/cron_test.exs index fc75a88e..44c7f67e 100644 --- a/test/sentry/integrations/oban/cron_test.exs +++ b/test/sentry/integrations/oban/cron_test.exs @@ -194,4 +194,49 @@ defmodule Sentry.Integrations.Oban.CronTest do assert_receive {^ref, :done}, 1000 end + + test "uses default monitor configuration in Sentry's config if present", %{bypass: bypass} do + put_test_config( + integrations: [ + monitor_config_defaults: [ + checkin_margin: 10, + max_runtime: 42, + failure_issue_threshold: 84 + ] + ] + ) + + test_pid = self() + ref = make_ref() + + Bypass.expect_once(bypass, "POST", "/api/1/envelope/", fn conn -> + {:ok, body, conn} = Plug.Conn.read_body(conn) + assert [{_headers, check_in_body}] = decode_envelope!(body) + + assert check_in_body["monitor_config"] == %{ + "checkin_margin" => 10, + "failure_issue_threshold" => 84, + "max_runtime" => 42, + "schedule" => %{ + "type" => "crontab", + "value" => "* 1 1 1 1" + } + } + + send(test_pid, {ref, :done}) + + Plug.Conn.send_resp(conn, 200, ~s<{"id": "1923"}>) + end) + + :telemetry.execute([:oban, :job, :exception], %{duration: 0}, %{ + state: :success, + job: %Oban.Job{ + worker: "Sentry.MyWorker", + id: 942, + meta: %{"cron" => true, "cron_expr" => "* 1 1 1 1"} + } + }) + + assert_receive {^ref, :done}, 1000 + end end