Logging foundation: structured LogEvent, JUL handler, Log API enhancements

[gnodet]

Summary

Focused, reviewable PR that lays the logging infrastructure foundation for the build report feature chain. Each feature has been split into its own PR for focused review and discussion.

What's included

Log API enhancements (maven-api-core)

• Log.trace() — 5 overloads + isTraceEnabled() to separate Maven core internals (trace) from user-facing debug output (debug). Maps to SLF4J TRACE / JUL FINEST. All 6 methods use default implementations (no-ops) to avoid AbstractMethodError for existing third-party Log implementors.
• Log.child(name) — hierarchical sub-loggers for plugins that want to separate concerns while keeping level control. Default implementation returns this for backward compatibility.

Structured LogEvent (maven-api-core, maven-core)

• LogEvent / LogLevel API for structured log event representation
• Source metadata fields: sourceClassName(), sourceMethodName(), threadId() — populated for both Log API events (via DefaultLog.withMetadata()) and JUL events (via MavenJulHandler); null/-1 for direct SLF4J logging

Custom JUL Handler (maven-logging)

• MavenJulHandler replaces SLF4JBridgeHandler — all JUL events always route through SLF4J so that MavenSimpleLogger produces a consistent formattedMessage (with timestamp, logger name, and ANSI styling) regardless of origin
• JUL metadata (sourceClassName, sourceMethodName, threadId) is stashed in a ThreadLocal before the SLF4J call and read by ProjectBuildLogAppender during the same synchronous call chain — no metadata is lost
• Null loggerName guard per JUL spec (falls back to root logger)
• Three sources, one pipeline: Log API → SLF4J, SLF4J direct, JUL → MavenJulHandler → SLF4J — all converge on the same structured LogEvent

Structured LogSink (maven-logging, maven-core)

• MavenSimpleLogger.LogSink — structured callback with (level, loggerName, cleanMessage, formattedMessage, throwable) replacing the old Consumer<String> sink
• Throwable rendering unified: write() reuses the existing writeThrowable() method instead of duplicating rendering logic
• ProjectBuildLogAppender produces LogEvent objects (with source metadata when available) instead of raw strings
• BuildEventListener.projectLogMessage() now takes LogEvent instead of String

Mojo MDC & logger name (maven-core)

• maven.mojo.id MDC key set during mojo execution (format: prefix:goal@executionId)
• Fork-aware mojoId: FORKING_MOJO_ID ThreadLocal mirrors the existing FORKING_PROJECT_ID pattern — saves/restores mojo context across forked lifecycles
• Logger name changed from getFullGoalName() ("compiler:compile") to getImplementation() (FQCN like org.apache.maven.plugins.compiler.CompilerMojo) for proper hierarchical SLF4J level configuration

Performance

• StackWalker (used to resolve the caller method name for Log API events) is gated behind ProjectBuildLogAppender.hasReportCapture() — the ~1-5μs per-call cost is only paid when build report capture is active; normal builds set only the ThreadLocal (logger name + thread ID)

Bug fix

• DefaultLog.warn(Supplier<String>, Throwable) was calling logger.info() instead of logger.warn()

PR chain

#	PR	Feature
1	This PR	Logging foundation
2	#12695	Build report
3	#12697	Console modes
4	#12698	Warning mode + diagnostics
5	#12699	mvnlog viewer
6	#12702	Structured problems pipeline
7	#12714	TRACE level migration

Related

• Backport Log API enhancements and mojo MDC to 4.0.x #12690 (4.0.x backport of Log API changes)
• Report structured BuilderProblems from compiler diagnostics maven-compiler-plugin#1101 (compiler plugin using Log API)

Test plan

• DefaultLogTest — 4 tests: warn/supplier regression, metadata lifecycle, trace delegation, trace no-op
• MavenJulHandlerTest — 10 tests: parameterized JUL→SLF4J level mapping, FINEST→TRACE, CONFIG→INFO, metadata null check
• mvn test -pl impl/maven-core,impl/maven-logging — all tests pass
• Full CI validation
• IT suite with JUL-using plugins (verify metadata preservation)

🤖 Generated with Claude Code

[gnodet]

