dateFile misleading parameter daysToKeep

[chienfuchen32]

Hi, I've tried to use log4js dateFile recently.

daysToKeep - integer (default 0) - if this value is greater than zero, then files older than that many days will be deleted during log rolling.

dateFile Document

When I test a logger like the example,
after a while, its behavior like rotating with numbers of archives.
Then I check the source code of dateFile, it implement DateRollingFileStream form streamroller,

and I look into DateRollingFileStream,

class DateRollingFileStream extends RollingFileWriteStream {
  constructor(filename, pattern, options) {
    //...
    if (options.daysToKeep) {
      options.numToKeep = options.daysToKeep;
    }
}

DateRollingFileStream inherit from RollingFileWriteStream which provide cleaning up mechanism based on numToKeep

class RollingFileWriteStream extends Writable {
  //...
  // Sorted from the oldest to the latest
  async _getExistingFiles() {
    const files = await fs.readdir(this.fileObject.dir).catch(() => []);

    debug(`_getExistingFiles: files=${files}`);
    const existingFileDetails = files
      .map(n => this.fileNameParser(n))
      .filter(n => n);

    const getKey = n =>
      (n.timestamp ? n.timestamp : newNow().getTime()) - n.index;
    existingFileDetails.sort((a, b) => getKey(a) - getKey(b));

    return existingFileDetails;
  }
  //...
  async _clean() {
    const existingFileDetails = await this._getExistingFiles();
    debug(
      `_clean: numToKeep = ${this.options.numToKeep}, existingFiles = ${existingFileDetails.length}`
    );
    debug("_clean: existing files are: ", existingFileDetails);
    if (this._tooManyFiles(existingFileDetails.length)) {
      const fileNamesToRemove = existingFileDetails
        .slice(0, existingFileDetails.length - this.options.numToKeep - 1)
        .map(f => path.format({ dir: this.fileObject.dir, base: f.filename }));
      await deleteFiles(fileNamesToRemove);
    }
  }
}

I think it's kind of confusing documentation... or if there's a plan that streamroller implement file rotation based on file modified time or datetime naming rule in the future?
Thank you. :D

[orbulant]

I agree, "daysToKeep' should actually be renamed to "filesToKeep".

[lamweili]

It's renamed to numBackups instead for clarity while daysToKeep is still backward compatible.
Usage of daysToKeep will result in process.emitWarning of deprecation.

https://github.com/log4js-node/streamroller/blob/272e5e32e742021a4340ab147f7bea24c8924ddd/lib/DateRollingFileStream.js#L21-L25

process.emitWarning(
  "options.daysToKeep is deprecated due the confusion it causes when used " + 
  "together with file size rolling. Please use options.numBackups instead.",
  "DeprecationWarning", "StreamRoller0001"
);