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

Response should be mapped to it's original Request in some form, Response path should be filterable #1067

Closed
SpiReCZ opened this issue Jun 16, 2021 · 6 comments

Comments

@SpiReCZ
Copy link
Contributor

SpiReCZ commented Jun 16, 2021

I have got stuck on problem where we needed to create ResponseFilter that would filter Response with JsonPathBodyFilters based on Request path. But currently there is no right way (without rewriting the framework) to tell if the Response is made to a particular Request. Which means that Response and Request are not connected at filtering phase.

We have temporarily solved this by injecting a custom HTTP header with the Request path to the Response in Strategy implementation class.

Context

We would use proper path filtering of both Request and Response for better performance of the new JsonPathBodyFilters that would be applied only to some specific requests that have required json body structure.
It would benefit others by allowing filtering of Responses by path, other Request data or allow creation of more complex filters.

Possible Implementation

It would be best to include Request path to the Response automatically either directly or by linking a cloned and unfiltered instance of Request without body and headers. This would allow filtering Responses by path (currently not supported).

Your Environment

Our environment uses logbook for HTTP logging requests and responses with bodies. We need JSON body filtering feature for anonymization of some objects and replacing multiple base64 binary content inside JSON payloads.

  • Version used: 2.9.0
  • Link to your project: N/A
@whiskeysierra
Copy link
Collaborator

We would use proper path filtering of both Request and Response for better performance of the new JsonPathBodyFilters that would be applied only to some specific requests that have required json body structure.

Is this a workaround for the bug with the JsonPathBodyFilters which occurs whenever a path in the JSON body is not found?

@SpiReCZ
Copy link
Contributor Author

SpiReCZ commented Jun 17, 2021

We would use proper path filtering of both Request and Response for better performance of the new JsonPathBodyFilters that would be applied only to some specific requests that have required json body structure.

Is this a workaround for the bug with the JsonPathBodyFilters which occurs whenever a path in the JSON body is not found?

No, this just solves for us the indirect performance issues that are present when JsonPathBodyFilters are applied to every Request/Response made. And it also enables path filtering Response.

@whiskeysierra
Copy link
Collaborator

indirect performance issues that are present when JsonPathBodyFilters are applied to every Request/Response made

Can you share your experience with this? Did you measure/benchmark it?

@SpiReCZ
Copy link
Contributor Author

SpiReCZ commented Jun 17, 2021

I have not benchmark it. But i just know that trying to process each and every JSON body there is will create additional strain where it shouldn't be. Processing JSON data is not that easy as it might seem from the code perspective.

JsonPathBodyFilters contains check for contentType which is great and that only makes it problem for a JSON bodies.


Let's say that i have an application that does have 25 unique API endpoints and 25 rest client endpoint mappings and are also JSON contentType.
That gives us 50 different Request/Responses that can be made with different JSON body structure.

I start using JsonPathBodyFilters and create a 5x merged BodyFilter (for 2x Requests, 3x Responses). Each BodyFilter have 3 JSON filters in average. That gives us 15 filters total we want to apply.

Now each time a Request/Response is being processed (filtered) i have to apply all the 15 filters.

If each API gets called atleast 2 times within the hour, that gives us 2x2x50=200 Http messages that has to be processed for an hour of runtime.

What you would normally want is to scrub specific Request/Response of sensitive data, but the BodyFilter is global (which is why it is not currently suitable for JsonPath filtering).

200 Http messages are now to be filtered with my 15 filters -> 3000 times the DocumentContext is to be checked atleast whether the JSON path actually exists and silently fail 2985 times.


We are currently filtering Requests and Responses by Request path. Each Http message is checked whether that Request path should have BodyFilter applied (JsonPathBodyFilters) or not.

So for 200 Http messages we check 200 times if filters should be applied and the DocumentContext gets checked 15 times total (We also differentiate between Request and Response on that Request path).

@whiskeysierra
Copy link
Collaborator

I have not benchmark it.

We have a JMH benchmark suite, so the infrastructure is there.
I'd prefer to see real numbers, so that we can make an objective decision.

200 Http messages are now to be filtered with my 15 filters -> 3000 times the DocumentContext is to be checked atleast whether the JSON path actually exists and silently fail 2985 times.

Absolutely correct, but it really depends on the overhead that "checking whether a JSON exists" actually incurs.

@no-response
Copy link

no-response bot commented Sep 10, 2021

This issue has been automatically closed because there has been no response to our request for more information from the original author. With only the information that is currently in the issue, we don't have enough information to take action. Please reach out if you have or find the answers we need so that we can investigate further.

@no-response no-response bot closed this as completed Sep 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants