[GitHub] logging-log4j2 pull request #225: LOG4J2-2405 Better handling of %highlight ...

[hupfdule <gi...@git.apache.org>]

GitHub user hupfdule opened a pull request:

    https://github.com/apache/logging-log4j2/pull/225

    LOG4J2-2405 Better handling of %highlight pattern when using jul-bridge	

    This patch fixes some glitches with the `%highlight` pattern and therefore fixes the issue [LOG4J2-2405](https://issues.apache.org/jira/browse/LOG4J2-2405).
    
    It provides the following changes:
    
    - do not print out "null" in front of log messages with unknown levels
    - allow to specify a style for unknown log levels
    
    The patch contains the actual code changes as well as some tests.
    
    However there is one small problem with this solution. To allow styles for unknown log levels the `HighlightConverter` creates these new log levels itself with an int value of `Integer.MAX_VALUE`. The problem is, when the actual application code tries to register this log level itself, the given int value will be ignored and `Integer.MAX_VALUE` remains. This is due to the implementation of `Level.forName(String, int)`, which specifies that the intValue is ignored when called a second time with the same level name.
    (This is actually not a problem when the unknown log level is registered before parsing the log4j2 config file, but we can not enforce this.)
    
    I can think of two solutions to this problem:
    
    1. Rewrite `Level.forName(String, int)` to always _update_ the intValue with the given one.
    2. Change the `HighlightConverter` to store the mapping of levels to styles not as `Map<Level, String>`, but instead as `Map<String, String>` where the key is the levels _name_.
    
    I see option 1 as the "more correct" one, since I think when calling `Level.forName(String, intValue)` the user would expect that the last definition wins, not the first one (or at least an Exception should be thrown and there needs to be a ways to override the existing definition of a log level).
    However it is also a more intrusive change as it changes one of the base classes of the API and changes the behaviour of a public method.
    
    Option 2 is less intrusive as it only changes some internals of `HighlightConverter`. However I still think that the current behaviour of `Level.forName(String, intValue)` is incorrect and should actually be changed.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/hupfdule/logging-log4j2 LOG4J2-2405_highlight_pattern

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/logging-log4j2/pull/225.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #225
    
----
commit 0678c41957b0d4cddecf534c0294736b6f1ee857
Author: Marco Herrn <ma...@...>
Date:   2018-10-24T11:24:48Z

    [LOG4j2-2405] Don't prepend "null" to uknown levels on highlighting

commit 4731fc435e388f5a30b65d0b688c483b6ebe4509
Author: Marco Herrn <ma...@...>
Date:   2018-10-24T11:26:20Z

    [LOG4J2-2405] Added test case for issue

commit 0662eaea04cdbfb6ff4416876603192f88cf00a9
Author: Marco Herrn <ma...@...>
Date:   2018-10-24T11:25:46Z

    [LOG4J2-2405] Allow highlighting of unknown Levels

----


---

[GitHub] logging-log4j2 pull request #225: [LOG4J2-2405] Better handling of %highligh...

[asfgit <gi...@git.apache.org>]

Github user asfgit closed the pull request at:

    https://github.com/apache/logging-log4j2/pull/225


---