Use constants for notifier events

[khjobran1]

What?

Adding constants to unify event names for notifiers.

Why?

To help ignoring any events in custom notifiers. And prevent any future breaking changes regarding event naming. Also, this approach should enhance memory usage as this will prevent many strings creation on each request runtime.

Custom notifier logic example:

def notify(circuit_name, event)
  return if event == Circuitbox::CircuitBreaker::SUCCESS_EVENT
  # ... custom logger
end

[matthewshafer]

Hey @khjobran1 Thanks for opening up the pull request!

From my understanding of the change, adding constants for all of the notification events would allow a custom notifier to reference the constants when making decisions on if an event should be processed.

I don't believe circuitbox has really any documentation on custom notifiers. I can see it being challenging to add a custom notifier when theres no documentation on what the events are without looking at the circuit breaker code or the active support notifier code. If you implemented a custom notifier did you run into something similar (needing to look at circuitbox code)?

[khjobran1]

Hey @matthewshafer, thanks for your reply. Yes indeed. I was trying to implement logging for specific events of the circuit breaker. Initially ChatGPT gave me a wrong response of how a custom notifier can be implemented for Circuitbox.

I took a look on existing notifiers and did the same. Then I noticed that I'm getting too many logs even for successful requests. At that point I had to check the Circuitbreaker class to understand what are these events.

For my case I wanted to have more observability on cuircuit's status (open/closed). Without all the other notifications which I have implemented as part of my client code.

Please let me know if there is a better way to do this change, or if we would need a documentation for custom notifiers, and I'll be happy to contribute.

[matthewshafer]

@khjobran1 Let's go with adding documentation for building a custom instrumenter and include a list of events and some details on them. If you would like to contribute the documentation updates that would be great! If not let me know and I can put something together.

[khjobran1]

@matthewshafer it would be better if you can add that documentation, as I don't have enough context to do that. And please let me know if I should change anything in this PR.

[matthewshafer]

@khjobran1 For now I'll keep the PR open until we've had time to look at the new documentation. I feel like documentation on custom instrumenters might be all that's needed. I'll tag you on the PR with the new documentation if that's okay with you.

[khjobran1]

@matthewshafer Sure please tag me in that PR.

Just to give more context about this PR, I am trying to avoid the hardcoded event names in my custom notifier's implementation, for better clarity and consistency. Also, even though its unlikely, changing an event name in Circuitbrox would break my custom notifier in that case.

Additionally, using constants for event strings can lead to some memory savings, especially in high-throughput scenarios.