JCU/fix(logging): add warn.log appender (A4) and split 404s to a dedicated logger - #1420
Merged
Merged
Conversation
…cated logger customer/jcu was branched from vanilla dspace-9.3 and never received the logging config the other instances carry, so no warn.log is produced. Add the A4 appender to log4j2.xml and to log4j2-container.xml - the container reads the latter via LOGGING_CONFIG - referenced from both the org.dspace logger and Root, since org.dspace is additivity='false'. Also port notFoundLog / logClientError() from dtq-dev-9-base, so 404 responses go to their own logger held at OFF instead of filling the new file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR aligns customer/jcu logging with other customer instances by adding a dedicated warn.log RollingFile appender (A4) and ensuring REST 404 responses can be suppressed via a dedicated logger, keeping warn.log actionable.
Changes:
- Add
A4RollingFile appender (${log.dir}/warn.log) with daily gzip rollover to both Log4j2 configs. - Route WARN+ from
org.dspaceandRoottoA4, while keepingorg.dspaceadditivity behavior intact. - Introduce a dedicated 404 logger in
DSpaceApiExceptionControllerAdviceand route 404 WARN logging through it (disabled by default in Log4j2 config).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
dspace/config/log4j2.xml |
Adds warn.log appender and routes WARN+ to it; defines an OFF-by-default dedicated 404 logger. |
dspace/config/log4j2-container.xml |
Mirrors warn.log appender/routing in the production-relevant container config; documents container config usage more explicitly. |
dspace-server-webapp/.../DSpaceApiExceptionControllerAdvice.java |
Adds dedicated 404 logger and logClientError() helper to suppress 404 WARN noise independently. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
References
customer/jcu, plus the 404 split from Reduce warn logs noise #1268 / MENDELU/Reduce warn logs noise - 404 dedicated logger #1278.Description
customer/jcuproduces nowarn.log. It was branched from vanilladspace-9.3and neverreceived the logging config the other instances carry. This adds the A4 appender to both config
files and ports the 404 logger split, so WARN and ERROR land in their own file the way they do
everywhere else.
Instructions for Reviewers
List of changes in this PR:
dspace/config/log4j2.xml: newA4RollingFile appender writing${log.dir}/warn.log,archives gzipped daily like
A1.<AppenderRef ref='A4' level='WARN'/>on theorg.dspacelogger and on
<Root>, plus the…NotFoundlogger atlevel='OFF'.dspace/config/log4j2-container.xml: the same appender and references. This is the file thatmatters in production — the backend container is started with
LOGGING_CONFIGpointing at it(
docker/docker-compose-rest.ymlin dspace-angular), so a change tolog4j2.xmlalone woulddo nothing. Also clarifies the header comment, which named
docker-compose-rest.ymlwithoutsaying which repo it lives in.
DSpaceApiExceptionControllerAdvice.java: thenotFoundLogfield and alogClientError()helper, taken from
dtq-dev-9-base. Without it every 404 lands in the newwarn.log, which isthe noise Reduce warn logs noise #1268 removed elsewhere. The unrelated CLARIN "expose the exception message to REST
clients for 4xx" change that also sits in 9-base's copy of this file is not part of this PR.
The
A4reference has to be in both places:org.dspaceisadditivity='false', so its eventsnever reach
<Root>.customer/mendeluhas A4 only under<Root>and itswarn.logthereforeholds third-party warnings only.
Not included, on purpose:
A2/checker.log— the appender on dtq-dev has no logger pointing at it and the file staysat 0 bytes. Checker output already comes from
log4j2-cli.xml, which this branch has.A3/file_downloads.log— it feeds offClarinMatomoBitstreamTracker, which does not existon the 9.x line. It would only create another empty file.
%tin the A1 pattern — it changes thedspace.logline format, so it belongs in its own PR.How to test:
[dspace]/log/warn.logis created at startup.401 and is logged by this advice:
curl -s -o /dev/null -w '%{http_code}' "$DSPACE_URL/api/eperson/epersons"-> 401Authentication is required (status:401 …)appears indspace.logand inwarn.log.grep -cE ' (WARN|ERROR) ' [dspace]/log/dspace.logvs the same overwarn.log.Checked against log4j-core 2.25.4, the version this branch pins:
dspace.logis unchanged insize,
warn.logcomes out byte-identical to dtq-dev's for the same events, and there is nolog4j2 status-logger output at startup.
Note for whoever deploys this:
LOGGING_CONFIGand the/dspace/configmount can be overriddenby the compose overlays under
/opt/dspace-envs/<instance>/, which are in no repo. Worth adocker inspecton the running backend container to confirm it reads the image's config.Checklist
mainbranch of code — no, this targetscustomer/jcu; it is a customer-branch fix.src/test/resources/log4j2-test.xml, not these files, so they neither cover nor are affected by this change.