Rcomian / bunyan-rotating-file-stream

Is an endpoint for bunyan that lets you control how much disk space logs take.
Other
29 stars 15 forks source link

rotateExisting config property not working correctly #9

Closed markgillnz closed 7 years ago

markgillnz commented 7 years ago

When using the config property rotateExisting (combined with period), it does not work as expected as a new file is not created when the old file is past the period configured.

The function checkIfRotationNeeded in initialperiodtrigger expects a datetime value to compare against Date.now() but receives data.path which is undefined.

Changing var rotation = checkIfRotationNeeded(data.path, Date.now()); to var rotation = checkIfRotationNeeded(Date.parse(data.stats.ctime), Date.now()); seems to work for me.

Am I correct or am I using the property wrongly? My config is const rotatingFileStream = { type: 'raw', level: 'debug', stream: new RotatingFileStream({ path: 'logfile.%Y-%m-%d.log', period: '1d', rotateExisting: true, totalFiles: 0 }) };

mauvm commented 7 years ago

Same issue here, while doing development I have a log file of Jan 2nd (log.2017-01-02.json), if I run my program it will write the log to that file instead of creating one for today - Jan 6th (log.2017-01-06.json).

@markgillnz Using the last modified timestamp (data.stats.ctime) to determine if rotation is needed won't work if the file gets modified afterwards. It would be safer to parse the date from the filename, but I'm not sure how to properly do this.

PS. The totalFiles option is 0 (disabled) by default and can be omitted.

Rcomian commented 7 years ago

This looks like a bug, I'll check what's happening. Your configuration looks correct.

Rcomian commented 7 years ago

Ok, I've resolved this by checking the birthtime of the file in version 1.6.2.

Hopefully this should work as you expect. There are some issues with EXT4 filesystem systems on Linux not returning this value - so it won't work on there, but it should work on windows systems.

mauvm commented 7 years ago

I'm on EXT4 and I can see data.stats.birthtime exists, however it's still not rotating correctly: I ran npm start just now (around 2017-01-13T08:40:00Z) and it writes to the log file of yesterday (log.2017-01-12.json). Dump of data.stats:

{ dev: 43,
  mode: 33204,
  nlink: 1,
  uid: 1000,
  gid: 1000,
  rdev: 0,
  blksize: 4096,
  ino: 6325447,
  size: 16843,
  blocks: 56,
  atime: 2017-01-13T08:19:43.589Z,
  mtime: 2017-01-13T08:41:10.326Z,
  ctime: 2017-01-13T08:41:10.326Z,
  birthtime: 2017-01-13T08:41:10.326Z }
Rcomian commented 7 years ago

Hmm, the problem with EXT4 is that the birthtime reporting is actually the date of the last modification of the file. So if you start your application within 24 hours of the last write to that file, it will still pick the old file. It might also cascade the error - essentially prolonging the file's life forever unless there's a period of > 24 hours between opening, or the program runs for more than 24 hours.

I'll have a look to see what I can do about this, but in general I'd suggest forcing the start of a new file each time using the startNewFile: true option.

Apologies, but there really is no sensible way to get the creation time of a file under EXT4 - I'm suffering from this as well.