Better log levels in the message publisher #343
Conversation
|
I pushed a commit to fix the tests on older ruby versions; filter is an alias of select. Please review them independently. |
| @@ -184,7 +184,7 @@ def bindings | |||
|
|
|||
| filtered = api_client.bindings. | |||
| reject { |b| b['destination'] == b['routing_key'] }. | |||
| filter { |b| b['source'] == @config[:mq_exchange] && b['vhost'] == @config[:mq_vhost] } | |||
| select { |b| b['source'] == @config[:mq_exchange] && b['vhost'] == @config[:mq_vhost] } | |||
There was a problem hiding this comment.
How is this change relevant?
There was a problem hiding this comment.
This fixes build failures for older ruby versions, see travis builds: https://travis-ci.org/gocardless/hutch/jobs/652451121?utm_medium=notification&utm_source=github_status
I'm fine with any solution as long as the current verbosity goes down. Just let me know what changes you want me to make. |
|
@sdwolfz let's keep the changes you have but change publish logging level to |
Are you sure you want to have two |
|
Good point. I think simply switching to debug logging would be best. |
Having the entire payload logged at the "INFO" level is not ideal due to the following reasons: - PII data might be contained there and there is no way to fitler it out - Payload might be big enough that logging aggregators might struggle with the sheer amount of data - Since the log level is shared by reusing the rails/sinatra logger, INFO level might be too verbose for hutch but WARN level not verbose enough for the other. While initially I considered implementing a feature to allow for filtering the payload for PII or triming the output, a quick solution to all problems listed above is to change the logging statement to DEBUG, which should only be needed in development/testing environments.
|
@michaelklishin all done! ♻️ 🙏 |
|
Thank you! |
|
Cheers mate! Any hint on when this will be released to rubygems? |
|
We haven't updated release notes in a while so it will take some time. Hopefully in the next few days. |
|
@michaelklishin Can I help with that? I assume I need to open a PR with changelog lines for all PRs since the last tag. |
Having the entire payload logged at the "INFO" level is not ideal due to
the following reasons:
the sheer amount of data
might be too verbose for hutch but WARN level not verbose enough for the other.
While initially I considered implementing a feature to allow for filtering
the payload for PII or triming the output, a quick solution to all problems
listed above is to split the logging into two, the INFO level handing the
generic part of the log, with the message ID injected for easy tracking
and in case the payload is needed, log level can be set to DEBUG, which
should only be needed in development/testing environments.