logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Joanne Polsky (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (LOG4J2-160) Include option in throwable pattern converter to control stack trace separator
Date Tue, 05 Feb 2013 23:39:13 GMT

    [ https://issues.apache.org/jira/browse/LOG4J2-160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13571694#comment-13571694
] 

Joanne Polsky edited comment on LOG4J2-160 at 2/5/13 11:38 PM:
---------------------------------------------------------------

I’ve attached the first patch for review; LOG4J2-160-1.patch.  I'd like to split this change
into 2 changes lists if possible to reduce complexity.  The first being the change to include
the new separator option and apply it to the pattern converters.  The second would address
the performance improvements for ExtendedThrowablePatternConverter and RootThrowablePatternConverter
in ThrowableProxy; there are some additional complexities that I would like to discuss outside
of the context of this initial change.

I’ve created a new class called ThrowableFormatOptions which contains the parsing logic
for the %throwable, %xThrowable and %rThrowable  patterns.  Each will support a new option
to have a custom stack trace separator, for instance:
%ex{full}{separator(|)}
%ex{separator(|)}

This works fine for ThrowablePatternConverter, however, for ExtendedThrowablePatternConverter/RootThrowablePatternConverter
there seems to be inconsistency between how options are delimited.  From the user guide (http://logging.apache.org/log4j/2.x/log4j-users-guide.pdf),
these patterns can use a comma delimiter:
%rEx["none"|"short"|"full"|depth],[filters(packages)}

However, this doesn't seem consistent with the way other patterns delimit the options by enclosing
them in braces { }, for instance the following seems more correct:
%rEx[{("none"|"short"|"full"|depth)}][{filters(packages)}]

I copied the existing code to support the case where 2 options are specified in one "{full,filters_or_separator}",
but I didn't add support for the case where all 3 options are specified in one "{full,filters,separator}".
 It complicates parsing considerably since filters uses commas to delimit the packages as
well.  It requires these classes to use custom parsing techniques that are outside the standard
framework which doesn't feel natural.  Is is possible to change the recommended usage in the
user guide to use the standard delimiters instead?  We can leave the support for 2 options
in one delimited by a comma, or since this is still considered beta, is it acceptable to drop
the comma syntax in favor of the braces exclusively?

                
      was (Author: jpolsky):
    I’ve attached the first patch for review; LOG4J2-160-1.patch.  I'd like to split this
change into 2 changes lists if possible to reduce complexity.  The first being the change
to include the new separator option and apply it to the pattern converters.  The second would
address the performance improvements for ExtendedThrowablePatternConverter and RootThrowablePatternConverter
in ThrowableProxy; there are some additional complexities that I would like to discuss outside
of the context of this initial change.

I’ve created a new class called ThrowableFormatOptions which contains the parsing logic
for the %throwable, %xThrowable and %rThrowable  patterns.  Each will support a new option
to have a custom stack trace separator, for instance:
%ex{full}{separator(|)}
%ex{separator(|)}

This works fine for ThrowablePatternConverter, however, for ExtendedThrowablePatternConverter/RootThrowablePatternConverter
there seems to be inconsistency between how options are delimited.  From the user guide (http://logging.apache.org/log4j/2.x/log4j-users-guide.pdf),
these patterns can use a comma delimiter:
%rEx["none"|"short"|"full"|depth],[filters(packages)}

However, this doesn't seem consistent with the way other patterns delimit the options by enclosing
them in braces { }, for instance the following seems more correct:
%rEx[{("none"|"short"|"full"|depth)}][{filters(packages)}]

I copied the existing code to support the case where 2 options are used {full,filters_or_separator),
but I didn't add support for the case where all 3 options are used {full,filters,separator}.
 It complicates parsing considerably since filters uses commas to delimit the packages as
well.  It requires these classes to use custom parsing techniques that are outside the standard
framework which doesn't feel natural.  Is is possible to change the recommended usage in the
user guide to use the standard delimiters instead?  Since this is still considered beta, is
it acceptable to drop the comma syntax in favor of the braces exclusively?

                  
> Include option in throwable pattern converter to control stack trace separator
> ------------------------------------------------------------------------------
>
>                 Key: LOG4J2-160
>                 URL: https://issues.apache.org/jira/browse/LOG4J2-160
>             Project: Log4j 2
>          Issue Type: Improvement
>          Components: Core
>            Reporter: Joanne Polsky
>            Priority: Trivial
>         Attachments: LOG4J2-160-1.patch
>
>
> Copying feature request from Bugzilla 1.x:
> https://issues.apache.org/bugzilla/show_bug.cgi?id=51122

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org


Mime
View raw message