-
Notifications
You must be signed in to change notification settings - Fork 41.7k
Add Log4j2 rolling policy configuration #47260
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
base: main
Are you sure you want to change the base?
Conversation
66fcaf7 to
b5d3b84
Compare
936ec72 to
05f8bb4
Compare
|
@hojooo can you please rebase this PR to @ppkarwasz would you have a min to review this one please? I am wondering, in particular, about |
|
@snicoll Got it, thanks. I’ll update the PR shortly. |
Signed-off-by: hojooo <ghwn5833@gmail.com>
Log4j2: Introduce SpringBootTriggeringPolicy and wire plugin discovery
- Add SpringBootTriggeringPolicy plugin supporting size, time, size-and-time, and cron strategies
- Read rolling strategy parameters from LOG4J2_ROLLINGPOLICY_* system properties (fallback to attributes)
- Register Boot plugin package in Log4J2LoggingSystem to ensure plugin discovery
- Use SpringBootTriggeringPolicy under a top-level Policies wrapper in log4j2-file.xml
Signed-off-by: hojooo <ghwn5833@gmail.com>
Log4j2: Add property-driven rolling strategy support and metadata
- Introduce rolling policy properties: strategy, time-based.interval, time-based.modulate, cron.schedule
- Extend Log4j2RollingPolicySystemProperty with STRATEGY, TIME_INTERVAL, TIME_MODULATE, CRON_SCHEDULE
- Propagate new properties in Log4j2LoggingSystemProperties (with deprecated fallback guarded for null)
- Document new properties in configuration metadata
- Update Log4j2LoggingSystemPropertiesTests to assert new system properties mappingAdd Log4j2 rolling policy configuration support
Signed-off-by: hojooo <ghwn5833@gmail.com>
Tests: Initialize with packaged file config and unwrap composite policy
- Initialize with classpath:org/springframework/boot/logging/log4j2/log4j2-file.xml to validate file-based rolling
- Unwrap CompositeTriggeringPolicy to locate nested SpringBootTriggeringPolicy and assert its delegate
- Keep assertions for time, size-and-time, and cron strategies
Signed-off-by: hojooo <ghwn5833@gmail.com>
Refactor Builder import path
Signed-off-by: hojooo <ghwn5833@gmail.com>
add clean-history-on-start and total-size-cap property in log4j2-file.xml
Signed-off-by: hojooo <ghwn5833@gmail.com>
refactor indent
Signed-off-by: hojooo <ghwn5833@gmail.com>
Refactor Log4j2LoggingSystemPropertiesTests to verify System.setProperty path
Signed-off-by: hojooo <ghwn5833@gmail.com>
Add Rolling Policy
Signed-off-by: hojooo <ghwn5833@gmail.com>
2e0bb46 to
ec926ad
Compare
Signed-off-by: hojooo <ghwn5833@gmail.com>
Signed-off-by: hojooo <ghwn5833@gmail.com>
Signed-off-by: hojooo <ghwn5833@gmail.com>
Signed-off-by: hojooo <ghwn5833@gmail.com>
Signed-off-by: hojooo <ghwn5833@gmail.com>
Signed-off-by: hojooo <ghwn5833@gmail.com>
Signed-off-by: hojooo <ghwn5833@gmail.com>
Signed-off-by: hojooo <ghwn5833@gmail.com>
Signed-off-by: hojooo <ghwn5833@gmail.com>
Signed-off-by: hojooo <ghwn5833@gmail.com>
|
@hojooo can you please stop pushing one commit at a time like that? Every time you do, it sends a notification to everyone watching this repo, which adds quite a bit of noise. Please build locally and push once you're ready. |
|
@snicoll Thanks for the heads-up, and sorry for the noise I caused. I didn’t realize each small commit was sending notifications to everyone. I’ll make sure to build and test locally and then push in larger batches once things are ready. |
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.
From a Log4j point of view, this PR looks overall correct, although it could be simplified and there are some smaller issues.
| private final TriggeringPolicy delegate; | ||
|
|
||
| private SpringBootTriggeringPolicy(TriggeringPolicy delegate) { | ||
| this.delegate = delegate; | ||
| } |
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.
There is no need to implement a wrapper for a TriggeringPolicy. You can return the appropriate triggering policy directly from your factory method:
public abstract class SpringBootTriggeringPolicy implements TriggeringPolicy {
private SpringBootTriggeringPolicy() {
// no instantiation
}
public static TriggeringPolicy createPolicy(@PluginAttribute("strategy") @Nullable String strategy,
@PluginAttribute("maxFileSize") @Nullable String maxFileSize,
@PluginAttribute("timeInterval") @Nullable Integer timeInterval,
@PluginAttribute("timeModulate") @Nullable Boolean timeModulate,
@PluginAttribute("cronExpression") @Nullable String cronExpression,
@PluginConfiguration Configuration configuration) {
...
}
}Factory methods can return whatever they want (see MongoDbProvider), although there is a small limitation in the injector that assumes they return the same type as the plugin type, which is why SpringBootTriggeringPolicy needs to implement TriggeringPolicy.
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.
Thank you for the feedback! I agree this is a much cleaner approach. The wrapper pattern was adding unnecessary complexity when the factory can simply return the standard Log4j2 policies directly
| @PluginBuilderFactory | ||
| public static SpringBootTriggeringPolicyBuilder newBuilder() { | ||
| return new SpringBootTriggeringPolicyBuilder(); | ||
| } | ||
|
|
||
| @PluginFactory | ||
| public static SpringBootTriggeringPolicy createPolicy(@PluginAttribute("strategy") @Nullable String strategy, | ||
| @PluginAttribute("maxFileSize") @Nullable String maxFileSize, | ||
| @PluginAttribute("timeInterval") @Nullable Integer timeInterval, | ||
| @PluginAttribute("timeModulate") @Nullable Boolean timeModulate, | ||
| @PluginAttribute("cronExpression") @Nullable String cronExpression, | ||
| @PluginConfiguration Configuration configuration) { |
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.
There is no need for both a factory method and a builder. Since the builder is easier to maintain than the factory method, you should leave just the builder.
| cleanHistoryOnStart="${sys:LOG4J2_ROLLINGPOLICY_CLEAN_HISTORY_ON_START:-false}" | ||
| totalSizeCap="${sys:LOG4J2_ROLLINGPOLICY_TOTAL_SIZE_CAP:-0}"/> |
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.
The attributes cleanHistoryOnStart and totalSizeCap do not exist, see the DefaultRolloverStrategy documentation and the automatically generated list of configuration attributes. You would need to implement them in a custom rollover strategy:
- If you want to rollover at each JVM startup, you can use
OnStartupTriggeringPolicy, - If you want a limit on the total size of archived files, you need to configure the appropriate
Deleteactions.
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.
Thank you for the clarification. I've removed these non-existent attributes from DefaultRolloverStrategy.
I’ve removed those non-existent attributes from DefaultRolloverStrategy, so the configuration is now:
<DefaultRolloverStrategy max="${sys:LOG4J2_ROLLINGPOLICY_MAX_HISTORY:-7}"/>
To keep this change focused, I’m not wiring in OnStartupTriggeringPolicy or Delete actions yet.
If we need startup-based rollover or a total size cap in the future, we can follow up with a custom rollover strategy and Delete actions in a separate change.
Signed-off-by: hojooo <ghwn5833@gmail.com>
Summary
Hello! Log4j2 does not support rolling policies. So this PR adds basic rolling policy configuration property support for Log4j2. It introduces Log4j2 specific properties equivalent to the existing
logging.logback.rollingpolicy.*properties, enabling consistent logging configuration through application.properties across different logging implementations.And advanced rolling strategy support to the Log4j2 rolling policy configuration. It introduces the
SpringBootTriggeringPolicyplugin to enable various rolling strategies and strategy-specific detailed configuration through application.properties.Changes
1. Standardized Log4j2 Rolling Policy Properties
We've added standard properties for Log4j2, similar to the existing
logging.logback.rollingpolicy.*properties. Users can now easily control the rolling policy using the following attributes inapplication.properties:2. Advanced Rolling Strategy
A custom Log4j2 plugin,
SpringBootTriggeringPolicy, has been introduced to support various advanced rolling strategies beyond simple size-based rolling.size(default): Rolls files based on their size.time: Rolls files based on a time interval.size-and-time: Rolls when both size and time conditions are met.cron: Rolls based on a cron expression schedule.Implementation
1. Property Standardization Implementation
Log4j2RollingPolicySystemProperty: An enum was implemented to map Spring Environment properties to system properties that Log4j2 can recognize (e.g.,max-file-size->LOG4J2_ROLLINGPOLICY_MAX_FILE_SIZE).Log4j2LoggingSystemProperties: This class handles the automatic conversion of Spring'sDataSizetype to bytes and ensures backward compatibility with deprecated properties likelogging.file.max-size.log4j2-file.xml: The configuration file was updated to reference the newly defined system properties using the${sys:PROPERTY_NAME}syntax.2. Advanced Rolling Strategy Implementation
SpringBootTriggeringPolicyPlugin: A customTriggeringPolicywas implemented using Log4j2's@Pluginannotation. Based on thestrategyproperty, this plugin internally selects and delegates to the appropriate policy (e.g., SizeBasedTriggeringPolicy, TimeBasedTriggeringPolicy).log4j2-file.xmlUpdate: The<Policies>block was modified to useSpringBootTriggeringPolicy, with its parameters (maxFileSize, timeInterval, cronExpression, etc.) configured via system properties.