Details
-
Improvement
-
Resolution: Fixed
-
Major
-
18.6.0
-
None
-
Unknown
-
N/A
-
N/A
-
Description
The logging best practices of the Java Code Style were applied in a sweep across xwiki-commons, xwiki-rendering and xwiki-platform: roughly 785 changed log statements over 24 commits and 22 pull requests, initially committed under [Misc] with no issue.
Why this issue exists
Most of those changes only affect the text of a message, but a substantial subset changes what XWiki actually prints at runtime — in particular whether a stack trace appears in the XWiki log output and in the job logs. That is a visible production behaviour change: it needs to be traceable, reviewable, and referenceable from every commit, which [Misc] does not provide. This issue is the umbrella for the whole campaign and for its follow-ups.
The cases that change behaviour
- A stack trace is no longer displayed for a warning. warn(msg, e) became warn(msg + " Root cause is [{}]", ExceptionUtils.getRootCauseMessage(e)) — about 108 sites. The throwable no longer reaches the log event at all, so it is also lost from ILoggingEvent#getThrowableProxy(), from XWiki's own LogEvent.throwable, and from the error.stack_trace / exception.stacktrace fields produced by ECS, OpenTelemetry and Datadog encoders. Log-collection tools can no longer analyse those failures.
- A stack trace was inlined into the message text. Six sites passed ExceptionUtils.getStackTrace(e) into a {} placeholder. In the job-log displayer this falls through to plain escaping, so the entire trace is rendered inside the list item with no click-to-expand and no pre-wrap whitespace handling — effectively unreadable.
- A stack trace is now printed at error level where it was previously only at debug. An error(root cause only) plus debug(msg, e) pair was collapsed into a single error(msg, e) — 3 sites. For failures that repeat on every request this floods the logs, which is precisely why the pair existed.
- Errors that printed no stack trace now print one. A throwable was added as the trailing argument of an error() call — 34 sites.
- More output at debug level. A throwable was added to a debug() call — 11 sites.
- Log levels changed. Six error to warn in commons and one warn to info in platform, which changes what is visible at the default level.
- Job-log arguments can now be lost entirely. Explicit toString() calls on log arguments were removed — 21 sites. Job logs capture the raw argument array and XStream-serialize it; on read-back SafeArrayConverter#readBareItem() swallows failures and yields null, so an argument whose class can no longer be resolved (typically a class from an extension jar) disappears completely, where a String would have survived.
- Logging can die silently on a deprecated construction path. private static final Logger was replaced by an @Inject Logger in 15 files. When such a class is built through its deprecated public constructor instead of by the component manager, the field stays null and nothing is logged.
- Output moved from stdout and stderr to the log. 22 printStackTrace() calls were replaced by real log calls.
- Arguments are now always evaluated. 61 redundant isXxxEnabled() guards were removed. This is harmless for a parameterized call, but it also removes the guard around any real work that was being skipped.
- Log message text changed. About 550 statements had their message text or format reworked: String.format replaced by {} placeholders, placeholder arity fixed, the bracket convention applied, context arguments added. Any log grep, log parser or alerting rule matching the old text stops matching.
Follow-ups tracked under this issue
- Fix the individual mistakes raised during review: the inlined stack traces, the collapsed error and debug pairs, the @Inject Logger breakage on a deprecated constructor, and the messages that merely duplicate their own cause.
- Amend the logging rule so that it says what it actually wants: a warning must not display a stack trace by default, enforced by the appender or the displayer, rather than by destroying the throwable at the call site. See the forum proposal at https://forum.xwiki.org/t/logging-warning-stacktraces-in-debug-mode/18728
- Revert the warn sites above once that rule change is agreed.
- Throttle repeated error stack traces, so that keeping the trace on an error() does not flood the logs.
- Make job-log argument serialization positive-signal and component-based, instead of relying on XStreamUtils.isSerializable() defaulting to true.
- Add a build-level check of the types allowed as log arguments, which is the only structural prevention for the whole class of toString() mistakes.
Convention
From now on, every commit belonging to this logging work — in xwiki-commons, xwiki-rendering and xwiki-platform alike — must reference this issue rather than using [Misc].
Attachments
Issue Links
- is related to
-
XCOMMONS-3738 Pass the exception to warn() and decide the stack trace display at rendering time
-
- Open
-
-
XWIKI-24668 Move the job log display to a paginated Live Data
-
- Open
-