Do I need to disable drive On Mon, Apr 19, 2021, 4:41 PM Tim Perry <tim.v...@gmail.com> wrote:
> After further thought, I am threading the context name into the location > where the StatusConfiguration creates the StatusConsoleListener and > registering the context name there. > > In addition, if the new logger would write to a destination other than > standard out or standard error then I do not reconfigure the existing > logger in StatusConfiguration.configureExistingStatusConsoleListener(), > instead I have the > > I am now correctly closing the status logger when the context is stopped. > > I'll push the changes to github after I do a full build > > On Mon, Apr 19, 2021 at 12:17 PM Tim Perry <tim.v...@gmail.com> wrote: > > > I rewrote this to shut down listeners based on the contextName. In > > testing, I discovered that the StatusConsoleListener is created in > > StatusConfiguration, but neither StatusConfiguration nor > > StatusConsoleListener receive events to indicate when they should stop. > > > > It appears that only one StatusConsoleListener object is ever created and > > it is never shut down. Looking at the api XmlConfiguration, it calls > > StatusConfiguration.initilize() which then either changes the log level > to > > match the config being parsed or creates a new StatusLogger directed to > the > > file indicated in the XML configuration. Unless I'm reading the code > wrong, > > this means that the status logger output location depends on if a > previous > > app was loaded. If so, then that location will continue to receive > > StatusLogger messages but at the log level of the new application's > config. > > Am I reading this correctly? If I am, is this the intended behaviour? > > > > > > > > On Wed, Feb 24, 2021 at 8:29 AM Matt Sicker <boa...@gmail.com> wrote: > > > >> The StatusLogger has various listeners attached. I think adding and > >> removing listeners on startup and shutdown of a LoggerContext might be > >> a potential way to do this? > >> > >> On Wed, 24 Feb 2021 at 01:07, Tim Perry <tim.v...@gmail.com> wrote: > >> > > >> > Ralph, > >> > > >> > Thanks for the review. Yep, that *is* a problem...I knew it was a > >> singleton > >> > but didn't think through the use case you describe. This is ironic > >> since a > >> > few months ago I recommended that one of my clients bundle log4j in > each > >> > war rather than on Tomcat's classpath so there would be less chance of > >> > instances walking on each other. Sigh. > >> > > >> > > >> > What is the correct behaviour if: > >> > > >> > - log4j is on Tomcat's classpath > >> > - App A has status_A.log > >> > - App B has status_B.log > >> > > >> > Now assume both apps are started. At this point I assume we should be > >> > writing to both status_A.log and status_B.log. Now we stop App B. I > >> assume > >> > we should stop writing to status_B.log but not status_A.log. Further, > I > >> > assume that if both apps are unloaded from Tomcat, but Tomcat is left > >> > running, then the status logger should send its messages to standard > >> out. > >> > If my assumptions are correct, then maybe we need to keep track of > what > >> > file, if any, each web app requested messages to be written to. On top > >> of > >> > that, I think we need a Callback in Log4j's shutdown registry and we > >> need > >> > to run it last. > >> > > >> > > >> > In some ways this seems like an XY problem. Is the correct question > how > >> do > >> > we reconfigure the logging when a web app shuts down? Or should it be: > >> > should the StatusLogger be shared across multiple LoggerContexts? > >> > > >> > > >> > This will be more interesting than I first realized! > >> > > >> > Tim > >> > > >> > > >> > On Tue, Feb 23, 2021 at 10:38 PM Ralph Goers < > >> ralph.go...@dslextreme.com> > >> > wrote: > >> > > >> > > Yeah, I started a review but then I thought it probably would be > >> better to > >> > > respond here. > >> > > > >> > > You are on the right track but there is a problem. StatusLogger is a > >> > > singleton - there is one instance anchored in a static. You are > >> invoking > >> > > the shutdown logic from the shutdown of the LoggerContext which is > >> not a > >> > > singleton. Log4j supports multiple LoggerContexts in an application. > >> For > >> > > example, if you are old school and running multiple web applications > >> in > >> > > Tomcat and have Log4j on Tomcat’s class path then you will have > >> multiple > >> > > LoggerContexts with a single StatusLogger. So if one web app gets > >> > > redeployed then its LoggerContext will be shutdown and a new one > >> created > >> > > all while another app is continuing to run. > >> > > > >> > > If you’ll notice the StatusConfiguration class in log4j-core tries > to > >> > > accommodate for this during startup, but it doesn’t do anything at > >> > > shutdown. StatusLogger currently isn’t smart enough to handle one > app > >> > > writing to one destination and a different on writing to a different > >> one. > >> > > Since StatusLogger is a singleton it can’t really know which app a > >> status > >> > > log event is for. > >> > > > >> > > There are a couple of ways I can think of to handle this but none of > >> them > >> > > is perfect. > >> > > Modify StatusConfiguration to keep track of what each > >> StatusConfiguration > >> > > set up and reset to whatever the prior StatusConfiguration had. The > >> problem > >> > > with this is that applications might shutdown in a different order > >> than > >> > > they were started, so figuring out what the prior configuration was > >> could > >> > > be difficult. > >> > > Add the call to prepareToStop() as a new Callback to Log4j’s > shutdown > >> > > registry. However, this callback would need to run last. The > shutdown > >> > > registry currently doesn’t support a way to specify the order of > >> callbacks. > >> > > Support for that would need to be added for this to work. > >> > > > >> > > Ralph > >> > > > >> > > > On Feb 23, 2021, at 10:48 PM, Tim Perry <tim.v...@gmail.com> > wrote: > >> > > > > >> > > > Ralph, > >> > > > > >> > > > I implemented what you suggested. Feel free to suggest > improvements. > >> > > > https://github.com/apache/logging-log4j2/pull/469 > >> > > > > >> > > > Tim > >> > > > > >> > > > On Tue, Feb 23, 2021 at 2:14 PM Ralph Goers < > >> ralph.go...@dslextreme.com> > >> > > > wrote: > >> > > > > >> > > >> I would suggest that if it is writing to something other than > >> System.out > >> > > >> that it be redirected back there and then the OutputStream be > >> closed. > >> > > >> However, I’ve not looked at the code recently so I am not sure > >> what it > >> > > >> takes to do that. > >> > > >> > >> > > >> Ralph > >> > > >> > >> > > >>> On Feb 23, 2021, at 2:22 PM, Tim Perry <tim.v...@gmail.com> > >> wrote: > >> > > >>> > >> > > >>> Thank you, Volkan. > >> > > >>> > >> > > >>> I'm not quite ready to submit a PR. I was hoping some of you > with > >> more > >> > > >>> knowledge of log4j-core would weigh in on what we should do > about > >> > > >> shutting > >> > > >>> down the StatusLogger. > >> > > >>> > >> > > >>> My thought is we choose one of two options: > >> > > >>> > >> > > >>> Option A: > >> > > >>> 1) check if any StatusLogger is writing to standard out or > >> standard > >> > > >> error. > >> > > >>> If not, add one. > >> > > >>> 2) stop any loggers that don't write to standard out or standard > >> error. > >> > > >>> > >> > > >>> Option B: > >> > > >>> 1) stop any loggers that don't write to standard out or standard > >> error. > >> > > >>> > >> > > >>> Option A could cause the log messages to be split across two > >> > > >> destinations, > >> > > >>> but they all get sent somewhere. Option B could lose shutdown > >> messages > >> > > >> when > >> > > >>> writing to a file, but by that point it may not matter. > >> > > >>> > >> > > >>> If any of you have a better idea, I'm happy to implement it. If > >> nobody > >> > > >>> weighs in on the best option, I'll probably submit Option A as a > >> pull > >> > > >>> request on Friday or Saturday. > >> > > >>> > >> > > >>> Tim > >> > > >>> > >> > > >> > >> > > >> > >> > > >> > >> > > > >> > > > >> > > >