Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃毀 Change Dynimoid logging level to :info #118

Closed
wants to merge 1 commit into from
Closed

Conversation

@huwd
Copy link
Contributor

huwd commented Mar 25, 2020

Trello: https://trello.com/c/99xm7mlq/86-ensure-sensitive-data-doesnt-go-to-logit-splunk

DRAFT PENDING TEST ON STAGING
Replaced by: #168

Test

  • Get this on staging
  • Noodle around
    • Normal browse behaviour
    • Cause an error
    • Submit a stand-in for PII
    • Cause validation error
  • Logs should have No PII
  • Logs should have validation errors
  • Logs should have errors
  • check the logs since deploy

What

tweak the logging levels in production to prevent PII getting in, but ensure we still get useful logs on validation attempts

Why

Keep cyber happy, keep debugging devs happy.

@huwd huwd requested a review from richardTowers Mar 25, 2020
@bevanloon bevanloon temporarily deployed to coronavirus-change-log-ukllkf8 Mar 25, 2020 Inactive
@bilbof
bilbof approved these changes Mar 25, 2020
Copy link
Contributor

bilbof left a comment

Looks good 馃憤

@huwd

This comment has been minimized.

Copy link
Contributor Author

huwd commented Mar 25, 2020

Made a paralell one for the vulnerable people form:
Sean noted - alphagov/govuk-coronavirus-vulnerable-people-form#172

Do turns out logs can have PII in them cry - We ended up having reduce the log level #141. I think Dynamoid is sharing the default Rails logger - so we are suppressing all Rails log with setting. Maybe need to make a separate logger? Think we need info level for rails as validation errors probably aren't "application errors".

@huwd huwd marked this pull request as ready for review Mar 30, 2020
@huwd huwd changed the title Change Dynimoid logging level to :info [Do not merge] Change Dynimoid logging level to :info Mar 30, 2020
@huwd huwd changed the title [Do not merge] Change Dynimoid logging level to :info 馃毀 Change Dynimoid logging level to :info Mar 30, 2020
@huwd huwd added the do not merge label Mar 30, 2020
@huwd

This comment has been minimized.

Copy link
Contributor Author

huwd commented Mar 30, 2020

This should go on hold or be closed depending on the outcome of this PR:
alphagov/govuk-coronavirus-vulnerable-people-form#205

I suggest we get the response right there and then mirror it from the vulnerable people app to this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can鈥檛 perform that action at this time.