SAP / cf-java-logging-support

The Java Logging Support for Cloud Foundry supports the creation of structured log messages and the collection of request metrics
Apache License 2.0
77 stars 48 forks source link

Starting with version 3.6.0 exception causes are not logged #145

Closed sencko closed 2 years ago

sencko commented 2 years ago

With the latest version of com.sap.hcp.cf.logging:cf-java-logging-support-logback the exception cause is not logged anymore, which makes troubleshooting errors much harder. Here is some sample code to reproduce the problem:


import com.sap.hcp.cf.logback.encoder.JsonEncoder;

import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.Logger;
import ch.qos.logback.classic.LoggerContext;
import ch.qos.logback.classic.spi.LoggingEvent;

...
    LoggerContext context = new LoggerContext();
    Logger logger = context.getLogger(this.getClass());

    Exception cause = new Exception();
    LoggingEvent loggingEvent = new LoggingEvent(ch.qos.logback.classic.Logger.class.getName(), logger, Level.ERROR, "This is message {}",
        new Exception(cause), new Object[] { "hh" });

    JsonEncoder encoder = new JsonEncoder();
    encoder.setContext(context);
    encoder.start();
    byte[] event = encoder.encode(loggingEvent);

    System.out.println(new String(event));
KarstenSchnitter commented 2 years ago

Hi,

thank you for reporting the issue. I will look into it.

Best Regards, Karsten

KarstenSchnitter commented 2 years ago

I am wondering though, why this is not picked up by the unit test case https://github.com/SAP/cf-java-logging-support/blob/456492122afd09625649753e13699b4ad804be16/cf-java-logging-support-logback/src/test/java/com/sap/hcp/cf/logging/common/TestAppLog.java#L125-L139 I will look into your example and the test.

s10v commented 2 years ago

Hi @KarstenSchnitter,

Not only the causes, but also the exception type line is missing in the stacktrace.

Regards, Stefan

KarstenSchnitter commented 2 years ago

I will run a comparison between 3.5.x and 3.6.1 to see what got lost. The unit tests will also be expanded to cover that requirement.

KarstenSchnitter commented 2 years ago

I think the issue lies within the new implementation of the JsonEncoder for cf-java-logging-support-logback: https://github.com/SAP/cf-java-logging-support/blob/456492122afd09625649753e13699b4ad804be16/cf-java-logging-support-logback/src/main/java/com/sap/hcp/cf/logback/encoder/JsonEncoder.java#L334-L336

This code omits the message and the cause from the ThrowableProxy. The log4j2 implementation in cf-java-logging-support-log4j2 still uses the old approach with a PrintWriter: https://github.com/SAP/cf-java-logging-support/blob/456492122afd09625649753e13699b4ad804be16/cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/layout/JsonPatternLayout.java#L209

I will test a fix for cf-java-logging-support-logback, that adds the missing information on top of the stacktrace and also in between different stacktrace elements.

Currently, I would assume this issue to be cf-java-logging-support-logback only and not to occur when using the log4j2 support.

KarstenSchnitter commented 2 years ago

I provided a fix with #146. It brings back the implementation from before v3.6.0, which is also used in the log4j2. The test was changed to detect future regressions.

When logging exceptions, keep in mind, that the information contained in the exceptions themselves depends on the JVM. The old test would not contain a message in the NullPointerException raised when running on an OpenJDK-11, but it would contain a message on a SAPMachine-17.

KarstenSchnitter commented 2 years ago

Resolved in release v3.6.2.