Well-designed foundational logging infrastructure. The structured LogEvent API, JUL handler, and Log API enhancements provide a solid base for the build report and console modes PRs. A few issues noted below.

Also noted:

• Good catch fixing warn(Supplier<String>, Throwable) calling logger.info() instead of logger.warn().
• The logger name change from getFullGoalName() to getImplementation() (FQCN) enables proper hierarchical SLF4J level configuration but is a behavioral change — worth mentioning in release notes for users who configured logging by short-form names.
• No unit tests were added for the new functionality (MavenJulHandler, DefaultLogEvent, StackWalker metadata capture, LogSink contract). Given this is foundational for the entire logging pipeline, targeted tests would increase confidence.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of gnodet

[gnodet]

The @return Javadoc says "always non-negative" but the default implementation returns -1, and DefaultLogEvent convenience constructors also pass -1. The threadId() method in this same interface correctly documents its sentinel ("or -1 if unavailable").

Suggested change

	}
	* @return the sequence number, or {@code -1} if unavailable

[gnodet]

Fixed — javadoc now reads @return the sequence number, or {@code -1} if unavailable.

[gnodet]

Fixed — javadoc now reads @return the sequence number, or {@code -1} if unavailable.

[gnodet]

formatForConsole() produces [LEVEL] message (no timestamp, no logger name), while MavenSimpleLogger produces a full formatted line with timestamp and logger name per configuration. This means LogEvent.formattedMessage() has an inconsistent format depending on whether the event originated from JUL or SLF4J. The formattedMessage() Javadoc promises "the level prefix, timestamp, and any ANSI styling" — JUL-sourced events would be missing the timestamp and logger name.

[gnodet]

Fixed — removed the formatForConsole() method entirely. All JUL events now always route through SLF4J so that MavenSimpleLogger produces a consistent formattedMessage (with timestamp, logger name, and ANSI styling) regardless of origin. The JUL metadata is stashed in a ThreadLocal before the SLF4J call so that ProjectBuildLogAppender can read it when constructing the LogEvent.

[ascheman]

Really solid foundation — the three-path convergence (Log API / SLF4J / JUL) onto one structured LogEvent is clean, and preserving the LogRecord metadata the stock SLF4JBridgeHandler drops is a genuine improvement. Nice catch on the warn(Supplier, Throwable) → logger.info() bug.

A few things worth a look before this becomes the base of the 7-PR chain — one API-compat question, one fork-context correctness question, one perf note, and some small nits. Nothing structural.

On tests (echoing the earlier note): the two I'd most want are a regression test asserting warn(Supplier, Throwable) actually logs at WARN, and a table test for the JUL→SLF4J level mapping (esp. FINEST→TRACE and CONFIG→INFO). Given the ThreadLocal/StackWalker plumbing, those would lock down the easy-to-regress bits.

[ascheman]

The six new trace methods (isTraceEnabled + 5 overloads) are abstract, while child(String) below ships with a default. Any existing third-party implementor of Log breaks — source and binary (AbstractMethodError) — on trace, but not on child. Log is @Experimental, so it's arguably in-bounds for 4.1.0, but the asymmetry looks unintentional. Could the trace methods get backward-compatible defaults — isTraceEnabled() returning false and the trace(…) overloads as no-ops — so existing implementations keep compiling and opt in by overriding? (DefaultLog overrides all of it, so the runtime path is unchanged.)

[gnodet]

Fixed — all 6 trace methods (isTraceEnabled + 5 overloads) now have default implementations: isTraceEnabled() returns false by default, and all trace(...) overloads are no-ops. This prevents AbstractMethodError for existing third-party Log implementors.

[ascheman]

withMetadata runs a StackWalker.walk(…) on every enabled Log-API call to recover the caller method name. The isXxxEnabled() guards mean disabled levels are free, but INFO-level logging in a loop now pays a stack-walk per call. Worth a quick benchmark; alternatively make the source-method capture lazy/opt-in, since most appenders won't render it.

Minor, same method: Thread.currentThread().getId() is deprecated since Java 19 (Thread.threadId()), and the JUL path already uses the modern getLongThreadID() — aligning them would be consistent.

[gnodet]

