On 17.09.2011, at 21:02, John Vasileff wrote:
>
> On Sep 17, 2011, at 2:40 PM, Joern Huxhorn wrote:
>
>>
>> On 17.09.2011, at 18:47, John Vasileff wrote:
>>
>>>
>>> On Sep 17, 2011, at 8:53 AM, Joern Huxhorn wrote:
>>>
>>>> Adding getThrowable() would also reintroduce the issue that such a Message
>>>> could not be serialized securely since deserialization would require the
>>>> exceptions class on the classpath. In my ParameterizedMessage over at
>>>> https://github.com/huxi/slf4j/blob/slf4j-redesign/slf4j-n-api/src/main/java/org/slf4j/n/messages/ParameterizedMessage.java
>>>> the Throwable is transient so it is deliberately lost while serializing,
>>>> i.e. it must be taken care of by the framework immediately after the
>>>> message is created.
>>>>
>>>> It is only contained in ParameterizedMessage at all simply because a
>>>> method can only return a single value. And the create-method is also
>>>> responsible for resolving the Throwable at the last position of arguments
>>>> if it is not used up by a corresponding {}.
>>>>
>>>> Yes, it would be "cleaner" to split that into a parseresult object that
>>>> contains both the message and an additional, optional Throwable. I just
>>>> didn't implement it that way to reduce the amount of garbage (that
>>>> parseresult object would get garbage-collected immediately after
>>>> evaluation).
>>>>
>>>> So adding Throwable to ParameterizedMessage was just a performance
>>>> optimization.
>>>>
>>>> The serialization issue is btw also the reason for *not* having Object[]
>>>> getParameters() but String[] getParameters() instead. The String
>>>> representation is the serialization-safe version of the parameters.
>>>>
>>>> But all of this is, in my books, an implementation detail of the Message
>>>> implementation with getFormattedMessage() being the only "public" API of
>>>> Message.
>>>>
>>>> Joern.
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: [email protected]
>>>> For additional commands, e-mail: [email protected]
>>>>
>>>
>>> Interesting point on serialization. When would you see serialization
>>> happening? Is this primarily for appenders?
>>>
>>
>> SocketAppender is using serialization. Since I'm the author of Lilith (
>> http://lilith.huxhorn.de/ ) I tend to focus on stuff like that.
>>
>> This is also the reason for the differentiation between the Message instance
>> and the (laziliy) formatted message string. A SocketAppender does not have
>> any need for a formatted message. It is perfectly valid to skip the
>> formatting entirely and simply transmit the message pattern and the message
>> parameters (as Strings) to safe some CPU in the logged application.
>>
>> Directly converting parameters into Strings as soon as we know that an event
>> really needs to be created is crucial to make sure that the logging
>> framework is not lying as is happening, for example, in this case:
>> http://techblog.appnexus.com/2011/webkit-chrome-safari-console-log-bug/
>> A lying logging framework is really, really bad...
>>
>> Joern.
>
> So, the two most significant points are:
>
> 1) ambiguity with info(Message msg, Throwable throwable) where for all
> practical purposes Message will at least sometimes also have a throwable.
> Minor additional point that having log methods with explicit Throwables adds
> several methods to the Logger interface.
>
> 2) issue with serializing appenders having to special case 'transient
> Throwable' in message objects (i.e. the appender would have to call
> getThrowable(), convert to String, and store separately from the message
> object.)
>
> My opinion is that Logger is by far the most important interface and design
> decisions should give preference to application developer facing interfaces.
> Thoughts?
The code simply looks like this:
public void log(Level level, String messagePattern, Object... args) {
if (!isEnabled(level)) return;
ParameterizedMessage message =
ParameterizedMessage.create(messagePattern, args);
performLogging(level, null, message, message.getThrowable());
}
public void log(Level level, Marker marker, String messagePattern, Object...
args) {
if (!isEnabled(level, marker)) return;
ParameterizedMessage message =
ParameterizedMessage.create(messagePattern, args);
performLogging(level, marker, message, message.getThrowable());
}
public void log(Level level, Message message) {
if (!isEnabled(level)) return;
if (message instanceof ParameterizedMessage) {
performLogging(level, null, message, ((ParameterizedMessage)
message).getThrowable());
} else {
performLogging(level, null, message, null);
}
}
public void log(Level level, Message message, Throwable throwable) {
if (!isEnabled(level)) return;
performLogging(level, null, message, throwable);
}
public void log(Level level, Marker marker, Message message) {
if (!isEnabled(level, marker)) return;
if (message instanceof ParameterizedMessage) {
performLogging(level, marker, message, ((ParameterizedMessage)
message).getThrowable());
} else {
performLogging(level, marker, message, null);
}
}
public void log(Level level, Marker marker, Message message, Throwable
throwable) {
if (!isEnabled(level, marker)) return;
performLogging(level, marker, message, throwable);
}
If a method with just a Message but no Throwable is called then this Message is
checked if it is a ParameterizedMessage. Otherwise null is used for the
Throwable. If, however, a method with an explicit Throwable is used then this
Throwable is used for logging.
The whole point of having the Throwable in ParameterizedMessage is simply
because create can only return a single value without creating additional
overhead caused by wrappers. The Throwable is not meant to be part of the
Message, it just happens to be like that directly after creation - but not
after deserialization! It's the job of the logging framework to handle the
Throwable in a sensible way.
Perhaps making getThrowable() package-private would make this more clear but
I'd have to move classes around in a non-logical way just to achieve this. Do
not want.
Joern.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
