Skip to content

ENG-17140 Ensure requests can be filtered because of method body #364

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

Merged
merged 2 commits into from
Jun 9, 2022

Conversation

bradAtTraceable
Copy link
Contributor

@bradAtTraceable bradAtTraceable commented Jun 8, 2022

Description

Please include a summary of the change, motivation and context.

Testing

Please describe the tests that you ran to verify your changes. Please summarize what did you test and what needs to be tested e.g. deployed and tested helm chart locally.

Checklist:

  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Any dependent changes have been merged and published in downstream modules

Documentation

Make sure that you have documented corresponding changes in this repository or hypertrace docs repo if required.

@bradAtTraceable
Copy link
Contributor Author

The purpose of this test is to ensure that the Filter receives the contents of an HTTP request body, so that it can block the request, if required. Previously, the request body was only being passed to the filter if the reader of the request body received an indication that the end-of-stream was reached. However, in some cases, the reader would stop reading the request body without seeing the normal end-of-stream indication, and the filter would not be called.

@shashank11p
Copy link
Contributor

@bradAtTraceable Some smoke tests are failing. Please update the tests as well :)

@bradAtTraceable bradAtTraceable merged commit 3b74567 into main Jun 9, 2022
@bradAtTraceable bradAtTraceable deleted the ENG-17140 branch June 9, 2022 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants