anthonyraymond / joal

An open source command line RatioMaster with an optional WebUI.
Apache License 2.0
572 stars 63 forks source link

async logging, start logging to file #189

Closed laur89 closed 1 year ago

laur89 commented 1 year ago

I wouldn't squash these 2 commits, as they demonstrate two separate ways of making log4j2 logging async.

laur89 commented 1 year ago

If you'd prepare to keep effective config as-is (ie console only) and make file-logging opt-in via sys config, leave a note.

anthonyraymond commented 1 year ago

Not sure about this feature 🤔 why do you want to introduce aync logging in an already high asynchronous app.

Also, you've removed the shutdownhook config in log4j file but i'm still managing the logger shutdown from code. That would cause issue

laur89 commented 1 year ago

Also, you've removed the shutdownhook config

https://github.com/anthonyraymond/joal/pull/189/files#diff-8ff88692f6f1c189efcc2d347464c74683dc25dcd63bba9359b8820a55f06a64R14

why do you want to introduce aync logging in an already high asynchronous app

I titled the PR bit wrong - main bit is adding file logging.

anthonyraymond commented 1 year ago

Actually there is already a wiki for that https://github.com/anthonyraymond/joal/wiki/Redirect-log-to-file It keeps the shutdown hook as is. it's important to keep that this way to prevent log loss on shut down.

laur89 commented 1 year ago

Note my change doesn't modify the hook either. But considering the wiki it's ok to close the PR

anthonyraymond commented 1 year ago

Woops i've missread the code 😄 indead the hook was still here.