Re: [PR] Consolidate stack trace rendering in Pattern Layout (logging-log4j2)

2024-09-27 Thread via GitHub
rgoers commented on PR #2691: URL: https://github.com/apache/logging-log4j2/pull/2691#issuecomment-2380229314 Well shucks, while you were in there it would be nice to resolve LOG4J2-2170 and add in the module information. -- This is an automated message from the Apache Git Service. To res

Re: [PR] Consolidate stack trace rendering in Pattern Layout (logging-log4j2)

2024-09-06 Thread via GitHub
ppkarwasz commented on code in PR #2691: URL: https://github.com/apache/logging-log4j2/pull/2691#discussion_r1746997789 ## log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowableRenderer.java: ## @@ -0,0 +1,290 @@ +/* + * Licensed to the Apache Software Foundati

Re: [PR] Consolidate stack trace rendering in Pattern Layout (logging-log4j2)

2024-09-06 Thread via GitHub
vy commented on code in PR #2691: URL: https://github.com/apache/logging-log4j2/pull/2691#discussion_r1746962343 ## log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowableRenderer.java: ## @@ -0,0 +1,290 @@ +/* + * Licensed to the Apache Software Foundation (ASF

Re: [PR] Consolidate stack trace rendering in Pattern Layout (logging-log4j2)

2024-09-05 Thread via GitHub
ppkarwasz commented on code in PR #2691: URL: https://github.com/apache/logging-log4j2/pull/2691#discussion_r1745113946 ## log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/ThrowableRenderer.java: ## @@ -0,0 +1,290 @@ +/* + * Licensed to the Apache Software Foundati

Re: [PR] Consolidate stack trace rendering in Pattern Layout (logging-log4j2)

2024-08-15 Thread via GitHub
alan0428a commented on code in PR #2691: URL: https://github.com/apache/logging-log4j2/pull/2691#discussion_r1718514492 ## log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingAppenderOnStartupTest.java: ## Review Comment: Change to `REPLACE_E

Re: [PR] Consolidate stack trace rendering in Pattern Layout (logging-log4j2)

2024-08-13 Thread via GitHub
github-actions[bot] commented on PR #2691: URL: https://github.com/apache/logging-log4j2/pull/2691#issuecomment-2286062002 Job Requested goals Build Tool Version Build Outcome Build ScanĀ® build-

Re: [PR] Consolidate stack trace rendering in Pattern Layout (logging-log4j2)

2024-07-09 Thread via GitHub
rgoers commented on PR #2691: URL: https://github.com/apache/logging-log4j2/pull/2691#issuecomment-2218458104 I do like Piotr's proposal of configuring the stack trace as a sub-element. However, I would do that as a separate follow-on item. The work being proposed here is already fairly inv

Re: [PR] Consolidate stack trace rendering in Pattern Layout (logging-log4j2)

2024-07-08 Thread via GitHub
vy commented on PR #2691: URL: https://github.com/apache/logging-log4j2/pull/2691#issuecomment-2215038458 > It seems to me this PR is more complicated then I thought and is more like refactoring now. But I will try my best to understand it. @alan0428a, thanks so much for your patience

Re: [PR] Consolidate stack trace rendering in Pattern Layout (logging-log4j2)

2024-07-08 Thread via GitHub
alan0428a commented on PR #2691: URL: https://github.com/apache/logging-log4j2/pull/2691#issuecomment-2214111273 I don't have feedback on deprecating `ThrowableProxy`, as you are more familiar with the historical background than I am. > // You need `StackLocatorUtil.getCurrentStackTrace(

Re: [PR] Consolidate stack trace rendering in Pattern Layout (logging-log4j2)

2024-07-08 Thread via GitHub
ppkarwasz commented on PR #2691: URL: https://github.com/apache/logging-log4j2/pull/2691#issuecomment-2213746303 > @ppkarwasz, agreed, but that would be a breaking change, right? IMHO, it stretches the focus of this PR far too much. Let's just start with consolidating the existing `Throwabl

Re: [PR] Consolidate stack trace rendering in Pattern Layout (logging-log4j2)

2024-07-08 Thread via GitHub
vy commented on PR #2691: URL: https://github.com/apache/logging-log4j2/pull/2691#issuecomment-2213682324 > I would rather retrofit `PatternConverter` with a generic parameter `T` to indicate the type of object supported by the converter. @ppkarwasz, agreed, but that would be a breaki

Re: [PR] Consolidate stack trace rendering in Pattern Layout (logging-log4j2)

2024-07-08 Thread via GitHub
ppkarwasz commented on PR #2691: URL: https://github.com/apache/logging-log4j2/pull/2691#issuecomment-2213652711 @vy, > 1. Create a new _internal_ (package-private, etc.) `ThrowableRenderer` class taking care of `Throwable` (not `ThrowableProxy`!) rendering such that: I wo

Re: [PR] Consolidate stack trace rendering in Pattern Layout (logging-log4j2)

2024-07-08 Thread via GitHub
vy commented on PR #2691: URL: https://github.com/apache/logging-log4j2/pull/2691#issuecomment-2213522269 @rgoers, I very much liked the idea of retiring `ThrowableProxy`. I analyzed the possibility of this. I will share my findings below. ### What does Logback do? AFAIK, Log4j

Re: [PR] Consolidate stack trace rendering in Pattern Layout (logging-log4j2)

2024-07-06 Thread via GitHub
rgoers commented on PR #2691: URL: https://github.com/apache/logging-log4j2/pull/2691#issuecomment-2211795872 As I recall ThrowableProxy was introduced so that LogEvents could be remotely shipped to remove systems. We no longer support doing that kind of thing using Java Serialization. So I

Re: [PR] Consolidate stack trace rendering in Pattern Layout (logging-log4j2)

2024-07-05 Thread via GitHub
alan0428a commented on PR #2691: URL: https://github.com/apache/logging-log4j2/pull/2691#issuecomment-2211071869 @vy Just want to make sure we are on the same page. So for `xxxThrowablePatternConverter#format()`, the existing implementation is that when `event.getThrownProxy()` i