From 0eafbc7bd81991fe60ab43a4dd25e7d0d17c6dbf Mon Sep 17 00:00:00 2001 From: Annanay Date: Wed, 29 Apr 2020 20:41:16 +0530 Subject: [PATCH 1/4] Add design doc for generalizing module service Signed-off-by: Annanay --- docs/proposals/generalize-modules.md | 72 ++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 docs/proposals/generalize-modules.md diff --git a/docs/proposals/generalize-modules.md b/docs/proposals/generalize-modules.md new file mode 100644 index 00000000000..7442d967201 --- /dev/null +++ b/docs/proposals/generalize-modules.md @@ -0,0 +1,72 @@ +--- +title: "Generalize Modules Service to make it extensible" +linkTitle: "Generalize Modules Service to make it extensible" +weight: 1 +slug: generalize-modules +--- + +- Author: @annanay25 +- Reviewers: +- Date: April 2020 +- Status: Draft + +## Overview + +Cortex uses modules to start and operate services with dependencies. Inter-service dependencies are specified in a map and passed to a module manager which ensures that they are started in the right order of dependencies. While this works really well, the implementation is tied in specifically to the Cortex struct and is not flexible for use with other projects like Loki, which also require similar forms of dependency management. + +We would like to extend modules in cortex to a generic dependency management framework, that can be used by any project with no ties to cortex. + +## Specific goals + +- Framework should allow for reusing cortex modules and allow us to: + - Add new modules + - Overwrite the implementation of a current module + - Manage dependencies +- Framework should allow for building an application from scratch using the `modules` package, with no dependencies on Cortex. For ex: Remove code from Loki that was copied from `pkg/cortex/cortex.go`. + + + +## Proposed Design + +To make the modules package extensible, we need to abstract away any Cortex specific details from the module manager. The proposed design is to - + +- Make a new component `moduleManager`, which is envisioned to be a central manager for all modules of the application. It stores modules & dependencies, and will be housed under a new package `pkg/util/modules`. The following is the interface for interacting with the `moduleManager`: +``` +// Manager defines the interface for interaction with moduleManager. +type Manager interface { + RegisterModule(name string, deps []string, svc service, options ...Option) + AddDependency(fromModule string, toModule string) error + ModuleServiceWrapper(name string, modServ services.Service) + InitModuleServices(target string) (map[string]services.Service, error) +} +``` + +- Modules can be created by the application and registered with the `ModuleManager` using `RegisterModule`. The parameters are: + - `name`: Name of the module + - `deps`: A list of modules that this module depends on to run. + - `svc`: A function that will be used to start the module. + - `options`: Options are a variadic function parameter that can be used by the `moduleManager` to wrap the service function. In Cortex, we pass `Manager.ModuleServiceWrapper`, which wraps service to work with dependencies. + +- Dynamic dependencies between modules can be added using `AddDependency`. However, these dependencies need to be added before the call to `InitModuleServices`. + +- The application can be initialized by running all the modules in the right order of dependencies by invoking `InitModuleServices`. + +- `WrappedService` present in the current `module` design has been deprecated. To distinguish between `Service` and `WrappedService` in `RegisterModule`, the following options are available: + - The simplest option is to pass a `boolean` parameter in that can default to `true` for a `WrappedService` since most services in Cortex are `WrappedServices`. + - The second option is to wrap every `Service` into `WrappedService` if `options == nil` (default case), and for the special case where a service need not be wrapped, a dummy function can be passed. + - A special function can be passed to the `ModuleManager` to wrap a `Service` into a `WrappedService` which waits on dependencies to start before it and stop after it. This parameter is `ModuleManager.ModuleServiceWrapper` referenced in the `Manager` interface. + +- While the process of loading modules into `modules.Manager` should be remain as part of the `Cortex.New()` function, `InitModuleServices` should be part of `Cortex.Run()` and to enable this, `modules.Manager` would be made a member of the `Cortex` struct. + + + +## Usage + +Following these changes, the Modules package will be a generic dependency management framework that can be used by any project. + +#### To use the modules framework: +- Import the `pkg/util/modules` package, and initialize a new instance of the `Manager` using `modules.NewManager()` +- Create components in the system that implement the services interface (present in `pkg/util/services`). +- Register each of these components as a module using `Manager.RegisterModule()` by passing name of the module, dependencies, initFn and any extra options that can be used to wrap the initFn. +- To add dynamic dependencies between modules, use `Manager.AddDependency()` +- Once all modules are added into `modules.Manager`, initialize the application by calling `Manager.InitModuleServices()` which initializes modules in the right order of dependencies. \ No newline at end of file From a650c0a93c95c3068dbb9bcdc22e7346089533bb Mon Sep 17 00:00:00 2001 From: Annanay Date: Mon, 4 May 2020 20:03:06 +0530 Subject: [PATCH 2/4] Update docs as per comments Signed-off-by: Annanay --- docs/proposals/generalize-modules.md | 44 ++++++++++++---------------- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/docs/proposals/generalize-modules.md b/docs/proposals/generalize-modules.md index 7442d967201..78b421ae488 100644 --- a/docs/proposals/generalize-modules.md +++ b/docs/proposals/generalize-modules.md @@ -6,15 +6,15 @@ slug: generalize-modules --- - Author: @annanay25 -- Reviewers: +- Reviewers: @jtlisi, @pstibrany - Date: April 2020 -- Status: Draft +- Status: Review ## Overview -Cortex uses modules to start and operate services with dependencies. Inter-service dependencies are specified in a map and passed to a module manager which ensures that they are started in the right order of dependencies. While this works really well, the implementation is tied in specifically to the Cortex struct and is not flexible for use with other projects like Loki, which also require similar forms of dependency management. - -We would like to extend modules in cortex to a generic dependency management framework, that can be used by any project with no ties to cortex. +Cortex uses modules to start and operate services with dependencies. Inter-service dependencies are specified in a map and passed to a module manager which ensures that they are started in the right order of dependencies. While this works really well, the implementation is tied in specifically to the Cortex struct and is not flexible for use with other projects like Loki, which also require similar forms of dependency management. + +We would like to extend modules in cortex to a generic dependency management framework, that can be used by any project with no ties to cortex. ## Specific goals @@ -25,39 +25,33 @@ We would like to extend modules in cortex to a generic dependency management fra - Framework should allow for building an application from scratch using the `modules` package, with no dependencies on Cortex. For ex: Remove code from Loki that was copied from `pkg/cortex/cortex.go`. - ## Proposed Design +### Modules package + To make the modules package extensible, we need to abstract away any Cortex specific details from the module manager. The proposed design is to - -- Make a new component `moduleManager`, which is envisioned to be a central manager for all modules of the application. It stores modules & dependencies, and will be housed under a new package `pkg/util/modules`. The following is the interface for interacting with the `moduleManager`: +- Make a new component `Manager`, which is envisioned to be a central manager for all modules of the application. It stores modules & dependencies, and will be housed under a new package `pkg/util/modules`. `Manager` has the following methods for interaction: ``` -// Manager defines the interface for interaction with moduleManager. -type Manager interface { - RegisterModule(name string, deps []string, svc service, options ...Option) - AddDependency(fromModule string, toModule string) error - ModuleServiceWrapper(name string, modServ services.Service) - InitModuleServices(target string) (map[string]services.Service, error) -} + func (m *Manager) RegisterModule(name string, deps []string, initFn func() (Service, error)) + func (m *Manager) AddDependency(fromModule string, toModule string) error + func (m *Manager) InitModuleServices(target string) (map[string]services.Service, error) ``` -- Modules can be created by the application and registered with the `ModuleManager` using `RegisterModule`. The parameters are: +- Modules can be created by the application and registered with `modules.Manager` using `RegisterModule`. The parameters are: - `name`: Name of the module - `deps`: A list of modules that this module depends on to run. - - `svc`: A function that will be used to start the module. - - `options`: Options are a variadic function parameter that can be used by the `moduleManager` to wrap the service function. In Cortex, we pass `Manager.ModuleServiceWrapper`, which wraps service to work with dependencies. + - `initFn`: A function that will be used to start the module. If it returns nil, and other modules depend on it `InitModuleServices` will return an error. -- Dynamic dependencies between modules can be added using `AddDependency`. However, these dependencies need to be added before the call to `InitModuleServices`. +- Dynamic dependencies between modules can be added using `AddDependency`. These need to be added before the call to `InitModuleServices`. - The application can be initialized by running all the modules in the right order of dependencies by invoking `InitModuleServices`. -- `WrappedService` present in the current `module` design has been deprecated. To distinguish between `Service` and `WrappedService` in `RegisterModule`, the following options are available: - - The simplest option is to pass a `boolean` parameter in that can default to `true` for a `WrappedService` since most services in Cortex are `WrappedServices`. - - The second option is to wrap every `Service` into `WrappedService` if `options == nil` (default case), and for the special case where a service need not be wrapped, a dummy function can be passed. - - A special function can be passed to the `ModuleManager` to wrap a `Service` into a `WrappedService` which waits on dependencies to start before it and stop after it. This parameter is `ModuleManager.ModuleServiceWrapper` referenced in the `Manager` interface. -- While the process of loading modules into `modules.Manager` should be remain as part of the `Cortex.New()` function, `InitModuleServices` should be part of `Cortex.Run()` and to enable this, `modules.Manager` would be made a member of the `Cortex` struct. +### Changes to `pkg/cortex`: +- `WrappedService` present in the current `module` design will be deprecated. All `initFn`'s will be wrapped into `WrappedService` by default. +- While the process of loading modules into `modules.Manager` should be remain as part of the `Cortex.New()` function, `InitModuleServices` should be part of `Cortex.Run()` and to enable this, `modules.Manager` would be made a member of the `Cortex` struct. ## Usage @@ -66,7 +60,7 @@ Following these changes, the Modules package will be a generic dependency manage #### To use the modules framework: - Import the `pkg/util/modules` package, and initialize a new instance of the `Manager` using `modules.NewManager()` -- Create components in the system that implement the services interface (present in `pkg/util/services`). -- Register each of these components as a module using `Manager.RegisterModule()` by passing name of the module, dependencies, initFn and any extra options that can be used to wrap the initFn. +- Create components in the system that implement the services interface (present in `pkg/util/services`). +- Register each of these components as a module using `Manager.RegisterModule()` by passing name of the module, dependencies, and `initFn`. - To add dynamic dependencies between modules, use `Manager.AddDependency()` - Once all modules are added into `modules.Manager`, initialize the application by calling `Manager.InitModuleServices()` which initializes modules in the right order of dependencies. \ No newline at end of file From bd4e79beb4aa69a5844ff47b3c91213f6d773508 Mon Sep 17 00:00:00 2001 From: Annanay Agarwal Date: Tue, 5 May 2020 12:28:08 +0530 Subject: [PATCH 3/4] Update docs/proposals/generalize-modules.md Co-authored-by: Marco Pracucci Apply suggestions from code review Co-authored-by: Marco Pracucci Signed-off-by: Annanay --- docs/proposals/generalize-modules.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/proposals/generalize-modules.md b/docs/proposals/generalize-modules.md index 78b421ae488..f04d27d8e78 100644 --- a/docs/proposals/generalize-modules.md +++ b/docs/proposals/generalize-modules.md @@ -29,7 +29,7 @@ We would like to extend modules in cortex to a generic dependency management fra ### Modules package -To make the modules package extensible, we need to abstract away any Cortex specific details from the module manager. The proposed design is to - +To make the modules package extensible, we need to abstract away any Cortex specific details from the module manager. The proposed design is to: - Make a new component `Manager`, which is envisioned to be a central manager for all modules of the application. It stores modules & dependencies, and will be housed under a new package `pkg/util/modules`. `Manager` has the following methods for interaction: ``` @@ -49,6 +49,7 @@ To make the modules package extensible, we need to abstract away any Cortex spec ### Changes to `pkg/cortex`: + - `WrappedService` present in the current `module` design will be deprecated. All `initFn`'s will be wrapped into `WrappedService` by default. - While the process of loading modules into `modules.Manager` should be remain as part of the `Cortex.New()` function, `InitModuleServices` should be part of `Cortex.Run()` and to enable this, `modules.Manager` would be made a member of the `Cortex` struct. @@ -59,8 +60,9 @@ To make the modules package extensible, we need to abstract away any Cortex spec Following these changes, the Modules package will be a generic dependency management framework that can be used by any project. #### To use the modules framework: + - Import the `pkg/util/modules` package, and initialize a new instance of the `Manager` using `modules.NewManager()` - Create components in the system that implement the services interface (present in `pkg/util/services`). - Register each of these components as a module using `Manager.RegisterModule()` by passing name of the module, dependencies, and `initFn`. - To add dynamic dependencies between modules, use `Manager.AddDependency()` -- Once all modules are added into `modules.Manager`, initialize the application by calling `Manager.InitModuleServices()` which initializes modules in the right order of dependencies. \ No newline at end of file +- Once all modules are added into `modules.Manager`, initialize the application by calling `Manager.InitModuleServices()` which initializes modules in the right order of dependencies. From 7cce7ba50d3ea1dfa42d42bd01226a23870e8aab Mon Sep 17 00:00:00 2001 From: Annanay Date: Tue, 5 May 2020 13:39:03 +0530 Subject: [PATCH 4/4] Address review comments Signed-off-by: Annanay --- docs/proposals/generalize-modules.md | 31 ++++++++++++++++++---------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/docs/proposals/generalize-modules.md b/docs/proposals/generalize-modules.md index f04d27d8e78..85e1f0c377b 100644 --- a/docs/proposals/generalize-modules.md +++ b/docs/proposals/generalize-modules.md @@ -6,13 +6,13 @@ slug: generalize-modules --- - Author: @annanay25 -- Reviewers: @jtlisi, @pstibrany +- Reviewers: @jtlisi, @pstibrany, @cyriltovena, @pracucci - Date: April 2020 -- Status: Review +- Status: Accepted ## Overview -Cortex uses modules to start and operate services with dependencies. Inter-service dependencies are specified in a map and passed to a module manager which ensures that they are started in the right order of dependencies. While this works really well, the implementation is tied in specifically to the Cortex struct and is not flexible for use with other projects like Loki, which also require similar forms of dependency management. +Cortex uses modules to start and operate services with dependencies. Inter-service dependencies are specified in a map and passed to a module manager which ensures that they are initialised in the right order of dependencies. While this works really well, the implementation is tied in specifically to the Cortex struct and is not flexible for use with other projects like Loki, which also require similar forms of dependency management. We would like to extend modules in cortex to a generic dependency management framework, that can be used by any project with no ties to cortex. @@ -33,19 +33,22 @@ To make the modules package extensible, we need to abstract away any Cortex spec - Make a new component `Manager`, which is envisioned to be a central manager for all modules of the application. It stores modules & dependencies, and will be housed under a new package `pkg/util/modules`. `Manager` has the following methods for interaction: ``` - func (m *Manager) RegisterModule(name string, deps []string, initFn func() (Service, error)) - func (m *Manager) AddDependency(fromModule string, toModule string) error + func (m *Manager) RegisterModule(name string, initFn func() (Service, error)) + func (m *Manager) AddDependency(name string, dependsOn... string) error func (m *Manager) InitModuleServices(target string) (map[string]services.Service, error) ``` - Modules can be created by the application and registered with `modules.Manager` using `RegisterModule`. The parameters are: - `name`: Name of the module - - `deps`: A list of modules that this module depends on to run. - - `initFn`: A function that will be used to start the module. If it returns nil, and other modules depend on it `InitModuleServices` will return an error. + - `initFn`: A function that will be used to start the module. If it returns nil, and other modules depend on it, `InitModuleServices` will return an error. -- Dynamic dependencies between modules can be added using `AddDependency`. These need to be added before the call to `InitModuleServices`. +- Dependencies between modules can be added using `AddDependency`. The parameters to the function are: + - `name`: Name of the module + - `dependsOn`: A variadic list of modules that the module depends on. + + These need to be added before the call to `InitModuleServices`. -- The application can be initialized by running all the modules in the right order of dependencies by invoking `InitModuleServices`. +- The application can be initialized by running `initFn`'s of all the modules in the right order of dependencies by invoking `InitModuleServices` with the target module name. ### Changes to `pkg/cortex`: @@ -63,6 +66,12 @@ Following these changes, the Modules package will be a generic dependency manage - Import the `pkg/util/modules` package, and initialize a new instance of the `Manager` using `modules.NewManager()` - Create components in the system that implement the services interface (present in `pkg/util/services`). -- Register each of these components as a module using `Manager.RegisterModule()` by passing name of the module, dependencies, and `initFn`. -- To add dynamic dependencies between modules, use `Manager.AddDependency()` +- Register each of these components as a module using `Manager.RegisterModule()` by passing name of the module and `initFn` for the module. +- To add dependencies between modules, use `Manager.AddDependency()` - Once all modules are added into `modules.Manager`, initialize the application by calling `Manager.InitModuleServices()` which initializes modules in the right order of dependencies. + + +## Future work + +- Extend the module manager to allow specifying multiple targets as opposed to a single target name supported currently. +- Factor out `Run()` method to make it independent of Cortex. This will help reduce replicated code in the Loki project as well as help manage `modules.Manager` outside of the Cortex struct. \ No newline at end of file