tupelo-schneck opened a new pull request, #4227:
URL: https://github.com/apache/logging-log4j2/pull/4227
**This changes the names of direct write files for cron based appenders.**
That is the point of the change, but it is user visible on upgrade, so it
deserves a decision rather than a rubber stamp. Details below.
An appender configured without a `fileName` writes directly to the file its
pattern resolves to. That file covers a whole rollover period, but it is
currently named after the moment the appender started rather than after the
period it covers.
With a weekly schedule and a day-granularity pattern:
```xml
<RollingFile name="errorLogAppender"
filePattern="${LOG_DIR}/error.log-%d{yyyyMMdd}">
<PatternLayout pattern="%m%n"/>
<CronTriggeringPolicy schedule="0 0 0 ? * SUN" evaluateOnStartup="true"/>
</RollingFile>
```
starting on Tuesday 2026-07-28 writes to `error.log-20260728`. The period
began on Sunday 2026-07-26, so a restart on Wednesday opens
`error.log-20260729` instead of continuing the same file. A week with daily
restarts leaves seven files where the schedule implies one, and rollover only
ever closes out whichever fragment happens to be current.
## Cause
`CronTriggeringPolicy.initialize()` computes the period start and records it
correctly:
```java
final Date lastRegularRoll = cronExpression.getPrevFireTime(new Date());
aManager.getPatternProcessor().setCurrentFileTime(lastRegularRoll.getTime());
```
`DirectWriteRolloverStrategy.getCurrentFileName()` then discards it:
```java
// LOG4J2-3339 - Always use the current time for new direct write files.
manager.getPatternProcessor().setCurrentFileTime(System.currentTimeMillis());
```
That override compensates for a different problem.
`CronTriggeringPolicy.rollover()` passes `lastRollDate`, the *previous*
period's start, as the new file's time:
```java
manager.rollover(cronExpression.getPrevFireTime(rollTime), lastRollDate);
```
and `RollingFileManager.rollover(prevFileTime, prevRollTime)` assigns that
to the current file time, so each replacement file was named after the file
just rolled. Substituting the current time hid this, and works only because at
the instant of a rollover "now" is approximately the new period's start. At
startup "now" is an arbitrary point inside the period, which is where it breaks.
## Fix
Pass the roll time rather than the previous roll date, so the replacement
file is named after the period it opens, and let `getCurrentFileName()` use the
period start the policy already recorded.
Blast radius is limited to policies that track a period.
`PatternProcessor.formatFileName()` already falls back to the current time when
the current file time is 0:
```java
final long time = useCurrentTime
? currentFileTime != 0 ? currentFileTime : System.currentTimeMillis()
: prevFileTime != 0 ? prevFileTime : System.currentTimeMillis();
```
and `PatternProcessor.updateTime()` resets it to 0, so
`TimeBasedTriggeringPolicy`, which never sets it, keeps its existing behaviour
unchanged. Only cron, which knows its period start, behaves differently.
## Compatibility
Direct write files for cron based appenders change name, from the start time
of the process to the start of the rollover period. For the example above that
is `error.log-20260726` rather than `error.log-20260728`. Existing files are
untouched and are neither read nor renamed; a restart after upgrading simply
begins writing to the period's file. The changelog entry calls this out.
Appenders configured *with* a `fileName` are unaffected, since
`getCurrentFileName()` is only consulted on the direct write path.
## Testing
`CronTriggeringPolicyTest#testDirectWriteFileNameUsesPeriodStart` asserts
the manager's file name matches the period start computed independently from
the same cron expression. Against `2.x` without the fix it fails with:
```
expected: <target/testcmd5.log-20260726> but was:
<target/testcmd5.log-20260728>
```
`RollingAppenderDirectCronTest`, the regression test added with LOG4J2-3339,
still passes; it asserts that a rolled file's name matches the timestamp of its
first line, which this change preserves.
`./mvnw verify` passes across all 41 modules: 11088 tests, 0 failures, 0
errors, 40 pre-existing environment-conditional skips.
## Relationship to the other PR
Independent of the startup-delay fix in the companion PR; the two touch
different code paths and can merge in either order. They surfaced from the same
investigation, which is why both concern `CronTriggeringPolicy`.
## Checklist
* [x] Based on `2.x`
* [x] `./mvnw verify` succeeds
* [x] Changelog entry in `src/changelog/.2.x.x`
* [x] Tests are provided
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
