Log API enhancements and mojo MDC

[gnodet]

Summary

Four Log-related improvements for 4.0.x:

• Log.trace() — new trace level (SLF4J TRACE / JUL FINEST) to separate Maven core internals from user-facing debug messages. Currently -X floods debug output with resolver/interpolation details that drown user-relevant diagnostics. All 6 trace methods have default implementations (no-ops) to prevent AbstractMethodError for existing third-party Log implementors.
• Log.child(name) — creates a sub-logger with an independently filterable name (e.g. CompilerMojo.diagnostics), letting plugin sub-components log under their own namespace.
• Logger name alignment — Maven 4 Log now uses the mojo implementation class name (e.g. org.apache.maven.plugins.compiler.CompilerMojo) instead of the goal name (compiler:compile). This matches what Maven 3 mojos already use and enables standard SLF4J hierarchical level configuration.
• Mojo MDC propagation — sets maven.mojo.id (prefix:goal@executionId) in the SLF4J MDC during mojo execution. All log messages — including those arriving through the JUL-to-SLF4J bridge — now carry mojo context, available to any SLF4J appender via %X{maven.mojo.id}. Fork-aware: saves and restores the forking mojo's ID across forked lifecycles.

Also fixes a pre-existing bug where DefaultLog.warn(Supplier, Throwable) delegated to logger.info() instead of logger.warn(), and adds isXxxEnabled() guards to the existing debug(Throwable), info(Throwable), warn(Throwable), and error(Throwable) methods.

Files changed

File	Change
Log.java	Add trace() default methods + child() default method
DefaultLog.java	Implement trace + child + fix warn bug + isXxxEnabled guards
DefaultMavenPluginManager.java	getFullGoalName() → getImplementation()
DefaultBuildPluginManager.java	getFullGoalName() → getImplementation()
ProjectBuildLogAppender.java	Add maven.mojo.id MDC key + fork-aware setMojoId() / setForkingMojoId()
LoggingExecutionListener.java	Set/clear mojoId MDC on mojo start/end, save/restore across forks

Related

• Logging foundation: structured LogEvent, JUL handler, Log API enhancements #12694 (logging foundation on master — superset including structured LogEvent, JUL handler, LogSink)

Test plan

• mvn test -pl impl/maven-core — 562 tests pass
• CI on maven-4.0.x branch

🤖 Generated with Claude Code

[gnodet]

Clean, well-scoped backport of Log API enhancements from #12572 to 4.0.x. All functional changes are correct, the bug fix (warn(Supplier, Throwable) delegating to logger.info() instead of logger.warn()) is verified, and the backport includes additional consistency improvements.

Notable observations:

• The backport correctly omits the @since tags that the original PR uses (4.1.0 for trace methods). Since the Log interface is @since 4.0.0 and the 4.0.x branch has no per-method @since tags, this is the right call.
• The backport improves on the original by adding isXxxEnabled() guards to the existing debug(Throwable), info(Throwable), warn(Throwable), and error(Throwable) methods, which previously called the logger unconditionally. This is a welcome consistency fix not in the original PR.
• The Log interface is @Provider and @Experimental, so adding new abstract methods (trace methods) is safe — external implementations are not expected.

Minor items:

• The original PR #12572 adds DefaultLogTest.java with a test for child(). This test was not included in the backport — consider adding it for parity.
• mojoSkipped() does not clear the mojo MDC. If a previous mojo set the MDC and a subsequent mojo is skipped, log messages during mojoSkipped handling would carry the stale mojo ID. This matches the original PR behavior and is a very minor edge case.

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

Claude Code on behalf of Guillaume Nodet

[gnodet]

Applied review fixes from #12694 to align the backport:

1. Log.java — default trace methods (prevents AbstractMethodError)
All 6 trace methods (isTraceEnabled + 5 overloads) now have default implementations — isTraceEnabled() returns false by default, all trace(...) overloads are no-ops. This prevents AbstractMethodError for existing third-party Log implementors (even though @Experimental+@Provider, this is the safe thing to do).

2. ProjectBuildLogAppender — fork-aware mojoId
Added FORKING_MOJO_ID ThreadLocal mirroring the existing FORKING_PROJECT_ID pattern. When setMojoId(null) is called after a fork completes, the forking mojo's ID is restored instead of simply clearing. This fixes the case where a forking mojo (e.g. a report goal) resumes after its fork — log messages during the resumed execution now correctly carry the mojo ID.

3. LoggingExecutionListener — fork save/restore + cleanup ordering

• forkStarted() now saves the current mojoId via setForkingMojoId()
• forkSucceeded()/forkFailed() clear the forking mojoId
• mojoSucceeded()/mojoFailed() now run the delegate before clearing the MDC (matching Logging foundation: structured LogEvent, JUL handler, Log API enhancements #12694's ordering)

Build: compiles cleanly and passes all maven-core tests (562 tests, 2 pre-existing failures unrelated to logging).

[gnodet]

Delta review — checking the latest commit e3c872f5 ("Address review: default trace methods and fork-aware mojoId").

The latest commit correctly addresses two important concerns:

1. Default trace methods in Log.java — Essential for this backport. On 4.0.x, existing third-party Log implementations would get AbstractMethodError if these were abstract. The no-op defaults (isTraceEnabled returns false, all trace overloads are no-ops) are the correct backward-compatible choice.

2. Fork-aware mojoId (FORKING_MOJO_ID ThreadLocal + setForkingMojoId) — An improvement over the original PR #12572 on master, which does not have this pattern. It correctly mirrors the existing FORKING_PROJECT_ID save/restore pattern, ensuring the forking mojo's MDC is restored after forked mojos complete.

Previous findings status:

Finding	Status	Notes
Missing DefaultLogTest.java	Still open (minor)	Test-parity gap only — functionality works correctly
mojoSkipped() doesn't clear MDC	Still open (non-issue)	mojoSkipped is called for mojos that were never started, so MDC is already cleared by the preceding mojo's mojoSucceeded/mojoFailed

Both remaining items are very minor and don't block the PR.

Additional observation: The ordering change in mojoSucceeded/mojoFailed (delegate runs before MDC clear) is a deliberate improvement — the delegate can still access the mojoId during its callback.

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

Claude Code on behalf of Guillaume Nodet

[gnodet]

Delta review — checking commit 56f4be34 ("Address review: add DefaultLogTest and clear MDC on mojoSkipped").

Both remaining items from the previous review are now resolved:

Finding	Status
Missing DefaultLogTest.java	✅ Resolved — added with 5 tests: childCreatesSubLogger, warnWithSupplierAndThrowableDelegatesToWarn, traceMethodsDelegateToSlf4jTrace, traceIsNoOpWhenDisabled, defaultTraceIsDisabled
mojoSkipped() doesn't clear MDC	✅ Resolved — now calls ProjectBuildLogAppender.setMojoId(null) after delegate, consistent with mojoSucceeded/mojoFailed

The clearing pattern (clear after delegate call) is consistent across all three mojo lifecycle callbacks. The test suite also covers the warn(Supplier, Throwable) bug fix and trace backward compatibility.

All review findings across 4 rounds are now addressed. 👍

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

Claude Code on behalf of Guillaume Nodet