Skip to content

MS-12645: add file appender in logback config#152

Open
damienlibouton wants to merge 2 commits intoicure:mainfrom
damienlibouton:feature/ms-12645-add-file-appender-logback
Open

MS-12645: add file appender in logback config#152
damienlibouton wants to merge 2 commits intoicure:mainfrom
damienlibouton:feature/ms-12645-add-file-appender-logback

Conversation

@damienlibouton
Copy link
Contributor

The idea is to have the possibility to log the application server in files.

Then we can use the parameter -Dlogging.file.name="/your/path/to/app.log" to expose the logs in a file when starting the fatjar.

@LotuxPunk
Copy link
Contributor

Hey Damien 👋
We do not see any problem of having such a change, just that by default if we do not provide that argument, there will be log created with logging.file.name_IS_UNDEFINED as a file name, would it be possible to adapt this change to disable that logging output if the path isn't provided ?

@damienlibouton
Copy link
Contributor Author

Hello Clément,

Thank you for your review !

To enable a conditional activation, I need to update the Logback dependency to aversion > 1.5.24 (currently 1.5.18). However, it seems there is a conflict with the Logback version in kmap, or I might be missing something... In short, I'm stuck with this solution !

I also tried activating it using a specific spring profile. To do this, I had to rename the logback.xml to logback-spring.xml to access the spring profile values. However, after renaming the file, for some reason, Logback could no longer find the configuration xml, and I couldn’t figure out why....

My other proposal is to add the Janino dependency to enable the use of if conditions in the xml configuration file.
I've pushed a commit with this proposal, and my local testing seems to work with this solution...

Let me know if this approach is acceptable to you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants