Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split log configuration into system and user entries #516

Open
kaikreuzer opened this issue Aug 13, 2017 · 14 comments
Open

Split log configuration into system and user entries #516

kaikreuzer opened this issue Aug 13, 2017 · 14 comments
Labels
bounty Issues with a Bountysource reward and the PRs that solve these enhancement An enhancement or new feature

Comments

@kaikreuzer
Copy link
Member

kaikreuzer commented Aug 13, 2017

Currently, the logging configuration is stored in userdata/etc/org.ops4j.pax.logging.cfg. This file either gets overwritten by updates or not - but we do not have a chance to do adjustments in the system configuration without wiping any changes the user might have done.

As we have moved to log4j2 now, there is chance to address this: log4j2 supports multiple configuration files.
It would therefore be ideal to have the system config stored in runtime/etc, while the user config could reside in conf or userdata/etc (in case it is possible to edit the file e.g. through logging changes in the console).

@kaikreuzer kaikreuzer added the bounty Issues with a Bountysource reward and the PRs that solve these label Aug 13, 2017
@cweitkamp
Copy link
Contributor

@kaikreuzer Can you give any hint where to start looking to implement this feature?

@kaikreuzer
Copy link
Member Author

Not really. I guess 80% of the work for this issue is finding the right place and way how to implement it :-)
The best starting point for the investigation would imho be the new feature in log4j2 itself and the Karaf logging documentation - this content about log4j2 wasn't there yet last week, so it must be pretty fresh :-)

We might think about moving from property files to XML files for the configuration as well (as it is also used in the docs now) - I somewhere read that XML includes should also work, so this might be another option for split files.

One thing to also keep in mind is how to deal with logging configuration changes through the Karaf shell - the log:set commands update the logging configuration files, which can be quite neat - but we must also ensure that nothing is written to conf and runtime as these folders are considered to be read-only.

Hope this helps to get an idea about the issue and where to look at!

@cweitkamp
Copy link
Contributor

I did some research. At the first try I got totally lost and gave up. A few days ago I coincidentally saw your comment in openhab/openhab-docs#476 containing the link to the new released documentation of Karaf logging. This manual is much better than the old version. Here are some facts I discovered so far. Now we have to put them together:

  • The initial log configuration is always loaded from etc/org.ops4j.pax.logging.cfg
  • You can add a log4j v2 XML-based configuration file by adding this line to the above file
org.ops4j.pax.logging.log4j2.config.file=${karaf.etc}/log4j2.xml
  • XML configuration files can include other files with XInclude
  • You can specify one or more comma separated XML file names (including path) in the log4j.configurationFile SystemPropertiy

The first three points definitely makes sense and I am sure we can work out a solution for our problem statement.
The fourth point sounds very interesting, but doesn't really fit into the scheme. It seems to be an option, if you are able to set it in some lines of code (see example).

@wborn
Copy link
Member

wborn commented Dec 9, 2017

The fourth point sounds very interesting, but doesn't really fit into the scheme. It seems to be an option, if you are able to set it in some lines of code (see example).

I don't think you need to add it to a piece of code. You can just set it using the JAVA_OPTS environment variable. I've used it before to suppress logging in Maven builds. :-)

E.g. -Dlog4j.configurationFile=log4j2-A.xml,log4j2-B.xml,log4j2-C.xml

@kaikreuzer
Copy link
Member Author

@wborn So would it be as simple as adding such a parameter to our setenv scripts?

@wborn
Copy link
Member

wborn commented Dec 9, 2017

After a very very brief look I don't think Pax will pickup such logging configurations so it will probably not be visible/editable from the console. You'll be directly configuring Log4j2 with this and have to hope other logic leaves such configuration alone.

@wborn
Copy link
Member

wborn commented Dec 10, 2017

You can add a log4j v2 XML-based configuration file by adding this line to the above file
org.ops4j.pax.logging.log4j2.config.file=${karaf.etc}/log4j2.xml

There is an issue about log:get and log:set commands not properly working when using a log4j2.xml file (see KARAF-5354). So for proper functionality we would need to upgrade to Karaf 4.2.0 or newer.

@kaikreuzer
Copy link
Member Author

Hm, ok, so nothing for the 2.2 release then. Ok for me to live with the single file for the moment.

@wborn
Copy link
Member

wborn commented Dec 12, 2017

I've made local build with OH 2.2.0 and Karaf 4.2.0.M1 and tested the implementation of using a log4j2.xml:

  • Using a XML configuration works pretty well, KARAF-5354 is also solved in this version so log:get and log:set work properly. These commands just read/write and create children in the <Loggers> element.
  • log:set default loggername also works, entries remain in the file but without the level= attribute, they just don't show up in a log:list and survive restarts
  • It can read configurations that have XIncludes like <xi:include href="log4j-xinclude-appenders.xml" /> but as soon as you do a log:set a configuration is written with the fully substituted contents of these includes with a xml:base attribute, e.g.
    <Appenders xml:base="log4j-xinclude-appenders.xml">...<Appenders>

So in its current state you can not easily use it for splitting up configurations with XIncludes. It would be a nice replacement for properties files though!

@AngelosF
Copy link
Member

AngelosF commented Nov 18, 2018

Is this still worth pursuing ? (or what @wborn mentioned in the last comment are show stoppers?)

I have an idea about a proper implementation but I have some questions:

  1. Where should we place the log config file?
  2. Which format should we use? (I like xml the best versus json and/or yaml)

etc/org.ops4j.pax.logging.cfg only 1 line:

org.ops4j.pax.logging.log4j2.config.file=${openhab.etc}/services/log4j2.???

I can make some tests and propose a solution.

@hakan42
Copy link
Contributor

hakan42 commented Nov 18, 2018

Is this still worth pursuing ? (or what @wborn mentioned in the last comment are show stoppers?)
I can make some tests and propose a solution.

I'd love whatever solution allows me to split up the log files according to bindings.
This would help tremendously, at least while developing.

@kaikreuzer
Copy link
Member Author

@AngelosF Yes, definitely worth pursuing!

Where should we place the log config file?

conf/ or conf/services/ for the user editable parts
runtime/etc for the static (read-only) system parts

Which format should we use? (I like xml the best versus json and/or yaml)

It seems as if everyone is using XML for log4j2, so we should do the same.

@splatch
Copy link
Contributor

splatch commented Jan 28, 2019

FYI it is possible to have "per bundle" logging thanks to MDC/NDC - see configuration sample https://issues.apache.org/jira/browse/LOG4J2-2021.

@rkoshak
Copy link
Contributor

rkoshak commented Sep 5, 2019

I can make some tests and propose a solution.

@AngelosF, did you make any progress on this in your tests? I started looking into this too but I don't want to waste my time if you've already discovered some limitations or problems.

@wborn wborn added the enhancement An enhancement or new feature label Sep 29, 2019
wborn added a commit to wborn/openhab-distro that referenced this issue Feb 22, 2020
Note that Karaf processes the log4j2 configuration to add the sshd logger and this processing of XML elements is case sensitive.
It will throw exceptions if lower case element names are used or when the root logger is at the end of the list of loggers.

When new loggers are added, the XML configuration is serialized such that:
* The <Configuration> element is added to the same line as the <?xml> element
* Attributes are sorted so therefor the 'name' is at the end of the appender/logger lines

Related to openhab#516

Signed-off-by: Wouter Born <github@maindrain.net>
kaikreuzer pushed a commit that referenced this issue Feb 23, 2020
Note that Karaf processes the log4j2 configuration to add the sshd logger and this processing of XML elements is case sensitive.
It will throw exceptions if lower case element names are used or when the root logger is at the end of the list of loggers.

When new loggers are added, the XML configuration is serialized such that:
* The <Configuration> element is added to the same line as the <?xml> element
* Attributes are sorted so therefor the 'name' is at the end of the appender/logger lines

Related to #516

Signed-off-by: Wouter Born <github@maindrain.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty Issues with a Bountysource reward and the PRs that solve these enhancement An enhancement or new feature
Projects
None yet
Development

No branches or pull requests

7 participants