Uploaded image for project: 'XWiki Commons'
  1. XWiki Commons
  2. XCOMMONS-3738

Pass the exception to warn() and decide the stack trace display at rendering time

    XMLWordPrintable

Details

    • Improvement
    • Resolution: Unresolved
    • Major
    • None
    • 17.10.0
    • Logging
    • None
    • Unknown

    Description

      Today the logging best practices tell developers not to pass the exception to warn(), and to append the root cause message to the log message instead. The reason is that a warning should not print a full stack trace in the console by default. See https://dev.xwiki.org/xwiki/bin/view/Community/CodeStyle/JavaCodeStyle/#HLoggingBestPractices

      The cost is that the throwable is destroyed at the call site. It never reaches ILoggingEvent#getThrowableProxy(), LogEvent#getThrowable(), or the error.stack_trace field of an ECS/OTel/Datadog encoder, so nobody downstream can get it back - not the job log UI, not a log collector, not someone reading an existing log after the fact.

      Proposal

      Always pass the exception, including on warn(), and move the decision of whether to display a stack trace to the rendering side:

      • a Logback ThrowableHandlingConverter (%xwikiEx) that renders either the full trace or only the root cause, depending on an xwiki.logging.warn.stacktrace property, with an optional per-logger-prefix list so that for example contrib extensions can keep full traces
      • an xwiki.stacktrace marker (Logger.STACKTRACE_MARKER) for the call sites whose whole payload is the stack trace, such as XWikiDocument#warnWithStackTrace, so that they can force the trace to be displayed

      A prototype exists in https://github.com/xwiki/xwiki-commons/pull/1877 - it adds public API (Logger.STACKTRACE_MARKER), so it needs an @since and a release note entry.

      Why this is blocked

      The concern raised on the forum thread is that systematically attaching a throwable to every warning inflates the job log, which today has no pagination and no size limit, and already OOMs on large logs (XWIKI-16647). The agreed way forward is that the job log has to be able to cope with it first:

      • a paginated job log display, so that the viewer cost stops being proportional to the size of the whole log (XWIKI-24668)
      • a job log storage that can filter, order and paginate efficiently (XCOMMONS-3249)

      Once those are in place, the warn() call sites can be swept. Note that XWIKI-24665 already restored the throwable at 106 warn() sites in platform, so platform master currently contradicts the documented rule. This issue is what makes the documentation and the code agree again, in whichever direction is decided.

      Forum thread: https://forum.xwiki.org/t/logging-warning-stacktraces-in-debug-mode/18728

      Attachments

        Issue Links

          Activity

            People

              Unassigned Unassigned
              vmassol Vincent Massol
              Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

                Created:
                Updated: