gnodet commented on code in PR #12694:
URL: https://github.com/apache/maven/pull/12694#discussion_r4357263780
##########
impl/maven-logging/src/main/java/org/apache/maven/slf4j/MavenSimpleLogger.java:
##########
@@ -39,14 +39,90 @@ public class MavenSimpleLogger extends MavenBaseLogger {
private String warnRenderedLevel;
private String errorRenderedLevel;
Review Comment:
Not a dumb question at all — it's actually the most fundamental
architectural choice in this PR.
The reason we didn't go pure JUL: Maven already has SLF4J as its logging
facade (plugins use `Logger` from SLF4J, not JUL), and the SLF4J→JUL bridge
(`SLF4JBridgeHandler`) goes in the wrong direction for our needs. We'd need
SLF4J events to flow *into* JUL for `LogRecord` capture, which means
reinstalling the bridge, which defeats the purpose.
The current approach goes the other way: we replace the JUL root handler
with `MavenJulHandler` so that JUL events from frameworks (Hibernate, Netty,
etc.) get captured into our structured `LogEvent` pipeline. SLF4J events flow
through `MavenBaseLogger`/`MavenSimpleLogger` which already knows about
`LogSink`.
If we based everything on JUL `LogRecord`: we'd need to go
SLF4J→JUL→LogRecord, which adds a round-trip and loses some SLF4J marker/MDC
information. The current architecture keeps SLF4J events on the fast path
(direct `LogSink.accept()` in `MavenBaseLogger`) and uses JUL only as the
capture point for non-SLF4J frameworks.
That said: if you think `LogRecord` is the right primitive and we should
consolidate everything there, it's a bigger rethink — worth discussing
separately. For this PR the goal was to establish the capture pipeline; we can
revisit the underlying primitive in a follow-up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
