gnodet commented on code in PR #12695:
URL: https://github.com/apache/maven/pull/12695#discussion_r4059675348
##########
impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLog.java:
##########
@@ -64,28 +63,22 @@ public DefaultLog(Logger logger) {
}
/**
- * Wraps a logging call with Log API metadata: sets the ThreadLocal with
- * source class name and thread ID, executes the actual SLF4J call, and
- * clears the ThreadLocal.
+ * Wraps a logging call with Log API metadata: captures the caller's
+ * method name via {@link StackWalker}, sets the ThreadLocal, executes
+ * the actual SLF4J call, and clears the ThreadLocal.
* <p>
* The source class name is taken from the SLF4J logger name (which
* is the mojo implementation FQCN, set at injection time). The
- * source method name is resolved via {@link StackWalker} only when
- * build report capture is active (to avoid the ~1-5μs per-call cost
- * on every enabled log statement during normal builds).
+ * source method name is resolved by walking the stack past this class
+ * to find the first external caller frame.
*/
private void withMetadata(Runnable logAction) {
- // Only pay the StackWalker cost when someone is actually capturing
metadata
- String callerMethodName = null;
- if (ProjectBuildLogAppender.hasReportCapture()) {
- callerMethodName = WALKER.walk(frames -> frames.dropWhile(f ->
THIS_CLASS.equals(f.getClassName()))
- .findFirst()
- .map(StackFrame::getMethodName)
- .orElse(null));
- }
- @SuppressWarnings("deprecation") // Thread.getId() — threadId()
requires Java 19+
- long threadId = Thread.currentThread().getId();
- LOG_API_METADATA.set(new LogApiMetadata(logger.getName(),
callerMethodName, threadId));
+ String callerMethodName = WALKER.walk(frames -> frames.dropWhile(f ->
THIS_CLASS.equals(f.getClassName()))
+ .findFirst()
+ .map(StackFrame::getMethodName)
+ .orElse(null));
+ LOG_API_METADATA.set(new LogApiMetadata(
+ logger.getName(), callerMethodName,
Thread.currentThread().getId()));
try {
logAction.run();
Review Comment:
Fixed in `f656ea9567` — `hasReportCapture()` guard restored in
`DefaultLog.withMetadata()`; StackWalker cost is now only paid when a build
report capture is active.
##########
api/maven-api-core/src/main/java/org/apache/maven/api/plugin/Log.java:
##########
@@ -38,65 +38,48 @@
public interface Log {
/**
* {@return true if the <b>trace</b> error level is enabled}
- * <p>
- * The default implementation returns {@code false} for backward
- * compatibility with existing {@code Log} implementations.
+ * @since 4.1.0
*/
- default boolean isTraceEnabled() {
- return false;
- }
+ boolean isTraceEnabled();
Review Comment:
Fixed in `f656ea9567` — all six trace methods in `Log.java` restored to
`default` with no-op implementations.
##########
api/maven-api-core/src/main/java/org/apache/maven/api/build/report/LogEvent.java:
##########
@@ -157,7 +158,8 @@ default long threadId() {
* providing a global ordering across all event sources (Log API,
* JUL, and direct SLF4J).
*
- * @return the sequence number, or {@code -1} if unavailable
+ * @return the sequence number, always non-negative
+ * @since 4.1.0
*/
default long sequenceNumber() {
return -1;
Review Comment:
Fixed in `f656ea9567` — Javadoc now says "or {@code -1} if unavailable",
consistent with the `default return -1`.
##########
impl/maven-core/src/main/java/org/apache/maven/logging/LoggingExecutionListener.java:
##########
@@ -129,45 +129,40 @@ public void mojoStarted(ExecutionEvent event) {
@Override
public void mojoSucceeded(ExecutionEvent event) {
setMdc(event);
- delegate.mojoSucceeded(event);
ProjectBuildLogAppender.setMojoId(null);
+ delegate.mojoSucceeded(event);
}
@Override
public void mojoFailed(ExecutionEvent event) {
setMdc(event);
- delegate.mojoFailed(event);
ProjectBuildLogAppender.setMojoId(null);
+ delegate.mojoFailed(event);
}
@Override
public void mojoSkipped(ExecutionEvent event) {
setMdc(event);
delegate.mojoSkipped(event);
Review Comment:
Fixed in `f656ea9567` — `mojoSkipped()` now calls
`ProjectBuildLogAppender.setMojoId(null)` after `delegate.mojoSkipped(event)`.
--
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]
