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

Make body filters aware if they are dealing with a request or response #1803

Closed
nhmarujo opened this issue Apr 11, 2024 · 8 comments
Closed

Comments

@nhmarujo
Copy link
Contributor

It would be great if we could have more context on body filters, like if we are dealing with a request or a response. That would allow us to take better decisions. For instance, imagine you have an API on which you wanna disable all response logging cause the payloads are too big, you could just check for a property. I realise this can also be done checking for a max-size but in some cases the flag would probably be more efficient. This is obviously just an example, there may be many more use cases.

Of course I do realise that just request/response awareness is not enough, because there is the potential to log bot server and client calls. For that origin awareness would also be useful.

Thanks

@kasmarian
Copy link
Member

I think it can be helpful to make the right filtering decision if it had more context. As you mentioned, at least to know if it's a request/response and what's the origin.

As per the example you mentioned, I understand it's just an example, but just to point out that this is already possible. You could use the ResponseFilter for cases when you want to omit logging the response body based on some conditions. That's what ResponseFilters.defaultValue() is using via BodyReplacers to avoid logging bodies with binary data.

@nhmarujo
Copy link
Contributor Author

Thank you for the prompt answer @kasmarian . Honestly I was not aware of that, but yes it was just an example. That said I still see value in having more context in filters so that people have more flexibility to make decisions 🙂

Would this be something that your team would consider adding?

Thank you

@kasmarian
Copy link
Member

We could have a look, yes, but I can't tell you any timeline promises. If you have time and desire, please feel free to suggest the changes, and we'll review them.

@nhmarujo
Copy link
Contributor Author

Thanks @kasmarian . Honestly I had no expectations time-wise. More than a matter of time and desire, I'm lacking deeper knowledge on how things are structured. But I will take a look and who knows, if I get some idea I will shared of course 😉 Have a nice weekend!

@nhmarujo
Copy link
Contributor Author

Was also thinking that it also be good to be aware of the status code, if you are dealing with a response.

@whiskeysierra
Copy link
Collaborator

A BodyFilter was meant to be just that, a filter for a message body. If you need more context or want to only apply something for requests but not responses, etc. then I'd argue you should use one of the lower-level abstractions like Request- or ResponseFilter.

Copy link
Contributor

In order to prioritize the support for Logbook, we would like to check whether the old issues are still relevant.
This issue has not been updated for over six months.

  • Please check if it is still relevant in latest version of the Logbook.
  • If so, please add a descriptive comment to keep the issue open.
  • Otherwise, the issue will automatically be closed after a week.

@github-actions github-actions bot added the stale label Oct 21, 2024
Copy link
Contributor

This issue has automatically been closed due to no activities. If the issue still exists in the latest version of the Logbook, please feel free to re-open it.

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

3 participants