Arvados

Commit 39292137 suppresses manifest_text from collection logs. Added test after re-purposing existing test.

Looks good to me, took me a while understanding that we were logging through a Log object.
If we are logging to the database using the Log model, and the creation of Log objects are logged to production.log file, wouldn't be possible to avoid logging Log's events to said file? I spent some time searching for this and all I could find were custom logging solutions, not customizing the built-in logger.

Lucas: since we use the log files for debug logging as well, the log files and the Log table serve different purposes. The Log table is also used by the crunch dispatcher to log events. Various clients such as jobs, workbench also subscribe to live logs using web sockets, which I think are served from the Log table. BTW, we also have a task in api server named "delete_old_job_logs.rake" that deletes the job log entries as specified (because the crunch job logs are expected to be in keep by then).

Tom Clegg wrote:

My impression is that the problem report concerns a case of production.log filling up with Rails request logs -- not SQL logs or our own logs about model changes.

For example, an unauthenticated client could send a lot of unroutable requests with huge POST bodies and produce the same problem, even though those requests never even hit our application code.

you're correct, but we went from ~100 Mb to 27 Gb (uncompressed) per day. Even a DDoS will have a hard time creating this kind of log size.

Losing the copy of the old manifest_text in the logs table addresses a different problem ("postgres database is too big", not "production.log is too big") and represents a loss of functionality. I'd like to revert this, or at least put it behind a config knob, because

• I expect some sites still want an audit trail even though it makes their Postgres database bigger.
• #9363.

I have no real objection here, but bear in mind that if "the possibility of an audit" is really affecting performance in postgres too we need to review logs storing process as a whole. For the record, pg database is store in a separate /data partition, production.log in a much smaller one, that's why the disk filled up

Nico Cesar wrote:

you're correct, but we went from ~100 Mb to 27 Gb (uncompressed) per day. Even a DDoS will have a hard time creating this kind of log size.

1 MB POST request per second = 86 GB logs per day ... not hard enough!

9709-restore-manifest-logging

test 2bd7edf29bad74e61da31b801afd85b4c33ef7fe

Just a couple minor comments.

• I think it would be nice to have both tests here with and without attribute suppression config setting: test "manifest_text not included in collection logs". You might also want to consider testing having multiple values in the config setting.

• In the config yaml comment: “# Attributes to avoid saving in events and audit logs” => can you use the word “suppress” instead of “saving” here?

LGTM