Fixed — StackWalker is now conditional: if (ProjectBuildLogAppender.hasReportCapture()) before walking. The ~1-5μs per-call cost is only paid when build report capture is actually active. During normal builds, withMetadata() just sets the ThreadLocal with the logger name and thread ID (no stack walking).

[ascheman]

Project id has a fork-aware restore via FORKING_PROJECT_ID (consulted in getProjectId()), but mojo id doesn't: mojoSucceeded/mojoFailed in LoggingExecutionListener call setMojoId(null), so when a forking mojo (e.g. a report goal that forks a lifecycle) resumes after its fork completes, maven.mojo.id has been cleared and is never restored — subsequent log lines from the forking mojo lose their MDC attribution. Should there be a symmetric FORKING_MOJO_ID (or a save/restore around the fork) mirroring the project-id handling?

Related: MOJO_ID is an InheritableThreadLocal; with reused pool threads a value not cleared on an exceptional path could inherit into a later, unrelated task.

[gnodet]

Fixed — added FORKING_MOJO_ID ThreadLocal mirroring the existing FORKING_PROJECT_ID pattern. LoggingExecutionListener.forkStarted() saves the current mojoId via setForkingMojoId(), and forkSucceeded()/forkFailed() clear it. Also fixed the cleanup ordering: setMojoId(null) now runs after delegate.mojoSucceeded/mojoFailed so the mojo context is available during the delegate callback, and setMojoId(null) restores the forking mojo's ID if one was saved.

[ascheman]

record.getLoggerName() can be null per the JUL spec; it's passed straight to LoggerFactory.getLogger(loggerName). Worth confirming that resolves to the root logger across SLF4J bindings rather than producing a literally "null"-named logger.

[gnodet]

Fixed — added a null guard: if (loggerName == null) { loggerName = ""; }. SLF4J's LoggerFactory.getLogger("") returns the root logger, which is safe.

[ascheman]

The new LogSink path hand-rolls throwable rendering (appendFormattedThrowable/appendStackTrace) while the non-sink path still goes through super.write(…) → writeThrowable. Two throwable renderings that can drift over time — could the sink path reuse the existing writeThrowable/printStackTrace (which still take Consumer<String>) to keep them single-sourced? Relatedly, MavenJulHandler.formatForConsole emits a bare "[LEVEL] message" that won't match this class's console layout, so JUL-origin lines look different from native lines on the sink path.

[gnodet]

Fixed — unified throwable rendering: the write() method now calls the existing writeThrowable() method instead of hand-rolling its own appendFormattedThrowable()/appendStackTrace(). Removed the duplicate methods entirely — single rendering path now.

[ascheman]

sequenceNumber() javadoc says "always non-negative," but the default returns -1 and DefaultLogEvent passes -1 when unknown. Fix the doc (or the sentinel) so they agree.

[gnodet]

