[
https://issues.apache.org/jira/browse/LOG4J2-555?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13923194#comment-13923194
]
Remko Popma commented on LOG4J2-555:
------------------------------------
I won't have time to work out the details for another 2 weeks or so, so let me
draw the big picture of the refactoring I have in mind.
The goal of the refactoring is to end up with a single {{doLog()}} method that
both AbstractLogger and Extended/Custom Loggers can call.
So for example in AbstractLogger
{code}
public void error(final String message, final Object... params) {
if (isEnabled(Level.ERROR, null, message, params)) {
final Message msg = messageFactory.newMessage(message, params);
log(null, FQCN, Level.ERROR, msg, msg.getThrowable());
}
}
{code}
the above would become
{code}
public void error(final String message, final Object... params) {
doLog(Enabled.stringVarargs, null, FQCN, Level.ERROR,
MsgBuilder.stringVarargs,
message, null, params);
}
{code}
The idea is to create enums for all {{isEnabled()}} methods, and passing this
enum value to the {{doLog}} method instead of calling the {{isEnabled}} method
directly in {{error()}}.
The new {{doLog}} method would look like this:
{code}
private void doLog(Enabled enabled, Marker marker, String FQCN, Level level,
MsgBuilder builder,
String message, Throwable throwable, Object... params) {
Message msg = builder.build(message, throwable, params);
if (enabled.isEnabled(this, level, marker, msg, throwable)) {
log(marker, FQCN, level, msg, msg.getThrowable());
}
}
{code}
Each of the 5 enum values for {{Enabled}} has a different implementation of the
{{isEnabled}} method that ignores some parameters, so we end up with the same
behaviour as what AbstractLogger currently does, with a few extra method
invocations inbetween (that Hotspot can easily optimize out).
A similar enum mechanism can be used to create the {{Message}} object.
So the trade-off is we gain a simpler implementation for the existing
convenience methods
(6 levels times 14 methods = 84 methods where we go from 3-4 lines to 1 line).
And in return we need to add the new {{doLog}} method above as well as enums
for Enabled and MsgBuilder.
The enums can be protected static so they can be re-used in Extended/Custom
loggers.
This also means there would be no need for complex tracking of FQCNs in the
class hierarchy.
Thoughts?
> Location-based functionality broken in AbstractLoggerWrapper subclasses
> -----------------------------------------------------------------------
>
> Key: LOG4J2-555
> URL: https://issues.apache.org/jira/browse/LOG4J2-555
> Project: Log4j 2
> Issue Type: Bug
> Components: API, Core
> Affects Versions: 2.0-rc1
> Reporter: Remko Popma
> Assignee: Remko Popma
> Fix For: 2.0-rc2
>
>
> *How to reproduce*
> * Create a custom logger that extends {{AbstractLoggerWrapper}} (or generate
> one with the tool attached to LOG4J2-519)
> * In the custom logger provide a public method that invokes the {{log(Level,
> String)}} method
> * Configure a pattern layout that uses location, like %C for the logger FQCN
> * From a sample app, call the public method on your custom logger.
> * The output will show the class name of the custom logger instead of the
> class name of the calling class in the sample application.
> *Cause*
> {{AbstractLogger}}'s FQCN field is {{static final}} and initialized to
> {{AbstractLogger.class.getName()}}. Then, in
> {{Log4jLogEvent#calcLocation()}}, when walking over the stack trace elements,
> the element _following_ the FQCN is returned. So only loggers that directly
> subclass from {{AbstractLogger}} will work correctly. Loggers that inherit
> from {{AbstractLoggerWrapper}} are two levels removed from {{AbstractLogger}}
> and the {{calcLocation()}} method will not work correctly.
> *Solution*
> I think {{AbstractLogger}}'s FQCN field should be made non-static, and
> initialized to {{getClass().getName()}} in the constructor of
> {{AbstractLogger}}. {{Log4jLogEvent#calcLocation()}} can then be modified to
> return the {{StackElement}} whose class name matches the FQCN, instead of the
> next element. Location-based functionality should then work for arbitrarily
> deep subclass hierarchies of AbstractLogger.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
