Skip to content

Forward-port: Log API enhancements and mojo MDC - #12929

Merged
gnodet merged 2 commits into
masterfrom
forward-port/log-api-enhancements
Aug 31, 2026
Merged

Forward-port: Log API enhancements and mojo MDC#12929
gnodet merged 2 commits into
masterfrom
forward-port/log-api-enhancements

Conversation

@gnodet

@gnodet gnodet commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Forward-port of #12690 (merged into maven-4.0.x) to master.

Brings the following Log API enhancements to master:

  • Log.trace() — new trace level (SLF4J TRACE) with default no-op implementations to prevent AbstractMethodError for existing third-party Log implementors
  • Log.child(name) — creates a sub-logger with an independently filterable hierarchical name
  • Logger name alignment — uses the mojo implementation class name instead of the goal name
  • Mojo MDC propagation — sets maven.mojo.id in SLF4J MDC during mojo execution, fork-aware (saves/restores across forked lifecycles)
  • Bug fixDefaultLog.warn(Supplier, Throwable) was delegating to logger.info() instead of logger.warn()
  • isXxxEnabled() guards — added to Throwable-only log overloads
  • DefaultLogTest — 5 new tests covering trace delegation, warn bug fix, child loggers, and backward compatibility

Once this merges, #12694 will be rebased on top to layer the master-only additions (structured LogEvent, JUL handler, LogSink, withMetadata/StackWalker).

Related

Test plan

  • Cherry-pick applied cleanly (no conflicts)
  • CI on master

🤖 Generated with Claude Code

* Backport Log API enhancements and mojo MDC to 4.0.x

Backport four Log-related improvements from master to the 4.0.x branch
for inclusion in rc-7:

1. Log.trace() — new trace level (maps to 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.

2. 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.

3. 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.

4. 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}.

Also fixes a pre-existing bug in DefaultLog where warn(Supplier, Throwable)
incorrectly delegated to logger.info() instead of logger.warn().

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Add isXxxEnabled() guards to Throwable-only log overloads

Align with master by wrapping the five xxx(Throwable) overloads
in level-enabled checks, avoiding unnecessary method calls and
empty string construction when the level is disabled.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Address review: default trace methods and fork-aware mojoId

Apply review fixes from #12694 to align the backport:

- Log.java: make all 6 trace methods default (no-ops) to prevent
  AbstractMethodError for existing third-party Log implementors.
  isTraceEnabled() returns false by default.

- ProjectBuildLogAppender: add FORKING_MOJO_ID ThreadLocal mirroring
  the existing FORKING_PROJECT_ID pattern. When setMojoId(null) is
  called, the forking mojo's ID is restored instead of clearing.

- LoggingExecutionListener: save current mojoId in forkStarted(),
  clear forking mojoId in forkSucceeded/forkFailed. Fix cleanup
  ordering in mojoSucceeded/mojoFailed — delegate runs first, then
  MDC is cleared.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* Address review: add DefaultLogTest and clear MDC on mojoSkipped

- Add DefaultLogTest with 5 tests: warn/supplier regression,
  trace delegation, trace no-op guard, child() sub-logger,
  and default trace methods (AbstractMethodError prevention).

- Clear mojo MDC in mojoSkipped() to prevent stale mojo context
  from leaking into subsequent log messages.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>

@gnodet gnodet left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean forward-port of #12690 from maven-4.0.x to master. The diff is byte-identical to the merged PR — no adaptation was needed. All findings from 4 prior review rounds on #12690 have been addressed.

Observations (informational):

  • Cherry-pick applied with zero conflicts; both diffs are 617 lines and byte-identical
  • Other Log implementations on master (MojoLogWrapper, compat DefaultLog, SystemStreamLog) implement the legacy org.apache.maven.plugin.logging.Log, not org.apache.maven.api.plugin.Log, so they are unaffected by the new trace/child default methods
  • The getFullGoalName()getImplementation() change is consistent with existing usage in DefaultMavenPluginManager.java which already uses getImplementation() for the mojo logger
  • PR description accurately documents the relationship with #12694 which will add master-only additions

📋 PR Metadata

Aspect Current Suggested
Labels (none) enhancement
Milestone (none) 4.1.0

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

}

/**
* Sends a message to the user in the <b>trace</b> error level.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usually written as "at the trace error level" rather than "in the trace error level"

* such as resolver negotiation, model interpolation, and lifecycle
* ordering details. Use {@link #debug(CharSequence)} instead for
* messages that help <em>users</em> investigate their build
* (e.g. why a module was recompiled).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e.g. --> For instance,

- "in the trace error level" → "at the trace error level"
- "e.g." → "for instance,"

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@gnodet gnodet added this to the 4.1.0 milestone Aug 30, 2026
@gnodet
gnodet merged commit 08c6ce2 into master Aug 31, 2026
42 of 43 checks passed
@gnodet
gnodet deleted the forward-port/log-api-enhancements branch August 31, 2026 08:50
@github-actions

Copy link
Copy Markdown

@gnodet Please assign appropriate label to PR according to the type of change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants