Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

Skip Masking of Uvicorn Logs [#766] #831

Merged
merged 2 commits into from
Jul 7, 2022
Merged

Skip Masking of Uvicorn Logs [#766] #831

merged 2 commits into from
Jul 7, 2022

Conversation

pattisdr
Copy link
Contributor

@pattisdr pattisdr commented Jul 7, 2022 •
edited

Purpose

The fact that we mask uvicorn logs makes issues difficult to debug. Exclude these logs from being replaced with MASKED.

Changes

Checklist

  • Update CHANGELOG.md file
    • Merge in main so the most recent CHANGELOG.md file is being appended to
    • Add description within the Unreleased section in an appropriate category. Add a new category from the list at the top of the file if the needed one isn't already there.
    • Add a link to this PR at the end of the description with the PR number as the text. example: #1
  • Applicable documentation updated (guides, quickstart, postman collections, tutorial, fidesdemo, database diagram.
  • If docs updated (select one):
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Good unit test/integration test coverage
  • This PR contains a DB migration. If checked, the reviewer should confirm with the author that the down_revision correctly references the previous migration before merging
  • The Run Unsafe PR Checks label has been applied, and checks have passed, if this PR touches any external services

Ticket

Fixes #766

@pattisdr pattisdr mentioned this pull request Jul 7, 2022
Closed
@pattisdr pattisdr changed the title Skip Masking of Uvicorn Logs [#166] Skip Masking of Uvicorn Logs [#766] Jul 7, 2022
@pattisdr pattisdr marked this pull request as ready for review July 7, 2022 21:16
eastandwestwind
eastandwestwind approved these changes Jul 7, 2022
View reviewed changes
Copy link
Contributor

@eastandwestwind eastandwestwind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the iterative approach here, to get something working for now!

src/fidesops/util/logger.py
@@ -28,7 +28,7 @@ def factory( # pylint: disable=R0913
) -> logging.LogRecord:
env_log_pii: bool = os.getenv("FIDESOPS__LOG_PII", "").lower() == "true"
new_args = args
if not env_log_pii:
if not env_log_pii and not name.startswith("uvicorn"):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oooo good call that we can simply check name!

@eastandwestwind eastandwestwind merged commit a6b91e5 into main Jul 7, 2022
@eastandwestwind eastandwestwind deleted the fidesops_166_uvicorn_logs branch July 7, 2022 21:35
@pattisdr pattisdr mentioned this pull request Jul 8, 2022
Closed
sanders41 pushed a commit that referenced this pull request Sep 22, 2022
@pattisdr
Skip Masking of Uvicorn Logs [#766] (#831)
d05d8f9 
* Update get_fides_log_record_factory to skip masking of all uvicorn logs.

* Update changelog.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@eastandwestwind eastandwestwind eastandwestwind approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Stop Fidesops masking uvicorn API logs
2 participants
@pattisdr @eastandwestwind