You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@logging.apache.org by Mikael Ståldal <mi...@apache.org> on 2017/11/30 21:02:43 UTC

[Log4j] Header/footer and thread context

I am looking at https://issues.apache.org/jira/browse/LOG4J2-2007

What is the expected behavior here? Should there be any thread context 
for header/footer?

I guess it should be consistent for sync and async logging, which it 
isn't right now. But maybe the async case is correct in not includin any 
thread context in header/footer, and the sync case incorrect?

Re: [Log4j] Header/footer and thread context

Posted by Mikael Ståldal <mi...@apache.org>.
This gets more and more confusing the more I dig into it.

SmtpAppender apparently supports PatternLayout conversions in the 
subject (which is not documented), but then are done on the very first 
log event sent through the SmtpAppender (or actually the SmtpManager, 
which can be shared among multiple SmtpAppenders). This is very confusing.

This feature was apparently added as part of 
https://issues.apache.org/jira/browse/LOG4J2-1192 by Gary. And I see now 
in the JIRA discussion that someone noticed this odd behaviour already.

We should fix this, I reopened LOG4J2-1192.

(You don't have to use ThreadContext, or async appender/logger to get 
this confusing behaviour.)


On 2017-12-03 12:54, Mikael Ståldal wrote:
> It does work for the subject and body, see the new unit test 
> SmtpAppenderAsyncTest i just pushed to Git master. And I think this 
> behaviour make sense and should be retained.
> 
> The question is about header/footer. I think that it doesn't make sense 
> to use ThreadContext lookups there. But maybe we need to document this, 
> and make the behaviour consistent (i.e. it should not work for sync 
> logging either, which it by accident do now).
> 
> 
> On 2017-11-30 23:35, Remko Popma wrote:
>> Generally it doesn’t make sense to expect ThreadLocal lookups to work 
>> with async appender/loggers. Another lookup should be used.
>>
>> I haven’t looked at the code but I expect that the subject is cached 
>> somehow. Perhaps this should be changed (to no caching) so that the 
>> user experience is consistent.
>>
>> (Shameless plug) Every java main() method deserves http://picocli.info
>>
>>> On Dec 1, 2017, at 6:05, Matt Sicker <bo...@gmail.com> wrote:
>>>
>>> I'm not sure if this use case makes much sense. Dynamic data like that
>>> makes more sense in the message itself, though I'm sure there are 
>>> several
>>> ways to do this.
>>>
>>>> On 30 November 2017 at 15:02, Mikael Ståldal <mi...@apache.org> wrote:
>>>>
>>>> I am looking at https://issues.apache.org/jira/browse/LOG4J2-2007
>>>>
>>>> What is the expected behavior here? Should there be any thread 
>>>> context for
>>>> header/footer?
>>>>
>>>> I guess it should be consistent for sync and async logging, which it 
>>>> isn't
>>>> right now. But maybe the async case is correct in not includin any 
>>>> thread
>>>> context in header/footer, and the sync case incorrect?
> 


Re: [Log4j] Header/footer and thread context

Posted by Mikael Ståldal <mi...@apache.org>.
It does work for the subject and body, see the new unit test 
SmtpAppenderAsyncTest i just pushed to Git master. And I think this 
behaviour make sense and should be retained.

The question is about header/footer. I think that it doesn't make sense 
to use ThreadContext lookups there. But maybe we need to document this, 
and make the behaviour consistent (i.e. it should not work for sync 
logging either, which it by accident do now).


On 2017-11-30 23:35, Remko Popma wrote:
> Generally it doesn’t make sense to expect ThreadLocal lookups to work with async appender/loggers. Another lookup should be used.
> 
> I haven’t looked at the code but I expect that the subject is cached somehow. Perhaps this should be changed (to no caching) so that the user experience is consistent.
> 
> (Shameless plug) Every java main() method deserves http://picocli.info
> 
>> On Dec 1, 2017, at 6:05, Matt Sicker <bo...@gmail.com> wrote:
>>
>> I'm not sure if this use case makes much sense. Dynamic data like that
>> makes more sense in the message itself, though I'm sure there are several
>> ways to do this.
>>
>>> On 30 November 2017 at 15:02, Mikael Ståldal <mi...@apache.org> wrote:
>>>
>>> I am looking at https://issues.apache.org/jira/browse/LOG4J2-2007
>>>
>>> What is the expected behavior here? Should there be any thread context for
>>> header/footer?
>>>
>>> I guess it should be consistent for sync and async logging, which it isn't
>>> right now. But maybe the async case is correct in not includin any thread
>>> context in header/footer, and the sync case incorrect?

Re: [Log4j] Header/footer and thread context

Posted by Remko Popma <re...@gmail.com>.
Generally it doesn’t make sense to expect ThreadLocal lookups to work with async appender/loggers. Another lookup should be used. 

I haven’t looked at the code but I expect that the subject is cached somehow. Perhaps this should be changed (to no caching) so that the user experience is consistent. 

(Shameless plug) Every java main() method deserves http://picocli.info

> On Dec 1, 2017, at 6:05, Matt Sicker <bo...@gmail.com> wrote:
> 
> I'm not sure if this use case makes much sense. Dynamic data like that
> makes more sense in the message itself, though I'm sure there are several
> ways to do this.
> 
>> On 30 November 2017 at 15:02, Mikael Ståldal <mi...@apache.org> wrote:
>> 
>> I am looking at https://issues.apache.org/jira/browse/LOG4J2-2007
>> 
>> What is the expected behavior here? Should there be any thread context for
>> header/footer?
>> 
>> I guess it should be consistent for sync and async logging, which it isn't
>> right now. But maybe the async case is correct in not includin any thread
>> context in header/footer, and the sync case incorrect?
>> 
> 
> 
> 
> -- 
> Matt Sicker <bo...@gmail.com>

Re: [Log4j] Header/footer and thread context

Posted by Matt Sicker <bo...@gmail.com>.
I'm not sure if this use case makes much sense. Dynamic data like that
makes more sense in the message itself, though I'm sure there are several
ways to do this.

On 30 November 2017 at 15:02, Mikael Ståldal <mi...@apache.org> wrote:

> I am looking at https://issues.apache.org/jira/browse/LOG4J2-2007
>
> What is the expected behavior here? Should there be any thread context for
> header/footer?
>
> I guess it should be consistent for sync and async logging, which it isn't
> right now. But maybe the async case is correct in not includin any thread
> context in header/footer, and the sync case incorrect?
>



-- 
Matt Sicker <bo...@gmail.com>