Skip to content

Introduce Gitlab::EventStore as simple pub-sub system

Fabio Pitino requested to merge poc-gitlab-event-store into master

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 A was scheduled from inside the domain B
  • after: domain A subscribes to an event X from domain B using its worker A::SomeWorker. When B publishes an event X, it's consumed by its subscribers, including A::SomeWorker.

Next steps

  1. Log publishing of domain events and dispatching to subscribers, in order to support better debugging in Kibana.
  2. Enable the feature flag ci_publish_pipeline_events
  3. Ensure that correlation_id from subscribers call be traced back to the caller - add provenance of events
  4. Deprecate UpdateHeadPipelineForMergeRequestWorker in favour of inlining the code
  5. Extract shared examples to test that events are published and received and have it as standard way to ensure end-to-end message passing.
  6. Static analyzer to ensure perform method is not overridden when including Gitlab::EventStore::Subscriber module and that handle_event method is defined.
  7. Static analyzer to ensure we don't call workers from other namespaces but publish events instead.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

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
Edited by Fabio Pitino

Merge request reports