search

expressjs / morgan

HTTP request logger middleware for node.js
MIT License
7.95k stars 536 forks source link

Updating code example for logging on rotation #221

Closed seven-deuce closed 4 years ago

seven-deuce commented 4 years ago

'rotating-file-stream' api has changed and most people will install the latest. So the example should adapt to that. Also the previous code example was incomplete and left many people wondering about details of implementation. My example is complete and short. Anyone can copy-paste it and get it immediately working. No need to go to the 'rotating-file-stream' library and read its documentation!

dougwilson commented 4 years ago

Also the previous code example was incomplete and left many people wondering about details of implementation.

Can you elaborate on that? What details were missing? Just reading the changes here, I'm still not sure what exactly is going on in that regard.

My example is complete and short. Anyone can copy-paste it and get it immediately working. No need to go to the 'rotating-file-stream' library and read its documentation!

I'm confused, as I believe the current documentation is already that way? At least, I copied out the example and it worked for me without modification. Can you elaborate on this point?

seven-deuce commented 4 years ago

@dougwilson So if the log files are on rotation, it means that the file names should change. In current example, filename is just a string: 'access.log'. Which will result in overwriting the same file, which does not serve the purpose of having new files created and all logs kept separately.
I have replaced it with generateName function, which creates the correct filenames. current documentation for 'rotating-file-stream' is NOT that way anymore. The old code will not run with the current api of that library. rfs.createStream should be used instead of rfs Also I had some problems with smooth creation of logs, the easiest way to fix them was to set immutable: true, , this is also a side-effect of changes made in 'rotating-file-stream' api

dougwilson commented 4 years ago

Thank you for the feedback. That all sound good, but sounds like feedback to make to the rotating-file-stream about the default options. I would rather keep the example simple, and let users expand it as they need. The example is just to get it to work.

On your rotation note, replacing the same file is still rotating the file; it sounds like your need it to keep the older data, which is fine, but not everyone needs that.

The goal of the example is to use as little non-morgan code as possible to show how to use an external stream with morgan.