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

Implement an internal events system #10457

Open
dacook opened this issue Feb 17, 2023 · 3 comments
Open

Implement an internal events system #10457

dacook opened this issue Feb 17, 2023 · 3 comments

Comments

@dacook
Copy link
Member

dacook commented Feb 17, 2023

What we should change and why (this is tech debt)

I think an events system would make the system more robust and scalable. It brings multiple benefits, and be overall simpler than implementing some of those benefits ad-hoc in various parts of the system.

What is an events system?
Whenever something happens that can trigger another part of the system, or external notifications, we can implement a Publish/Subscribe layer. I've not done much research at all yet, but this article (and the mentioned gem) look like a good way to go:
https://blog.carbonfive.com/evented-rails-decoupling-complex-domains-in-rails-with-domain-events/

Context

While working on #9616 to introduce webhooks, I discovered a few ways that an internal events system could improve our system (see #9687 (comment)). I realised I was implementing some parts of this, but in domain-specific code, which was going to make the codebase bigger and harder to follow, so I opted not to do that, and wait until we can implement something like this.

Benefits / Requirements:

  • Decouple logic
    • for example, consider one method calling two unrelated modules. If the first one fails, the second would not execute.
  • asynchronous: being able to run published events as scheduled jobs means the calling code doesn't get interrupted
  • event deduplication (eg order_cycle.opened should only happen once, according to a key like "#{order_cycle.id}-#{order_cycle.opened_at}"). Although i'm not sure Wisper supports this.
  • Debugging: decoupling could make it harder to debug issues, so we need to ensure good help is in place
    • what if you subscribe to an event and the code doesn't run like you expected?
    • What the subscribing code runs and fails, and you can't see what triggered it? One way to help could be to store the calling scope in the event metadata, and in the event of an exception it could be added to the bugsnag context

Impact and timeline

Some parts of the code that might be changed/enhanced:

  1. OrderCycleClosingJob - Right now it's got a single purpose, but in the future we may need to trigger more than one result.
    • Can be decoupled from OrderCycleNotificationJob and run it async
    • Instead of recording completion (and preventing re-running) with processed_at, we could potentially deduplicate the event instead.
  2. Soon-to-be-added OrderCycleOpenedJob will be similar
  3. SubscriptionPlacementJob could be decoupled from OrderManagement::Subscriptions::Summarizer
  4. Maybe webhooks could be more closely integrated with this, making it easy to subscribe to any internal events.

I'm not sure how long it would take to implement. I would start first where it's needed the most (perhaps webhooks).

@rioug
Copy link
Collaborator

rioug commented Feb 21, 2023

Rails offer some sort of pub/sub system with Active Support Instrumentation , it's maybe something we can leverage as it supports adding custom events

@abdellani
Copy link
Member

This looks interesting :)

if I can help anywhere let me know.

@lin-d-hop
Copy link
Contributor

I'm late to the party, but this is a great proposal @dacook. I wonder how we can tag this onto some feature set.... 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: All the things
Development

No branches or pull requests

4 participants