Fixed (same as @gnodet's comment above) — javadoc now reads @return the sequence number, or {@code -1} if unavailable.

[gnodet]

Well-designed logging infrastructure foundation with clean three-path convergence (Log API, JUL, SLF4J). The bug fix for warn(Supplier, Throwable) calling logger.info() is confirmed correct.

Findings:

1. [medium] sequenceNumber() javadoc/contract mismatch — LogEvent.sequenceNumber() javadoc says @return the sequence number, always non-negative but the default implementation returns -1. The sibling method threadId() correctly documents or -1 if unavailable in its @return tag. The sequenceNumber() javadoc should follow the same pattern for consistency.

2. [medium] Inconsistent formattedMessage format between JUL and SLF4J — When a LogSink is installed, JUL events' formattedMessage is built by formatForConsole() which produces a minimal [LEVEL] message string, while SLF4J events produce a full formatted string with timestamps, thread names, and logger names via MavenBaseLogger.innerHandleNormalizedLoggingCall(). The practical impact is limited since the clean message() field is available for consumers who need consistent content, but in SimpleBuildEventListener.projectLogMessage() which uses formattedMessage() for console output, JUL events will look noticeably different from SLF4J events.

3. [low] Log4j2/Logback backend removal — The removal of Log4j2Configuration, LogbackConfiguration, and the logback-classic dependency means Maven no longer supports these as alternative SLF4J backends. This is intentional for the Maven 4.x logging redesign, but warrants mention in release notes for users who embedded Maven with a custom logging backend.

This review was generated by an AI agent (Claude Code) and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[gnodet]

Well-architected logging foundation PR. Clean design with proper ThreadLocal management, volatile concurrency handling, and good layering (API → impl → collector). A few items to address:

High severity:

1. API contract contradiction (LogEvent.java line 188): sequenceNumber() Javadoc says "@return the sequence number, always non-negative" but the default implementation returns -1. Compare with threadId() which correctly documents "or -1 if unavailable". This is a public API interface marked @Experimental/@since 4.1.0 — the Javadoc should match the actual contract.

2. No test coverage: 1000+ lines of foundational code across 22 files with zero test files. LogEvent/DefaultLogEvent, MavenJulHandler (249 lines), DefaultLog.withMetadata/trace/child, LogSink interface, ProjectBuildLogAppender structured event creation, and the mojo MDC lifecycle are all untested. The PR description mentions "580 tests pass" but these are all pre-existing tests.

Medium severity:

3. StackWalker overhead (DefaultLog.java line 649): withMetadata() calls StackWalker.walk() on every log call for enabled levels. While trace/debug are typically disabled and info/warn/error are low-volume, plugins logging many INFO/WARN messages will pay the 1-5μs per-call cost.

4. Logger name change (DefaultBuildPluginManager.java line 128): Logger name changed from getFullGoalName() (e.g., "compiler:compile") to getImplementation() (e.g., "org.apache.maven.plugins.compiler.CompilerMojo"). Intentional for proper hierarchical SLF4J configuration, but a user-visible behavior change that could break existing SLF4J level configurations.

5. Dead code for future PR (ProjectBuildLogAppender.java line 130): reportCapture volatile field and setter are infrastructure for PR #12695 (build report). Currently unused in this PR — consider adding a brief comment noting the intent.

Low severity:

6. setMojoId(null) is called before delegate.mojoSucceeded/mojoFailed callbacks, inconsistent with the forkSucceeded/forkFailed pattern where cleanup happens after the delegate.

7. The bug fix changing logger.info() to logger.warn() in warn(Supplier<String>, Throwable) is correct and important. 👍

The removal of Logback/Log4j2 support is a significant architectural decision — worth explicit mention in release notes since users plugging in alternative SLF4J backends will lose that ability.

---

This review was generated by an AI agent (Claude Code) and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[gnodet]

Review feedback addressed

All 8 review comments from @gnodet and @ascheman have been addressed in the latest force-push. Summary of changes:

Bug fixes

• sequenceNumber() javadoc — corrected from "always non-negative" to "or -1 if unavailable"
• warn(Supplier, Throwable) regression — was calling logger.info() instead of logger.warn() (added regression test)

Design improvements

• MavenJulHandler simplified — removed formatForConsole() entirely; all JUL events now always route through SLF4J so MavenSimpleLogger produces a consistent formattedMessage regardless of origin
• Log.trace() backward compatibility — all 6 trace methods changed from abstract to default implementations (no-ops) to prevent AbstractMethodError for existing third-party Log implementors
• StackWalker gated — withMetadata() only walks the stack when ProjectBuildLogAppender.hasReportCapture() is true; normal builds pay zero StackWalker cost
• Fork-aware mojoId — added FORKING_MOJO_ID ThreadLocal mirroring FORKING_PROJECT_ID; forkStarted()/forkSucceeded()/forkFailed() save and restore mojo context
• Throwable rendering unified — MavenSimpleLogger.write() now reuses the existing writeThrowable() method; removed the duplicate appendFormattedThrowable()/appendStackTrace() methods
• Null loggerName guard — MavenJulHandler.publish() now handles null logger names per JUL spec

Tests added

• DefaultLogTest (4 tests): warn/supplier regression, metadata lifecycle, trace delegation, trace no-op
• MavenJulHandlerTest (10 tests): parameterized JUL→SLF4J level mapping, FINEST→TRACE, CONFIG→INFO, metadata null check

All 6 downstream PRs (#12695, #12697, #12698, #12699, #12702, #12714) have been rebased onto the updated commit.