search

gbv / jskos-server

Web service to access JSKOS data
https://coli-conc.gbv.de/api/
MIT License
6 stars 4 forks source link

Support access logging #172

Closed nichtich closed 2 years ago

nichtich commented 2 years ago

For monitoring there should be a configuration option to specify a logfile for access logs, e.g. with https://github.com/expressjs/morgan.

stefandesu commented 2 years ago

Should access logs be completely separate from error/warning logs? Currently, we log a few selected messages (depending on the config option verbosity) to the console. Normally, I'd say that all logs should be logged to STDOUT and the process manager (e.g. pm2) should deal with the rest, but we could also keep them separate and add an option just for access logs that are written to a specific file (or to STDOUT if the user wants that).

Integrating morgan works well, I now have it running locally and I'm testing log rotation via rotating-file-stream as well. It logs requests even if they result in an error.

nichtich commented 2 years ago

STDOUT and STDERR are captured by pm2, so just logging requests to STDOUT seems easiest, also to not have too many logfiles. Logfile rotating should also be handled outside of the application. Better directly log to STDOUT e.g. with https://www.npmjs.com/package/access-log. Could be enabled with a boolean configuration flag.

stefandesu commented 2 years ago

Yeah that sounds reasonable. Though I don't understand why we should use a small (as in barely anyone uses it) package like access-log when morgan that you suggested first can also do this. (Both haven't been updated in a while, but morgan seems like the defacto default library for this case and is related to express.js as well.)

I will implement this with a flag.

stefandesu commented 2 years ago

I utilized the existing config option verbosity and enabled access logging when it is set to log (default is warn so it's disabled by default). I also adjusted the "Standard Apache combined" output format to start with the ISO date (same as our existing logs) and removed user information.