Introduce Gitlab::EventStore as simple pub-sub system
Problem
With GitLab monolith becoming larger and more domains being defined we have a problem of entangling these domains with each others due to temporal coupling.
An emblematic example is PostReceive worker where a lot happens across multiple domains. If a new behavior requires to react to a new commit being pushed then we add code somewhere in PostReceive or its sub-components. (Git::ProcessRefChangesService, etc.). There are of course may of these examples in our codebase.
This type of architecture causes:
- Violation of the Single Responsibility Principle.
- Increase risk in adding code to a code base you are not familiar with, simply because your domain needs to do something at that particular moment. There may be nuances you don't know about which may introduce bugs or performance degradation.
- Domain boundaries are violated. Within a specific namespace (e.g.
Git::) we suddenly see classes from other domains chiming in (Ci::,MergeRequests::, etc.).
What does this MR do?
This MR introduces an event-driven pattern to allow us decoupling domains by implementing a simple pub-sub system. This system leverages our existing Sidekiq workers and the observability we have today, so no new technology or message queuing systems.
This essentially leaves the existing workers as is to perform async work but inverts the dependency where:
- before: a worker for domain
Awas scheduled from inside the domainB - after: domain
Asubscribes to an eventXfrom domainBusing its workerA::SomeWorker. WhenBpublishes an eventX, it's consumed by its subscribers, includingA::SomeWorker.
Next steps
- Log publishing of domain events and dispatching to subscribers, in order to support better debugging in Kibana.
-
Enable the feature flag
ci_publish_pipeline_events - Ensure that
correlation_idfrom subscribers call be traced back to the caller - add provenance of events - Deprecate
UpdateHeadPipelineForMergeRequestWorkerin favour of inlining the code - Extract shared examples to test that events are published and received and have it as standard way to ensure end-to-end message passing.
- Static analyzer to ensure
performmethod is not overridden when includingGitlab::EventStore::Subscribermodule and thathandle_eventmethod is defined. - Static analyzer to ensure we don't call workers from other namespaces but publish events instead.
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers -
Informed Infrastructure department of a default or new setting change, if applicable per definition of done
Security
If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:
-
Label as security and @ mention @gitlab-com/gl-security/appsec -
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team