Skip to content

chore(audit-log): add audit log file transport configurable via appli…#6377

Open
corinnekrych wants to merge 1 commit into
mainfrom
audit-log-configurable-path
Open

chore(audit-log): add audit log file transport configurable via appli…#6377
corinnekrych wants to merge 1 commit into
mainfrom
audit-log-configurable-path

Conversation

@corinnekrych

Copy link
Copy Markdown
Contributor

…cation.properties

Proposed changes

Testing Instructions

  1. Step-by-step how to test
  2. Environment or config notes

Related issues

  • Closes #ISSUE-NUMBER

Checklist

  • I consider the submitted work as finished
  • I tested the code for its functionality
  • I wrote test cases for the relevant uses case
  • I added/update the relevant documentation (either on github or on notion)
  • Where necessary I refactored code to improve the overall quality
  • For bug fix -> I implemented a test that covers the bug

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

Copilot AI review requested due to automatic review settings June 24, 2026 16:33
@github-actions github-actions Bot added the filigran team Item from the Filigran team. label Jun 24, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR makes the audit log file transport configurable via Spring application.properties by wiring audit-log path/name and rolling-policy settings into logback-spring.xml.

Changes:

  • Read audit log file settings from openaev.audit-logs.file.* Spring properties.
  • Switch audit log rolling from time-based to size-and-time-based rolling, with configurable limits.
  • Add new openaev.audit-logs.file.* and openaev.audit-logs.engine.* configuration keys to application.properties.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
openaev-api/src/main/resources/logback-spring.xml Loads audit file transport settings from Spring properties and updates rolling policy to size+time-based rollover.
openaev-api/src/main/resources/application.properties Defines new audit-log configuration keys for file and engine transports.

Comment on lines +2 to +8
<!-- Audit log file path and name from Spring properties -->
<springProperty scope="context" name="AUDIT_LOG_DIR" source="openaev.audit-logs.file.dir" defaultValue="logs" />
<springProperty scope="context" name="AUDIT_LOG_FILENAME" source="openaev.audit-logs.file.filename" defaultValue="audit.log" />
<springProperty scope="context" name="AUDIT_LOG_ROLLOVER_MAX_SIZE" source="openaev.audit-logs.file.rollover-max-size" defaultValue="10MB" />
<springProperty scope="context" name="AUDIT_LOG_RETENTION_DAYS" source="openaev.audit-logs.file.retention-days" defaultValue="90" />
<springProperty scope="context" name="AUDIT_LOG_ROLLOVER_TOTAL_SIZE_CAP" source="openaev.audit-logs.file.rollover-total-size-cap" defaultValue="1GB" />
<property name="AUDIT_LOG_FILE" value="${AUDIT_LOG_DIR}/${AUDIT_LOG_FILENAME}" />
<maxHistory>90</maxHistory>
<totalSizeCap>1GB</totalSizeCap>
<rollingPolicy class="ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy">
<fileNamePattern>${AUDIT_LOG_DIR}/${AUDIT_LOG_FILENAME}.%d{yyyy-MM-dd}.%i</fileNamePattern>
<maxHistory>90</maxHistory>
<totalSizeCap>1GB</totalSizeCap>
<rollingPolicy class="ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy">
<fileNamePattern>${AUDIT_LOG_DIR}/${AUDIT_LOG_FILENAME}.%d{yyyy-MM-dd}.%i</fileNamePattern>
@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 43.50%. Comparing base (6fa23d7) to head (ce046b3).
⚠️ Report is 292 commits behind head on main.

❌ Your project check has failed because the head coverage (2.96%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #6377      +/-   ##
============================================
+ Coverage     43.41%   43.50%   +0.09%     
- Complexity     6924     6959      +35     
============================================
  Files          2249     2252       +3     
  Lines         61898    62027     +129     
  Branches       8142     8167      +25     
============================================
+ Hits          26870    26987     +117     
- Misses        33294    33303       +9     
- Partials       1734     1737       +3     
Flag Coverage Δ
backend 65.74% <ø> (+0.05%) ⬆️
e2e 18.31% <ø> (-0.43%) ⬇️
frontend 2.96% <ø> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

engine.audit-log-rollover-max-size=5gb
engine.audit-log-rollover-max-age=30d
# File transport configuration
openaev.audit-logs.file.dir=logs

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: Are we aware that is a breaking changes for client ? If we want to go that way, maybe we can keep the previous global variable and add the fine-grained one.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused here. Isn't that the previous global variable values here ? This should be transparent for client, no ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the variable was {AUDIT_LOG_DIR} which defaulted to logs. the file name was hardcoded audit.log we now have a props that default to same file name, i dont see any breaking chanhe

<maxHistory>90</maxHistory>
<totalSizeCap>1GB</totalSizeCap>
<rollingPolicy class="ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy">
<fileNamePattern>${AUDIT_LOG_DIR}/${AUDIT_LOG_FILENAME}.%d{yyyy-MM-dd}.%i</fileNamePattern>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been digging and it's possible when specifying the rollover format to set the format to .gz. In that case, the rolleover file will automatically be zipped. I'm wondering if we shouldn't set that by default here ? (I see no wrong in setting the rollover files as compressed given that the feature is still technically under FF)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

filigran team Item from the Filigran team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants