Add information on leadership changes to oban.peer.election event (#…

…1148) Include `was_leader?` in `[:oban, :peer, :election | _]` event metadata
oban-bg · Sep 16, 2024 · b003cf2 · b003cf2
1 parent 83d2fc1
commit b003cf2
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 10 deletions.
diff --git a/lib/oban/peers/global.ex b/lib/oban/peers/global.ex
@@ -90,13 +90,13 @@ defmodule Oban.Peers.Global do
 
   @impl GenServer
   def handle_info(:election, %State{} = state) do
-    meta = %{conf: state.conf, leader: state.leader?, peer: __MODULE__}
+    meta = %{conf: state.conf, leader: state.leader?, peer: __MODULE__, was_leader?: nil}
 
     locked? =
       :telemetry.span([:oban, :peer, :election], meta, fn ->
         locked? = :global.set_lock(key(state), nodes(), 0)
 
-        {locked?, %{meta | leader: locked?}}
+        {locked?, %{meta | leader: locked?, was_leader?: meta.leader}}
       end)
 
     if locked?, do: notify_lock(state.conf)

diff --git a/lib/oban/peers/postgres.ex b/lib/oban/peers/postgres.ex
@@ -94,7 +94,7 @@ defmodule Oban.Peers.Postgres do
 
   @impl GenServer
   def handle_info(:election, %State{} = state) do
-    meta = %{conf: state.conf, leader: state.leader?, peer: __MODULE__}
+    meta = %{conf: state.conf, leader: state.leader?, peer: __MODULE__, was_leader?: nil}
 
     state =
       :telemetry.span([:oban, :peer, :election], meta, fn ->
@@ -106,14 +106,14 @@ defmodule Oban.Peers.Postgres do
 
         case Repo.transaction(state.conf, fun, retry: 1) do
           {:ok, state} ->
-            {state, %{meta | leader: state.leader?}}
+            {state, %{meta | leader: state.leader?, was_leader?: meta.leader}}
 
           {:error, :rollback} ->
             # The peer maintains its current `leader?` status on rollback—this may cause
             # inconsistency if the leader encounters an error and multiple rollbacks happen in
             # sequence. That tradeoff is acceptable because the situation is unlikely and less of
             # an issue than crashing the peer.
-            {state, meta}
+            {state, %{meta | was_leader?: meta.leader}}
         end
       end)
 

diff --git a/lib/oban/telemetry.ex b/lib/oban/telemetry.ex
@@ -177,13 +177,14 @@ defmodule Oban.Telemetry do
   | event        | measures       | metadata                                                       |
   | ------------ | -------------- | -------------------------------------------------------------- |
   | `:start`     | `:system_time` | `:conf`, `:leader`, `:peer`,                                   |
-  | `:stop`      | `:duration`    | `:conf`, `:leader`, `:peer`,                                   |
+  | `:stop`      | `:duration`    | `:conf`, `:leader`, `:peer`, `:was_leader?`                    |
   | `:exception` | `:duration`    | `:conf`, `:leader`, `:peer`, `:kind`, `:reason`, `:stacktrace` |
 
   #### Metadata
 
   * `:conf`, `:kind`, `:reason`, `:stacktrace` — see the explanation in notifier metadata above
   * `:leader` — whether the peer is the current leader
+  * `:was_leader?` — whether the peer was the leader before the election occurred
   * `:peer` — the module used for peering
 
   ## Queue Shutdown Events

diff --git a/test/oban/peers/global_test.exs b/test/oban/peers/global_test.exs
@@ -54,7 +54,10 @@ defmodule Oban.Peers.GlobalTest do
 
     start_supervised_oban!(peer: Global, node: "worker.1")
 
-    assert_receive {:event, [:election, :start], _measure, %{leader: _, peer: Oban.Peers.Global}}
-    assert_receive {:event, [:election, :stop], _measure, %{leader: _, peer: Oban.Peers.Global}}
+    assert_receive {:event, [:election, :start], _measure,
+                    %{leader: _, peer: Oban.Peers.Global, was_leader?: nil}}
+
+    assert_receive {:event, [:election, :stop], _measure,
+                    %{leader: _, peer: Oban.Peers.Global, was_leader?: false}}
   end
 end
diff --git a/test/oban/peers/postgres_test.exs b/test/oban/peers/postgres_test.exs
@@ -18,8 +18,11 @@ defmodule Oban.Peers.PostgresTest do
              |> Enum.map(&start_supervised!({Peer, conf: conf, name: &1}))
              |> Enum.filter(&Postgres.leader?/1)
 
-    assert_received {:event, [:election, :start], _measure, %{leader: _, peer: Postgres}}
-    assert_received {:event, [:election, :stop], _measure, %{leader: _, peer: Postgres}}
+    assert_received {:event, [:election, :start], _measure,
+                     %{leader: _, peer: Postgres, was_leader?: nil}}
+
+    assert_received {:event, [:election, :stop], _measure,
+                     %{leader: _, peer: Postgres, was_leader?: false}}
   end
 
   test "gracefully handling a missing oban_peers table" do