This is an automated email from the ASF dual-hosted git repository.
hansva pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new 70bd50a379 Enhance logging to include stack traces and associated
throwables in log messages (#7682)
70bd50a379 is described below
commit 70bd50a37999cea311ad102fd71ac16f80ea00a8
Author: kxs-mtalbi <[email protected]>
AuthorDate: Thu Jul 30 04:44:41 2026 -0400
Enhance logging to include stack traces and associated throwables in log
messages (#7682)
---
.../hop/core/logging/FixedWidthLogLayout.java | 11 ++++-
.../org/apache/hop/core/logging/HopLogLayout.java | 11 ++++-
.../org/apache/hop/core/logging/ILogMessage.java | 10 ++++
.../org/apache/hop/core/logging/LogChannel.java | 24 +++++++---
.../org/apache/hop/core/logging/LogMessage.java | 35 ++++++++++++++
.../core/logging/Slf4jLoggingEventListener.java | 19 +++++++-
.../hop/core/logging/FixedWidthLogLayoutTest.java | 53 ++++++++++++++++++++++
.../apache/hop/core/logging/HopLogLayoutTest.java | 29 ++++++++++++
.../apache/hop/core/logging/LogChannelTest.java | 27 +++++++++++
.../apache/hop/core/logging/LogMessageTest.java | 39 ++++++++++++++++
.../java/org/apache/hop/git/GitPerspective.java | 34 ++++++++------
11 files changed, 266 insertions(+), 26 deletions(-)
diff --git
a/core/src/main/java/org/apache/hop/core/logging/FixedWidthLogLayout.java
b/core/src/main/java/org/apache/hop/core/logging/FixedWidthLogLayout.java
index 4d76d74d6f..b7a34f55a6 100644
--- a/core/src/main/java/org/apache/hop/core/logging/FixedWidthLogLayout.java
+++ b/core/src/main/java/org/apache/hop/core/logging/FixedWidthLogLayout.java
@@ -54,8 +54,15 @@ public class FixedWidthLogLayout {
String logLevel = getLogLevelPadded(message.getLevel());
- String[] parts =
- message.getMessage() == null ? new String[] {} :
message.getMessage().split(Const.CR);
+ // Append the message's stack trace (pre-rendered, or rendered on demand
from an attached
+ // throwable) so the GUI log browser still shows it.
+ String text = message.getMessage();
+ String stackTrace = message.getStackTrace();
+ if (stackTrace != null && !stackTrace.isEmpty()) {
+ text = (text == null || text.isEmpty()) ? stackTrace : text + Const.CR
+ stackTrace;
+ }
+
+ String[] parts = text == null ? new String[] {} : text.split(Const.CR);
for (int i = 0; i < parts.length; i++) {
if (!message.isSimplified()) {
diff --git a/core/src/main/java/org/apache/hop/core/logging/HopLogLayout.java
b/core/src/main/java/org/apache/hop/core/logging/HopLogLayout.java
index 8112870ac7..87c03ade41 100644
--- a/core/src/main/java/org/apache/hop/core/logging/HopLogLayout.java
+++ b/core/src/main/java/org/apache/hop/core/logging/HopLogLayout.java
@@ -57,8 +57,15 @@ public class HopLogLayout {
Object object = event.getMessage();
if (object instanceof LogMessage message) {
- String[] parts =
- message.getMessage() == null ? new String[] {} :
message.getMessage().split(Const.CR);
+ // Append the message's stack trace (pre-rendered, or rendered on demand
from an attached
+ // throwable) so console/file output still shows it.
+ String text = message.getMessage();
+ String stackTrace = message.getStackTrace();
+ if (stackTrace != null && !stackTrace.isEmpty()) {
+ text = (text == null || text.isEmpty()) ? stackTrace : text + Const.CR
+ stackTrace;
+ }
+
+ String[] parts = text == null ? new String[] {} : text.split(Const.CR);
for (int i = 0; i < parts.length; i++) {
// Start every line of the output with a dateTimeString
if (!message.isSimplified()) {
diff --git a/core/src/main/java/org/apache/hop/core/logging/ILogMessage.java
b/core/src/main/java/org/apache/hop/core/logging/ILogMessage.java
index 177de9c2d1..756261bf5f 100644
--- a/core/src/main/java/org/apache/hop/core/logging/ILogMessage.java
+++ b/core/src/main/java/org/apache/hop/core/logging/ILogMessage.java
@@ -31,4 +31,14 @@ public interface ILogMessage {
Object[] getArguments();
boolean isSimplified();
+
+ /**
+ * The throwable associated with this message, if any, so structured
listeners can render
+ * dedicated error fields instead of a pre-stringified stack-trace event.
+ *
+ * @return the associated throwable, or {@code null} when none.
+ */
+ default Throwable getThrowable() {
+ return null;
+ }
}
diff --git a/core/src/main/java/org/apache/hop/core/logging/LogChannel.java
b/core/src/main/java/org/apache/hop/core/logging/LogChannel.java
index af7c13c394..34f876e577 100644
--- a/core/src/main/java/org/apache/hop/core/logging/LogChannel.java
+++ b/core/src/main/java/org/apache/hop/core/logging/LogChannel.java
@@ -144,12 +144,24 @@ public class LogChannel implements ILogChannel {
}
public void println(ILogMessage message, Throwable e, LogLevel
channelLogLevel) {
- println(message, channelLogLevel);
-
- String stackTrace = Const.getStackTracker(e);
- LogMessage traceMessage =
- new LogMessage(stackTrace, message.getLogChannelId(), LogLevel.ERROR,
simplified);
- println(traceMessage, channelLogLevel);
+ if (message instanceof LogMessage logMessage) {
+ if (e != null) {
+ // Pre-render the trace (bounded memory), attach the throwable for
SLF4J, and bump to ERROR.
+ logMessage.setStackTrace(Const.getStackTracker(e));
+ logMessage.setThrowable(e);
+ logMessage.setLevel(LogLevel.ERROR);
+ }
+ println(message, channelLogLevel);
+ // Release the throwable so the buffered event doesn't retain the
exception graph.
+ logMessage.setThrowable(null);
+ } else {
+ // Non-standard ILogMessage can't carry a throwable: fall back to the
legacy two-event path.
+ println(message, channelLogLevel);
+ String stackTrace = Const.getStackTracker(e);
+ LogMessage traceMessage =
+ new LogMessage(stackTrace, message.getLogChannelId(),
LogLevel.ERROR, simplified);
+ println(traceMessage, channelLogLevel);
+ }
}
public void logWithLevel(String s, LogLevel logMessageLevel) {
diff --git a/core/src/main/java/org/apache/hop/core/logging/LogMessage.java
b/core/src/main/java/org/apache/hop/core/logging/LogMessage.java
index 8c0a38976f..e4852d45e7 100644
--- a/core/src/main/java/org/apache/hop/core/logging/LogMessage.java
+++ b/core/src/main/java/org/apache/hop/core/logging/LogMessage.java
@@ -33,6 +33,8 @@ public class LogMessage implements ILogMessage {
private LogLevel level;
private String copy;
private boolean simplified;
+ private Throwable throwable;
+ private String stackTrace;
/** Backward compatibility : no registry used, just log the subject as part
of the message */
public LogMessage(String subject, LogLevel level) {
@@ -215,4 +217,37 @@ public class LogMessage implements ILogMessage {
public void setSimplified(boolean simplified) {
this.simplified = simplified;
}
+
+ @Override
+ public Throwable getThrowable() {
+ return throwable;
+ }
+
+ public void setThrowable(Throwable throwable) {
+ this.throwable = throwable;
+ }
+
+ /**
+ * Returns the pre-rendered trace if set, else renders it from the
throwable, else {@code null}.
+ */
+ public String getStackTrace() {
+ if (stackTrace != null) {
+ return stackTrace;
+ }
+ if (throwable != null) {
+ return Const.getStackTracker(throwable);
+ }
+ return null;
+ }
+
+ public void setStackTrace(String stackTrace) {
+ this.stackTrace = stackTrace;
+ }
+
+ /**
+ * @param level the log level to set
+ */
+ public void setLevel(LogLevel level) {
+ this.level = level;
+ }
}
diff --git
a/core/src/main/java/org/apache/hop/core/logging/Slf4jLoggingEventListener.java
b/core/src/main/java/org/apache/hop/core/logging/Slf4jLoggingEventListener.java
index ee0f75bbca..3fe8340eec 100644
---
a/core/src/main/java/org/apache/hop/core/logging/Slf4jLoggingEventListener.java
+++
b/core/src/main/java/org/apache/hop/core/logging/Slf4jLoggingEventListener.java
@@ -60,7 +60,10 @@ public class Slf4jLoggingEventListener implements
IHopLoggingEventListener {
if (loggingObject == null) {
// this can happen if logObject has been discarded while log events
are still in flight.
logToLogger(
- hopLogger, message.getLevel(), message.getSubject() + " " +
message.getMessage());
+ hopLogger,
+ message.getLevel(),
+ message.getSubject() + " " + message.getMessage(),
+ message.getThrowable());
} else if (loggingObject.getObjectType() == PIPELINE
|| loggingObject.getObjectType() == TRANSFORM
|| loggingObject.getObjectType() == DATABASE) {
@@ -77,7 +80,19 @@ public class Slf4jLoggingEventListener implements
IHopLoggingEventListener {
private void logToLogger(
Logger logger, LogLevel logLevel, ILoggingObject loggingObject,
LogMessage message) {
logToLogger(
- logger, logLevel, "[" + getDetailedSubject(loggingObject) + "] " +
message.getMessage());
+ logger,
+ logLevel,
+ "[" + getDetailedSubject(loggingObject) + "] " + message.getMessage(),
+ message.getThrowable());
+ }
+
+ private void logToLogger(Logger logger, LogLevel logLevel, String message,
Throwable throwable) {
+ if (throwable == null) {
+ logToLogger(logger, logLevel, message);
+ return;
+ }
+
+ logger.error(message, throwable);
}
private void logToLogger(Logger logger, LogLevel logLevel, String message) {
diff --git
a/core/src/test/java/org/apache/hop/core/logging/FixedWidthLogLayoutTest.java
b/core/src/test/java/org/apache/hop/core/logging/FixedWidthLogLayoutTest.java
new file mode 100644
index 0000000000..850a626b5a
--- /dev/null
+++
b/core/src/test/java/org/apache/hop/core/logging/FixedWidthLogLayoutTest.java
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hop.core.logging;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.junit.jupiter.api.Test;
+
+/** Test for {@link FixedWidthLogLayout}. */
+class FixedWidthLogLayoutTest {
+
+ @Test
+ void testFormatRendersAttachedThrowable() {
+ LogMessage message = new LogMessage("Boom", "Channel 01", LogLevel.ERROR);
+ message.setThrowable(new IllegalStateException("kaboom"));
+
+ HopLoggingEvent event = new HopLoggingEvent(message, 0, LogLevel.ERROR);
+ final String formatted = new FixedWidthLogLayout().format(event);
+
+ assertTrue(formatted.contains("Boom"), "The message text must be
present.");
+ assertTrue(
+ formatted.contains("IllegalStateException"),
+ "The attached throwable's stack trace must be rendered for the GUI log
browser.");
+ assertTrue(formatted.contains("kaboom"), "The throwable message must be
rendered.");
+ }
+
+ @Test
+ void testFormatWithoutThrowableIsUnchanged() {
+ LogMessage message = new LogMessage("No error here", "Channel 01",
LogLevel.BASIC);
+
+ HopLoggingEvent event = new HopLoggingEvent(message, 0, LogLevel.BASIC);
+ final String formatted = new FixedWidthLogLayout().format(event);
+
+ assertFalse(
+ formatted.contains("\tat "),
+ "Messages without a throwable must not contain any stack-trace
frames.");
+ }
+}
diff --git
a/core/src/test/java/org/apache/hop/core/logging/HopLogLayoutTest.java
b/core/src/test/java/org/apache/hop/core/logging/HopLogLayoutTest.java
index ff7fdf0397..d1fb51708c 100644
--- a/core/src/test/java/org/apache/hop/core/logging/HopLogLayoutTest.java
+++ b/core/src/test/java/org/apache/hop/core/logging/HopLogLayoutTest.java
@@ -17,6 +17,8 @@
package org.apache.hop.core.logging;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import org.junit.jupiter.api.Test;
@@ -38,4 +40,31 @@ class HopLogLayoutTest {
formattedMsg.substring(formattedMsg.indexOf('-') + 2),
"The log message must be formatted and not contain placeholders.");
}
+
+ @Test
+ void testFormatRendersAttachedThrowable() {
+ LogMessage message = new LogMessage("Boom", "Channel 01", LogLevel.ERROR);
+ message.setThrowable(new IllegalStateException("kaboom"));
+
+ HopLoggingEvent event = new HopLoggingEvent(message, 0, LogLevel.ERROR);
+ final String formatted = new HopLogLayout().format(event);
+
+ assertTrue(formatted.contains("Boom"), "The message text must be
present.");
+ assertTrue(
+ formatted.contains("IllegalStateException"),
+ "The attached throwable's stack trace must be rendered for
console/file output.");
+ assertTrue(formatted.contains("kaboom"), "The throwable message must be
rendered.");
+ }
+
+ @Test
+ void testFormatWithoutThrowableIsUnchanged() {
+ LogMessage message = new LogMessage("No error here", "Channel 01",
LogLevel.BASIC);
+
+ HopLoggingEvent event = new HopLoggingEvent(message, 0, LogLevel.BASIC);
+ final String formatted = new HopLogLayout().format(event);
+
+ assertFalse(
+ formatted.contains("\tat "),
+ "Messages without a throwable must not contain any stack-trace
frames.");
+ }
}
diff --git a/core/src/test/java/org/apache/hop/core/logging/LogChannelTest.java
b/core/src/test/java/org/apache/hop/core/logging/LogChannelTest.java
index 49095a3dc3..c070010b25 100644
--- a/core/src/test/java/org/apache/hop/core/logging/LogChannelTest.java
+++ b/core/src/test/java/org/apache/hop/core/logging/LogChannelTest.java
@@ -17,6 +17,10 @@
package org.apache.hop.core.logging;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock;
@@ -110,4 +114,27 @@ class LogChannelTest {
logChannel.println(logMsgInterfaceFil, LogLevel.BASIC);
verify(logChFileWriterBuffer,
times(0)).addEvent(any(HopLoggingEvent.class));
}
+
+ @Test
+ void testPrintlnWithThrowableBumpsLevelPreRendersTraceAndReleasesThrowable()
{
+ LoggingBuffer loggingBuffer = mock(LoggingBuffer.class);
+ mockedHopLogStore.when(HopLogStore::getAppender).thenReturn(loggingBuffer);
+ logChannel.setFilter("");
+
+ LogMessage message = new LogMessage("Boom", "1234-5678-abcd-efgh",
LogLevel.BASIC);
+ IllegalStateException failure = new IllegalStateException("kaboom");
+
+ logChannel.println(message, failure, LogLevel.ERROR);
+
+ // Surfaced at ERROR level so error scanners/counters still detect it.
+ assertEquals(LogLevel.ERROR, message.getLevel());
+ // Live throwable released so the buffered event does not retain the
exception object graph.
+ assertNull(message.getThrowable());
+ // Pre-rendered trace survives for text/buffer consumers (console, file,
GUI log browser).
+ assertNotNull(message.getStackTrace());
+ assertTrue(message.getStackTrace().contains("IllegalStateException"));
+ assertTrue(message.getStackTrace().contains("kaboom"));
+ // A single combined event is emitted instead of the legacy two-event path.
+ verify(loggingBuffer,
times(1)).addLogggingEvent(any(HopLoggingEvent.class));
+ }
}
diff --git a/core/src/test/java/org/apache/hop/core/logging/LogMessageTest.java
b/core/src/test/java/org/apache/hop/core/logging/LogMessageTest.java
index ccffda5f61..ead18dad52 100644
--- a/core/src/test/java/org/apache/hop/core/logging/LogMessageTest.java
+++ b/core/src/test/java/org/apache/hop/core/logging/LogMessageTest.java
@@ -18,6 +18,8 @@
package org.apache.hop.core.logging;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import org.junit.jupiter.api.Test;
@@ -48,4 +50,41 @@ class LogMessageTest {
new LogMessage("Hello {0}", "channel-1", new Object[] {"world"},
LogLevel.BASIC, false);
assertEquals("Hello world", msg.getMessage());
}
+
+ @Test
+ void getStackTrace_withoutThrowableOrRenderedTrace_returnsNull() {
+ LogMessage msg = new LogMessage("no error", "channel-1", LogLevel.BASIC);
+ assertNull(msg.getStackTrace());
+ }
+
+ @Test
+ void getStackTrace_rendersFromAttachedThrowable() {
+ LogMessage msg = new LogMessage("boom", "channel-1", LogLevel.ERROR);
+ msg.setThrowable(new IllegalStateException("kaboom"));
+
+ String stackTrace = msg.getStackTrace();
+ assertTrue(stackTrace.contains("IllegalStateException"));
+ assertTrue(stackTrace.contains("kaboom"));
+ }
+
+ @Test
+ void getStackTrace_preRenderedTraceSurvivesThrowableRelease() {
+ LogMessage msg = new LogMessage("boom", "channel-1", LogLevel.ERROR);
+ msg.setThrowable(new IllegalStateException("kaboom"));
+ // Persist the rendered form, then release the live throwable (as
LogChannel does after
+ // dispatch) — the buffer/layout consumers must still see the trace.
+
msg.setStackTrace(org.apache.hop.core.Const.getStackTracker(msg.getThrowable()));
+ msg.setThrowable(null);
+
+ assertNull(msg.getThrowable());
+ assertTrue(msg.getStackTrace().contains("IllegalStateException"));
+ assertTrue(msg.getStackTrace().contains("kaboom"));
+ }
+
+ @Test
+ void setLevel_overridesLevel() {
+ LogMessage msg = new LogMessage("boom", "channel-1", LogLevel.BASIC);
+ msg.setLevel(LogLevel.ERROR);
+ assertEquals(LogLevel.ERROR, msg.getLevel());
+ }
}
diff --git
a/plugins/misc/git/src/main/java/org/apache/hop/git/GitPerspective.java
b/plugins/misc/git/src/main/java/org/apache/hop/git/GitPerspective.java
index 68e79a38fe..c0deea7357 100644
--- a/plugins/misc/git/src/main/java/org/apache/hop/git/GitPerspective.java
+++ b/plugins/misc/git/src/main/java/org/apache/hop/git/GitPerspective.java
@@ -337,26 +337,26 @@ public class GitPerspective implements IHopPerspective {
PropsUi.setLook(wSearchText);
// Create a composite with toolbar and tree for the border
- // Composite composite = new Composite(treeComposite, SWT.BORDER);
- // composite.setLayout(new FormLayout());
- // composite.setLayoutData(FormDataBuilder.builder().top(wSearchText,
+ // Composite composite = new Composite(treeComposite, SWT.BORDER);
+ // composite.setLayout(new FormLayout());
+ // composite.setLayoutData(FormDataBuilder.builder().top(wSearchText,
// PropsUi.getMargin()).bottom().fullWidth().result());
- // PropsUi.setLook(composite);
+ // PropsUi.setLook(composite);
// Create toolbar
//
- // IToolbarContainer toolBarContainer =
- // ToolbarFacade.createToolbarContainer(composite, SWT.WRAP |
SWT.LEFT | SWT.HORIZONTAL);
+ // IToolbarContainer toolBarContainer =
+ // ToolbarFacade.createToolbarContainer(composite, SWT.WRAP | SWT.LEFT |
SWT.HORIZONTAL);
//
- // refToolBarWidgets = new GuiToolbarWidgets();
- // refToolBarWidgets.registerGuiPluginObject(this);
- // refToolBarWidgets.createToolbarWidgets(toolBarContainer,
+ // refToolBarWidgets = new GuiToolbarWidgets();
+ // refToolBarWidgets.registerGuiPluginObject(this);
+ // refToolBarWidgets.createToolbarWidgets(toolBarContainer,
// GUI_PLUGIN_REF_TOOLBAR_PARENT_ID);
//
- // wRefToolBar = toolBarContainer.getControl();
- //
wRefToolBar.setLayoutData(FormDataBuilder.builder().fullWidth().top().result());
- // wRefToolBar.pack();
- // PropsUi.setLook(wRefToolBar, Props.WIDGET_STYLE_TOOLBAR);
+ // wRefToolBar = toolBarContainer.getControl();
+ //
wRefToolBar.setLayoutData(FormDataBuilder.builder().fullWidth().top().result());
+ // wRefToolBar.pack();
+ // PropsUi.setLook(wRefToolBar, Props.WIDGET_STYLE_TOOLBAR);
wRefTree = new Tree(composite, SWT.BORDER | SWT.V_SCROLL | SWT.H_SCROLL);
wRefTree.setHeaderVisible(false);
@@ -620,7 +620,13 @@ public class GitPerspective implements IHopPerspective {
}
return isCommitInCurrentBranch(uiGit.getGit().getRepository(),
commit.getId());
} catch (MissingObjectException e) {
- // Object id not present in the currently open repository (e.g. after
project switch)
+ // Commit not in the local object store: expected (e.g. after a
fetch/refresh), so log at
+ // debug level and report it's not in the branch.
+ LogChannel.UI.logDebug(
+ "Commit "
+ + commit.getId().getName()
+ + " is not present in the local object store while checking the
current branch: "
+ + e.getMessage());
return false;
} catch (Exception e) {
LogChannel.UI.logError("Error checking if commit is in current branch",
e);
