remove the start method from the Module interface #588
Conversation
| val executor = Executors.newSingleThreadExecutor(threadFactory(settings.name)) | ||
| Entry(settings.name, ExecutionContext.fromExecutorService(executor), programmaticallyAdded, settings, instance) | ||
|
|
||
| // Scheduling any task on the executor ensures that the underlying Thread is created and that the JVM will stay |
There was a problem hiding this comment.
Why is this desired? Couldn't this cause the JVM to hang on exit if there is an issue? I think that could be extremely confusing fo rusers
There was a problem hiding this comment.
The idea is to force people to shutdown the modules so that they have a final chance to report any accumulated data, we even have some tests covering this behavior and #533 that are sort of related to this.
If an user sees that while testing their JVM doesn't shutdown then their are much more likely to take this into account and properly call Kamon.stopModules() before shutting down the app.
There was a problem hiding this comment.
This should be probably documented somewhere. We've just came into this issue of hanging application because of Kamon modules. I'd expect somewhere in installation guide that user should handle stopping of modules.
|
@ivantopo LGTM! |
While creating and updating modules I noticed one small annoyance that is sort of forced by the
Moduleinterface: since users are supposed to implement astartmethod but obviously there are always a number of "things" that need to be instantiated for the module to work then we end up creating modules with a bunch ofvarsthat are initialized with some sort of default value or defined asOption[X]and then create the actual values whenstartis called, but, in practice, there is no need to do that. All modules are instantiated right when they are going to be started, there is no chance to have an instantiated but not started module so I'm proposing to remove thestartmethod from theModuleinterface.Besides that change, I'm adding a
ModuleFactoryto be used when the modules are automatically instantiated by Kamon instead of provided by the user (which will be the default behavior).. this way, we force the parameter-less constructor only on the Factory and the Module itself is free to receive/grab any dependencies it might need on its constructor.