-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[processor/redaction] apply redaction to log.body (#37239) #37369
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
8c9b0ca
to
3db3e73
Compare
@dmitryax @mx-psi @TylerHelmuth @jpkrohling Any comment on the approach? I tried to keep the implementation as basic as possible. I can add a changelog and cover it with the tests if it seems good to go. |
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@karakayasemi please add some tests for this new functionality
3afc5d2
to
41f94cb
Compare
@TylerHelmuth I've added corresponding tests for |
hey @karakayasemi the current implementation covers the situation when the Log's body is a string or one-level map value. However, if users use highly structured logs with nested map values (I'm one of these users🙂) it will not work (nested properties will be ignored and not redacted/masked). should we take into account the use-case of nested-structured logs body within the implementation? (otherwise this feature feels half-baked) |
Hey @gray-oni, The current implementation processes map values similarly to attribute handling—it converts them to a string, applies redaction, and then sets the modified value back. You can check the implementation here. From this perspective, it should work as expected for now. However, I agree that supporting nested map structures would be a good improvement. But, IMHO, since this would impact logs, metrics, and traces, I believe it would be better to track it as a separate issue rather than expand the scope of this PR. What do you think? |
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
8ca8458
to
531b24a
Compare
8eb1482
to
f9bf49a
Compare
11733db
to
552720b
Compare
@mx-psi @TylerHelmuth @dmitryax @evan-bradley @jpkrohling — Anything I can do to get attention on this PR? Appreciate your review when you get a chance! |
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
@dmitryax Since you started reviewing this, would you mind taking another look? |
552720b
to
a7a1791
Compare
a7a1791
to
4ebc42c
Compare
Passes my review. @mx-psi @TylerHelmuth @dmitryax please review as codeowners. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delayed response. The PR LGTM
…) (open-telemetry#37369) ## Description This update applies redaction rules to the `log.body` field by supporting multiple data types, including string, slices, maps, and nested maps. It applies key-based configurations consistently across all map structures. - **Key-Based Configurations**: Applies redaction rules at all nesting levels. - **New Debug Attributes**: - `redaction.body.redacted.keys` / `redaction.body.redacted.count` - `redaction.body.masked.keys` / `redaction.body.masked.count` - `redaction.body.allowed.keys` / `redaction.body.allowed.count` - `redaction.body.ignored.count` #### Link to tracking issue Fixes open-telemetry#37239 ## Testing & Refactoring - **Extended Tests**: Log body redaction added to all existing cases + new tests for detailed body structure validation. - **Code Cleanup**: Introduced helper methods for redaction, masking, allowing, and ignoring logic. - **Improved Readability**: Renamed some const variables for clarity and consistency. --------- Co-authored-by: Antoine Toulme <[email protected]>
Description
This update applies redaction rules to the
log.body
field by supporting multiple data types, including string, slices, maps, and nested maps. It applies key-based configurations consistently across all map structures.redaction.body.redacted.keys
/redaction.body.redacted.count
redaction.body.masked.keys
/redaction.body.masked.count
redaction.body.allowed.keys
/redaction.body.allowed.count
redaction.body.ignored.count
Link to tracking issue
Fixes #37239
Testing & Refactoring