Replies: 1 comment
-
Take a look at #476 and let me know your thoughts. Particularly with the side effect I mentioned. |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
Many of the components of this project take care of start/stopping the other components they use during their own start sequence.
For instance, the
AbstractLogstashTcpAppender
will start theEncoder
it is configured with if the later is not yet started at the time the appender is started. It won't stop it however... Other components like theCompositeJsonEncoder
will start/stop the formatter and the prefix/suffix encoders.It is not clear (to me) when a component should start other components they are given and when they should stop them.
Let's take an example. What would happen if I reuse the same
Encoder
instance for two distinct TCP appenders and I stop one of them at runtime? TheEncoder
will be stopped although it is still used by the other appender.According to me, a component should take care of the lifecycle of others only if it used to create them (i.e. it is responsible of them). Otherwise, if the components are set as part of the configuration process (given through a setter method or in the constructor), it is up to the "user/caller" to start and stop them.
With this in mind, the
AbstractLogstashTcpAppender
should not take care of the encoder's lifecycle and theCompositeJsonEncoder
should only care about theformatter
(and not the prefix/suffix encoders) since it is responsible to create an instance thereof.I'm well aware that Logback itself is not very consistent regarding this LifeCyle and taking care of starting components like we do for the moment is probably very convenient for the user. Moreover, changing this "auto start" behaviour may have some unexpected breaking impacts for some...
Maybe we could opt for an intermediate solution where:
This should be compatible with the current behaviour while still allowing users to reuse the same component instances (providing they take care of starting them during the configuration process).
Beta Was this translation helpful? Give feedback.
All reactions