-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
log4j2: Use size based triggering instead of time based #9849
Conversation
@blueorangutan package |
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
@vishesh92 the new file names are not user-friendly. It will be difficult to find the logs of a specific date. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9849 +/- ##
==========================================
Coverage 15.78% 15.78%
- Complexity 12564 12565 +1
==========================================
Files 5627 5627
Lines 492250 492250
Branches 61405 61913 +508
==========================================
Hits 77710 77710
Misses 406066 406066
Partials 8474 8474
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vishesh92 maybe we can keep the default triggering policy to be time based but document how size based policy could be used? So the PR is essentially changes in replace.properties & a separate doc PR to given examples on configuration of logging.
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11426 |
actually we could add both policies in the packages and install on mgmt server and agent, and let user to choose. I also agree with @NuxRo that it would be better to add the timestamp to the filename, instead of `%i%.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove size based trigger policy from the PR - but instead create a doc PR for users who would want it, on how to do that.
Created a doc PR for this here: apache/cloudstack-documentation#450 Closing this PR. |
Description
This PR changes the triggering policy for log4j from time based to size based. This helps prevents issues where disk gets full due to logs on the disk and the application becomes unavailable.
I have set the max file size to 100 MB and the maximum number of old logs files to be retained to 10. At max the storage required for a single log file will be 100MB * (10 old log files + 1 current log file) = 1100MB. The actual size would be less since the old log files are gzipped.
Added these 2 variables which are configured in replace.properties:
LOGMAXINDEX
- Maximum number of old files to retain.LOGMAXFILESIZE
- Maximum size of the log file before rotationTypes of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
How Has This Been Tested?
How did you try to break this feature and the system with this change?