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 > >>> > >> > >> > >> > >
