Creating custom Levels in HighLightConverter feels wrong to me. It’s possible to register custom log levels programmatically as well as in configuration (https://logging.apache.org/log4j/2.x/manual/customloglevels.html).
If the user has not defined the log levels by the time the HighLightConverter is used, the user made a mistake. The existing code handles this mistake in a strict manner, and logs an error to the console. I think that’s good, it’s the best we can do to notify the user of their mistake. (Please revert the error logging. ) Now, how to improve things to handle this situation gracefully? What about changing the map of styles in HighLightConverter to use the level _name_ (String) as key instead of Level objects? That would allow us to register the style even though the custom Level has not been defined yet. If this Level is defined and used later, the name-based lookup will find the associated style. Remko. (Shameless plug) Every java main() method deserves http://picocli.info > On Oct 24, 2018, at 21:31, Marco Herrn <[email protected]> wrote: > > Hello, > > I have prepared a pull request for LOG4J2-2405. > There is still one decision to be made how to handle custom log levels. > I added my comments to the pull request, but duplicate it here to have a > broader audience for discussing it: > > 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. > > ---- > > Please share your idea about this. > > JIRA-Ticket: https://issues.apache.org/jira/browse/LOG4J2-2405 > Pull-Request: https://github.com/apache/logging-log4j2/pull/225 > > Best regards > Marco
