LG-14334 | Don't log redacted values #11158
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
changelog: Internal, Logging, Don't log "[REDACTED]" fields in TMX response since they are useless
|
There are actually 185 attributes allowed. It's possible that other analytics parameters might still push us to a bit over 200. We could probably pare this list down further, but I would advocate that any such work be done in a followup PR. |
n1zyy
commented
Aug 28, 2024
| @@ -197,12 +197,7 @@ class ResponseRedacter | |||
| def self.redact(hash) | |||
| return { error: 'TMx response body was empty' } if hash.nil? | |||
| return { error: 'TMx response body was malformed' } unless hash.is_a? Hash | |||
| filtered_response_h = hash.slice(*ALLOWED_RESPONSE_FIELDS) | |||
| unfiltered_keys = hash.keys - filtered_response_h.keys | |||
There was a problem hiding this comment.
I briefly had the thought that we could keep this line, and then do something like:
filtered_response_h['redacted_keys'] = unfiltered_keys.join(", ")I don't see a need for this, though.
n1zyy
commented
Aug 28, 2024
| filtered_response_h[key] = '[redacted]' | ||
| end | ||
| filtered_response_h | ||
| hash.slice(*ALLOWED_RESPONSE_FIELDS) |
There was a problem hiding this comment.
You can read this PR as lines 201-204 being deleted, and then me removing the variable assignment on line 200.
matthinz
approved these changes
Aug 30, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🎫 Ticket
Link to the relevant ticket:
LG-14334
🛠 Summary of changes
Note: This is a discussion about the literal string
[redacted]being logged. Its use in the following paragraphs is about the literal word "redacted" in brackets, not a representation that information has been redacted.We log an analytics event with the ThreatMetrix response as a hash, which includes several hundred fields—so many that CloudWatch stops parsing them. (It has a limit of 200 fields that it will parse out.) This means that sometimes, important attributes cannot be queried through CloudWatch because they were the >200th field.
A large number of these logged parameters have the literal value "[redacted]", so they are effectively junk. These come from the ResponseRedacter class, which replaces any non-allowlisted attribute with "[redacted]" to guard against accidentally logging PII.
Because the sheer volume of unusable attributes is breaking things, this PR changes the ResponseRedacter to only return the non-redacted attributes. Fields which previously would have had their value replaced with "[redacted]" are now not logged at all. This means that:
ALLOWED_RESPONSE_FIELDSin the redacter class)