Add ability to provide fields to remove from Rollbar payload [CIRCLE-16074] #41
Conversation
This file contains hidden or 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
technomancy
approved these changes
Feb 27, 2019
src/circleci/rollcage/json.clj
Outdated
| (json/generate-string item {:key-fn snake-case}) | ||
| (catch JsonGenerationException _ | ||
| (json/generate-string (postwalk backstop-encoder item) {:key-fn snake-case})))) | ||
| [item block-fields] |
There was a problem hiding this comment.
Since this is a public function, maybe we should include an arity where block-fields may be omitted so it can default to an empty list?
marcomorain
suggested changes
Feb 27, 2019
src/circleci/rollcage/core.clj
Outdated
| :block-fields | ||
| A list of fields to remove/scrub from the payload prior to sending to Rollbar | ||
| using snake case keywords. The list will be automatically expanded into different |
There was a problem hiding this comment.
Can you make it clear what the input is expected to be, please? It looks like Kebab case keywords are expected.
src/circleci/rollcage/core.clj
Outdated
| "Send a Rollbar item using the HTTP REST API. | ||
| Return the result JSON parsed as a Map" | ||
| [^String endpoint ^Throwable exception item] | ||
| [block-fields ^String endpoint ^Throwable exception item] |
There was a problem hiding this comment.
The blocking of fields should happen higher up the callstack. I would put it into the notify function.
We need to separate concerns so that we can test in isolation.
This function is the function that sends items over HTTP, and should not be concerned with the contents of an item. make-rollbar returns an item in the shape expected by the Rollbar API.
from Rollbar payload. This allows services to customize what PII information should not be logged to rollbar. Rename
2808ef0 to
d5c2b32
Compare
marcomorain
pushed a commit
that referenced
this pull request
Feb 27, 2019
generated from commit 55b3cf5
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This allows services to customize what PII information should not be logged to Rollbar.
This change is prompted by the following JIRA: https://circleci.atlassian.net/browse/CIRCLE-16074
Considerations: