FIx monitor slug in Oban + Quantum integrations (#721)

Co-authored-by: Milton Mazzarri <[email protected]>

Author: whatyouhide

lib/sentry/config.ex

@@ -5,7 +5,8 @@ defmodule Sentry.Config do
     oban: [
       type: :keyword_list,
       doc: """
-      Configuration for the [Oban](https://github.com/sorentwo/oban) integration. *Available
+      Configuration for the [Oban](https://github.com/sorentwo/oban) integration. The Oban
+      integration requires at minumum Oban Pro v0.14 or Oban v.2.17.6. *Available
       since v10.2.0*.
       """,
       keys: [

lib/sentry/integrations/oban/cron.ex

@@ -72,7 +72,7 @@ defmodule Sentry.Integrations.Oban.Cron do
       [
         check_in_id: "oban-#{job.id}",
         # This is already a binary.
-        monitor_slug: job.worker,
+        monitor_slug: slugify(job.worker),
         monitor_config: [schedule: schedule_opts]
       ]
     else
@@ -99,4 +99,12 @@ defmodule Sentry.Integrations.Oban.Cron do
     |> System.convert_time_unit(:native, :millisecond)
     |> Kernel./(1000)
   end
+
+  # MyApp.SomeWorker -> "my-app-some-worker"
+  defp slugify(worker_name) do
+    worker_name
+    |> String.split(".")
+    |> Enum.map_join("-", &(&1 |> Macro.underscore() |> String.replace("_", "-")))
+    |> String.slice(0, 50)
+  end
 end

lib/sentry/integrations/quantum/cron.ex

@@ -65,7 +65,7 @@ defmodule Sentry.Integrations.Quantum.Cron do
       [
         check_in_id: "quantum-#{id}",
         # This is already a binary.
-        monitor_slug: "quantum-#{inspect(job.name)}",
+        monitor_slug: "quantum-#{slugify(job.name)}",
         monitor_config: [schedule: schedule_opts]
       ]
     else
@@ -92,4 +92,18 @@ defmodule Sentry.Integrations.Quantum.Cron do
     |> System.convert_time_unit(:native, :millisecond)
     |> Kernel./(1000)
   end
+
+  defp slugify(job_name) when is_atom(job_name) do
+    case Atom.to_string(job_name) do
+      "Elixir." <> module -> module |> Macro.underscore() |> slugify()
+      other -> slugify(other)
+    end
+  end
+
+  defp slugify(job_name) when is_binary(job_name) do
+    job_name
+    |> String.downcase()
+    |> String.replace(["_", "#", "<", ">", ".", " ", "/"], "-")
+    |> String.slice(0, 50)
+  end
 end

test/sentry/integrations/oban/cron_test.exs

@@ -64,7 +64,7 @@ if Version.match?(System.version(), "~> 1.13") do
 
         assert check_in_body["check_in_id"] == "oban-123"
         assert check_in_body["status"] == "in_progress"
-        assert check_in_body["monitor_slug"] == "Sentry.MyWorker"
+        assert check_in_body["monitor_slug"] == "sentry-my-worker"
         assert check_in_body["duration"] == nil
         assert check_in_body["environment"] == "test"
 
@@ -120,7 +120,7 @@ if Version.match?(System.version(), "~> 1.13") do
 
           assert check_in_body["check_in_id"] == "oban-942"
           assert check_in_body["status"] == unquote(expected_status)
-          assert check_in_body["monitor_slug"] == "Sentry.MyWorker"
+          assert check_in_body["monitor_slug"] == "sentry-my-worker"
           assert check_in_body["duration"] == 12.099
           assert check_in_body["environment"] == "test"
 
@@ -164,7 +164,7 @@ if Version.match?(System.version(), "~> 1.13") do
 
         assert check_in_body["check_in_id"] == "oban-942"
         assert check_in_body["status"] == "error"
-        assert check_in_body["monitor_slug"] == "Sentry.MyWorker"
+        assert check_in_body["monitor_slug"] == "sentry-my-worker"
         assert check_in_body["duration"] == 12.099
         assert check_in_body["environment"] == "test"
 

test/sentry/integrations/quantum/cron_test.exs

@@ -59,7 +59,7 @@ if Version.match?(System.version(), "~> 1.12") do
 
         assert check_in_body["check_in_id"] == "quantum-#{:erlang.phash2(ref)}"
         assert check_in_body["status"] == "in_progress"
-        assert check_in_body["monitor_slug"] == "quantum-:test_job"
+        assert check_in_body["monitor_slug"] == "quantum-test-job"
         assert check_in_body["duration"] == nil
         assert check_in_body["environment"] == "test"
 
@@ -99,7 +99,7 @@ if Version.match?(System.version(), "~> 1.12") do
 
         assert check_in_body["check_in_id"] == "quantum-#{:erlang.phash2(ref)}"
         assert check_in_body["status"] == "error"
-        assert check_in_body["monitor_slug"] == "quantum-:test_job"
+        assert check_in_body["monitor_slug"] == "quantum-test-job"
         assert check_in_body["duration"] == 12.099
         assert check_in_body["environment"] == "test"
 
@@ -141,7 +141,7 @@ if Version.match?(System.version(), "~> 1.12") do
 
         assert check_in_body["check_in_id"] == "quantum-#{:erlang.phash2(ref)}"
         assert check_in_body["status"] == "ok"
-        assert check_in_body["monitor_slug"] == "quantum-:test_job"
+        assert check_in_body["monitor_slug"] == "quantum-test-job"
         assert check_in_body["duration"] == 12.099
         assert check_in_body["environment"] == "test"
 
@@ -170,5 +170,38 @@ if Version.match?(System.version(), "~> 1.12") do
 
       assert_receive {^ref, :done}, 1000
     end
+
+    for {job_name, expected_slug} <- [
+          {:some_job, "quantum-some-job"},
+          {MyApp.MyJob, "quantum-my-app-my-job"}
+        ] do
+      test "works for a job named #{inspect(job_name)}", %{bypass: bypass} do
+        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_slug"] == unquote(expected_slug)
+          send(test_pid, {ref, :done})
+
+          Plug.Conn.send_resp(conn, 200, ~s<{"id": "1923"}>)
+        end)
+
+        duration = System.convert_time_unit(12_099, :millisecond, :native)
+
+        :telemetry.execute([:quantum, :job, :stop], %{duration: duration}, %{
+          job:
+            Scheduler.new_job(
+              name: unquote(job_name),
+              schedule: Crontab.CronExpression.Parser.parse!("@daily")
+            ),
+          telemetry_span_context: ref
+        })
+
+        assert_receive {^ref, :done}, 1000
+      end
+    end
   end
 end