Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Sep 16, 2024

Conversation

notslang
Copy link
Contributor

@notslang notslang commented Sep 4, 2024

This is an attempt at including whether Oban leadership changed in the oban.peer.election

I added the key was_leader, which indicates if the current node was the leader before the election happened.

If was_leader=false and leader=true in a telemetry event, then we know that the current node just became the leader. If was_leader=true and leader=false then we know that the current node lost leadership.

Marking this as a draft because I haven't tested it out yet in my application. I think I can add some tests to test/oban/peers/postgres_test.exs and test/oban/peers/global_test.exs too. That seems to be where this telemetry event gets tested.

Also, lmk if you'd prefer a different name.

@sorentwo
Copy link
Member

sorentwo commented Sep 4, 2024

Marking this as a draft because I haven't tested it out yet in my application. I think I can add some tests to test/oban/peers/postgres_test.exs and test/oban/peers/global_test.exs too. That seems to be where this telemetry event gets tested.

That's the right place.

Also, lmk if you'd prefer a different name.

The was_leader name works for me. Most of our other predicates have a trailing question mark, so perhaps was_leader? instead?

@sorentwo sorentwo added kind:enhancement New feature or request area:oss Related to Oban OSS labels Sep 4, 2024
@sorentwo
Copy link
Member

@notslang Do you have time to add the tests or would you like us to pick up this change?

@notslang notslang force-pushed the previous-leader-telemetry-event branch 2 times, most recently from 6cc44c0 to e6cf9c1 Compare September 16, 2024 18:09
@notslang notslang force-pushed the previous-leader-telemetry-event branch from e6cf9c1 to 41b0986 Compare September 16, 2024 19:16
@notslang notslang marked this pull request as ready for review September 16, 2024 20:13
@notslang
Copy link
Contributor Author

notslang commented Sep 16, 2024

I think this is good to review now. Renamed was_leader -> was_leader? and tested it out in my application. Looking for events where leader=true and was_leader?=false seems to reliably detect nodes becoming leaders.

@sorentwo sorentwo merged commit b003cf2 into oban-bg:main Sep 16, 2024
2 checks passed
@sorentwo
Copy link
Member

Excellent, thanks!

@notslang notslang deleted the previous-leader-telemetry-event branch September 17, 2024 14:30
@notslang notslang restored the previous-leader-telemetry-event branch September 17, 2024 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:oss Related to Oban OSS kind:enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants