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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update: As per #2006 Empty body being redacted is confusing #2225

Merged

Conversation

redcinelli
Copy link
Contributor

As per #2006

When the body is empty, It feels surprising to set it to redacted.
Even more so when we have the CaptureBody: true.

When the body is empty, I set it to an empty string
Copy link

👋 @redcinelli Thanks a lot for your contribution!

It may take some time before we review a PR, so even if you don’t see activity for some time, it does not mean that we have forgotten about it.

Every once in a while we go through a process of prioritization, after which we are focussing on the tasks that were planned for the upcoming milestone. The prioritization status is typically reflected through the PR labels. It could be pending triage, a candidate for a future milestone, or have a target milestone set to it.

@Mpdreamz Mpdreamz marked this pull request as ready for review June 4, 2024 07:02
@Mpdreamz
Copy link
Member

Mpdreamz commented Jun 4, 2024

Thanks for tackling this @redcinelli and apologies for taking forever to get around to it.

I've amended the PR slightly to forcibly set the body to redacted if the config is set to off and made sure the default is not an empty string because we attempt to read the body multiple times at various stages in the pipeline.

LGTM but my colleague @stevejgordon will need to review now that I also added code 😸

Copy link
Contributor

@stevejgordon stevejgordon left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@Mpdreamz Mpdreamz merged commit 5542b34 into elastic:main Jun 4, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants