Establish log rotation for persona app logs

[gene1wood]

/var/browserid/log/*.log

[gene1wood]

This needs to be completed as it caused these performance issues : https://bugzilla.mozilla.org/show_bug.cgi?id=959860

[gene1wood]

And this issue today : http://personastatus.org/#1390889760

[gene1wood]

Looking at the code that uses Winston for logging some ideas come to mind

1

Maybe if the only thing that uses var_path is the logging, we can just remove that configuration option, in doing so, disable logging to a file

https://github.com/mozilla/persona/blob/dev/lib/logging/transports/log-file.js#L22

and enable logging to the console with the LOG_TO_CONSOLE environment variable.

https://github.com/mozilla/persona/blob/dev/lib/logging/logging.js#L49

Then use something like Cronolog to handle the log rotation by piping console output to it.

If however other things in the app depend on var_path then we'd probably need to change the code to stop writing logs to files.

2

Another approach is to continue logging with winston, and modify the app to set a maxsize option : http://stackoverflow.com/a/11409049/168874

3

A third option would be to modify the app with something like this https://gist.github.com/suprememoocow/5133080 and use logrotate to rotate the logs then HUP the app to trigger using the new file.

[jrgm]

Just noting that output to stderr and stdout is already being logged and rotated on every 1MB of output (although almost nothing is going there at the moment). It's multilog from daemontools.

[gene1wood]

@jrgm Excellent, good point. So ya, option 1 doesn't need cronolog.

[gene1wood]

@shane-tomlinson how viable do you think option 1 is above, removing the var_path config setting in hopes of forcing logs to go to the console? Are there any other parts of persona that depend on var_path?

[jrgm]

So, speaking strictly of the code as-is on what's the current production release, var_path is used this way:

1. var_path is also the location of other configuration information (e.g., the cookie secret)
2. If not set, then the default value is __dirname/var. i.e., there is no way to have no value.
3. https://github.com/mozilla/persona/blob/dev/lib/logging/transports/log-file.js#L22 is the code on HEAD, but that's not the current branch in production. (Actually, the code there is wrong, since log_path cannot be "falsy" after the line above).

So, option 1 is out as a no-code-change option, although I really liked this option and just let multilog take care of logfile rotation and expiry.

Code changes for option 2 to the 10.09 branch would look like https://gist.github.com/jrgm/8834431

[gene1wood]

@jrgm I like your option 2 gist, I added some comments to it. Want to turn it into a PR?

[gene1wood]

I've asked Edwin when we can get QA time to merge and test this so we can release.

[gene1wood]

After merging this was tested in stage and deployed to production.