[OPENDJ-6710] Use a single method for generating log error messages and stack traces if needed Created: 09/Oct/19  Updated: 28/Jul/20

Status: Dev in Progress
Project: OpenDJ
Component/s: core apis, core server, ease of use, logging, tech-debt
Affects Version/s: 7.0.0
Fix Version/s: 7.1.0

Type: Task Priority: Critical
Reporter: Matthew Swift Assignee: Nicolas Capponi
Resolution: Unresolved Votes: 0
Labels: None

Issue Links:
Relates
relates to OPENDJ-6505 ConcurrentModificationException in To... Done
relates to OPENDJ-6528 Randomly getting ConcurrentModificati... Dev backlog
Epic Link: Miscellaneous 7.1
Story Points: 3
Dev Assignee: Nicolas Capponi

 Description   

Motivation: OPENDJ-6528 and OPENDJ-6505.

We could do with rationalizing some of our exception logging. At the moment we have many methods that output different varieties of exception message:

  • com.forgerock.opendj.util.StaticUtils#getExceptionMessage - this is the method used for the CME in this bug. It does not include any stack trace when the main exception has a message
  • com.forgerock.opendj.util.StaticUtils#stackTraceToSingleLineString(java.lang.StringBuilder, java.lang.Throwable, boolean)
  • com.forgerock.opendj.util.StaticUtils#stackTraceToSingleLineString(java.lang.Throwable, boolean) - delegates to method above
  • org.opends.server.util.StaticUtils#getBacktrace
  • org.opends.server.util.StaticUtils#getExceptionMessage - does some stuff then delegates to SDK equivalent
  • org.opends.server.util.StaticUtils#stackTraceToSingleLineString - delegates to SDK
  • org.opends.server.util.StaticUtils#stackTraceToString(java.lang.Throwable) - no delegation

I wonder if we could replace all these with a single method that combines getExceptionMessage(), which handles recognized exception types, then fall through to stackTraceToSingleLineString() with full stack always set to true for unrecognized exceptions.

Additional input from Jean-Noël Rouvignac: I see a lot of code calling e.getMessage() or e.getLocalizedMessage() (Java exceptions) or e.getMessageObject() (OpenDJ exceptions). I would ban them all in favour of a single call to StaticUtils.getExceptionMessage(e) which does the right thing (TM).



 Comments   
Comment by Jean-Noël Rouvignac [ 02/Dec/19 ]

By luck (or lack thereof) I found out that StaticUtils.getExceptionMessage() prints the stacktrace for the first exception, but not for the exception causes, which would have been very useful in my case (The cause was a NullPointerException)

Most/all these methods have an interesting feature, but none have the whole picture.
It would be very nice for our unified method to be able to:

  • print the whole stack trace in the reduced form we have today (i.e. print simple class names + file and line numbers) for {{RuntimeException}}s
  • also print the causes with their stacktraces
  • do not duplicate the stacktraces (like the JDK does)
  • number the causes: InvocationTargetException ... (1) Caused by ExecutionException ... (2) Caused by RuntimeException ... (3) Caused by NullPointerException
Comment by Jean-Noël Rouvignac [ 02/Dec/19 ]

If we really want to show new lines, some logging frameworks use this technique:

Where you can see that ‘u2028’ (https://www.fileformat.info/info/unicode/char/2028/index.htm) is separating what used to be line feeds in the stack trace. Now the exception message and stack trace will be sent as a single unit.

Comment by Matthew Swift [ 03/Dec/19 ]

This would be a great issue to work on during the tech debt week. We've all (eng and services) suffered death by a million cuts due to DS's mixed and sometimes poor logging of stack traces.

Generated at Sun Aug 09 23:14:35 UTC 2020 using Jira 7.13.12#713012-sha1:6e07c38070d5191bbf7353952ed38f111754533a.