[ 
https://issues.apache.org/jira/browse/LOG4J2-3627?focusedWorklogId=924535&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-924535
 ]

ASF GitHub Bot logged work on LOG4J2-3627:
------------------------------------------

                Author: ASF GitHub Bot
            Created on: 04/Jul/24 13:10
            Start Date: 04/Jul/24 13:10
    Worklog Time Spent: 10m 
      Work Description: vy commented on PR #2691:
URL: https://github.com/apache/logging-log4j2/pull/2691#issuecomment-2208951635

   > @vy Actually `%rEx` and `%xEx` both operate on a `ThrowableProxy` in 
existing implementation.
   
   I think this is a ~feature~ bug. `%rEx` should operate on the `Throwable`, 
just like `%ex`. Hence, I prefer this to be fixed.
   
   > So only `%ex` needs to rely on `Throwable#printStackTrace()`. However, the 
output of `Throwable#printStackTrace()` looks like below:
   > ...
   > If we want to do filtering, we can only check if a line contains target 
package in `options.getIgnorePackages()`.
   
   For one, this won't be exactly correct. You will be matching against the 
stack trace line, not the package name of the associated `StackTraceElement`. 
But I am not concerned about this loose end.
   
   My main concern is, implementing `packages` filtering on a `StringBuilder` 
containing a stack trace won't be trivial. You not only need to filter lines, 
but also report them ala `ThrowableProxyRenderer.appendSuppressedCount()`. We 
will just be duplicating the effort that is already there for 
`ThrowableProxyRenderer`.
   
   Even though [I earlier supported the idea of _"sticking to 
`Throwable#printStackTrace()` wherever 
possible"_](https://github.com/apache/logging-log4j2/pull/2691#issuecomment-2204076668),
 coming to think about it again... If a user provides either `packages` or 
`maxLineCount`, it is certain that they won't get a standard stack trace 
anymore. Given this, I suggest both `%ex` and `%rEx` delegate to 
`ThrowableProxyRenderer` (just like `%xEx`) iff either `packages`, 
`maxLineCount`, `suffix`, or `lineSeparator` is provided. This approach will 
have the following pros/cons:
   
   1. All stack trace dump mangling will be consolidated at one place – Great!
   1. `%ex` and `%rEx` will report package information unnecessarily – 
Acceptable (I will explain a fix for this!)
   1. Allocation cost will slightly increase – Acceptable
   
   Let me be more clear:
   
   1. If either `packages`, `maxLineCount`, `suffix`, or `lineSeparator` is 
provided,
   2. `ThrowablePatternConverter` will obtain a `ThrowableProxy`:
   ```
   event.getThrownProxy() != null ? event.getThrownProxy() : new 
ThrowableProxy(event.getThrown())
   ```
   3. `RootThrowablePatternConverter`, will find the root cause and wrap it 
with in `ThrowableProxy`:
   ```
   Throwable rootCause = Throwables.getRootCause(event.getThrown());
   new ThrowableProxy(rootCause);
   ```
   4. Both `ThrowablePatternConverter` and `RootThrowablePatternConverter` will 
use `ThrowableProxyRenderer` with the obtained `ThrowableProxy`.
   
   ### What about unsolicited package information?
   
   If we consolidate all stack trace mangling to `ThrowableProxyRenderer`, 
`%ex` and `%rEx` will report package information too. We can fix this by adding 
an extra `extendedClassInfoRendered` boolean parameter to its 
`formatWrapper()`. This flag needs to be passed all the way down to 
`ExtendedStackTraceElement#renderOn()`.
   
   ### Feedback
   
   @alan0428a, before starting to implement this, I really would like to hear 
your thoughts too. In the meantime, I will ask this to some other PMC members 
too.
   
   
   ### Thanks!
   
   @alan0428a, I can't thank you enough for this contribution and walking with 
us along this process. :bow: Please try to understand us for the thorough 
review process and see that it is not easy to come up with a good resolution, 
putting aside the difficulty of implementing that resolution. I am very very 
happy with interacting with you so far and please keep these contributions 
coming. It would be great to see you as a regular Log4j contributor! 
:heart_eyes:




Issue Time Tracking
-------------------

    Worklog Id:     (was: 924535)
    Time Spent: 4.5h  (was: 4h 20m)

> PatternLayout: %xEx{ [ "short" | depth]} not working
> ----------------------------------------------------
>
>                 Key: LOG4J2-3627
>                 URL: https://issues.apache.org/jira/browse/LOG4J2-3627
>             Project: Log4j 2
>          Issue Type: Bug
>          Components: Layouts
>    Affects Versions: 2.11.0, 2.11.1, 2.11.2, 2.12.0, 2.12.1, 2.13.0, 2.13.1, 
> 2.13.2, 2.14.0, 2.13.3, 2.14.1, 2.15.0, 2.16.0, 2.17.1, 2.17.0, 2.12.3, 
> 2.12.2, 2.18.0, 2.12.4, 2.17.2, 2.19.0
>            Reporter: Thorsten Heit
>            Assignee: Volkan Yazici
>            Priority: Minor
>          Time Spent: 4.5h
>  Remaining Estimate: 0h
>
> According to the documentation the patterns {{{}%xEx{short{}}}} or 
> {{{}%xEx{<num>{}}}} should limit the number of lines of a stack trace that is 
> logged. This doesn't work; instead, the complete stack trace is logged 
> (always!). This is in contrary to the patterns {{%ex}} or {{%rEx}} where this 
> works.
> In commit 9ff63b2e50be754ae394feda2c33d9e64fd0ab3a (2018-01-25) a change was 
> implemented because of LOG4J2-2195 (according to the commit message) that 
> removed the option of limiting the number of lines to output.
> Therefore all versions since 2.11.0 should be affected.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to