tap: add HTTP request/response body tapping#5823
Conversation
1) Add request/response body tapping 2) Add buffered body limits (TBI for transport socket) 3) Add the JSON_BODY_AS_BYTES and JSON_BODY_AS_STRING output formats for convenience when the body is known to be human readable. 4) Add JSON output for the file per tap sink. Signed-off-by: Matt Klein <mklein@lyft.com>
|
Please merge master to pick up #5827. |
Signed-off-by: Matt Klein <mklein@lyft.com>
Signed-off-by: Matt Klein <mklein@lyft.com>
| repeated OutputSink sinks = 1 [(validate.rules).repeated = {min_items: 1, max_items: 1}]; | ||
|
|
||
| // [#comment:TODO(mattklein123): Output filtering. E.g., certain headers, truncated body, etc.] | ||
| // For buffered tapping, the maximum amount of received body that will be buffered prior to |
There was a problem hiding this comment.
There will be some stream/buffered selection later I assume?
There was a problem hiding this comment.
Yes, I'm going to start working on streaming once I get the basics in place.I still think that we always want to have buffering capabilities, mainly because it's easier for the end user to work with / look at in certain cases.
|
|
||
| void Utility::addBufferToProtoBytes(envoy::data::tap::v2alpha::Body& output_bytes, | ||
| uint32_t max_buffered_bytes, const Buffer::Instance& data) { | ||
| const uint64_t num_slices = data.getRawSlices(nullptr, 0); |
There was a problem hiding this comment.
Oh nice. Thanks. Forgot we had this. This will save me a bunch of time in my next PR.
There was a problem hiding this comment.
I was just looking at this, and I think if I do copyOut I'm going to need a third copy. I can move an std::string into the protobuf string, but I can't reserve() an std::string, copy data into it, and then set its size. Right? Is there any way to do this without another copy that I'm missing?
There was a problem hiding this comment.
You can do resize() the string and copyOut into &str[0].
There was a problem hiding this comment.
some example: https://github.com/envoyproxy/envoy/blob/master/source/extensions/filters/network/dubbo_proxy/hessian_utils.cc#L81
Perhaps we can also make copyOut take a std::string& ?
There was a problem hiding this comment.
This is still slower since
resize()will initialize the string data up to the length.
Agree, so I think we can make copyOut take a std::string&, as we see this pattern being used in a couple other places.
There was a problem hiding this comment.
That still wouldn't help unless we reimplement copyout() manually outside of the libevent API. cc @brian-pane for the new buffer implementation. I think for now I will just use copyOut() and leave a TODO for perf cleanup later.
There was a problem hiding this comment.
Yeah that's what I meant, in other words, toString with start and size.
There was a problem hiding this comment.
Yup agreed, will add a TODO and fix in a follow up.
There was a problem hiding this comment.
I just looked at this again and I realized that since I'm appending to the proto string across calls, this will require an extra copy no matter what we do to the buffer API, so I think I'm just going to leave it as is. I will think about what to do in my next change since this is going to get more complicated. I will add a TODO along these lines.
Signed-off-by: Matt Klein <mklein@lyft.com>
Signed-off-by: Matt Klein <mklein@lyft.com>
Signed-off-by: Matt Klein <mklein@lyft.com>
1) Add request/response body tapping 2) Add buffered body limits (TBI for transport socket) 3) Add the JSON_BODY_AS_BYTES and JSON_BODY_AS_STRING output formats for convenience when the body is known to be human readable. 4) Add JSON output for the file per tap sink. Signed-off-by: Matt Klein <mklein@lyft.com> Signed-off-by: Fred Douglas <fredlas@google.com>
3 participants
formats for convenience when the body is known to be human
readable.
Risk Level: Low
Testing: New tests
Docs Changes: Added
Release Notes: N/A