tcp_proxy: add on-upstream-connected access log flush #26094
Conversation
Signed-off-by: Ohad Vano <ohadvano@gmail.com>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
Signed-off-by: Ohad Vano <ohadvano@gmail.com>
Signed-off-by: Ohad Vano <ohadvano@gmail.com>
Signed-off-by: Ohad Vano <ohadvano@gmail.com>
|
/retest |
|
Retrying Azure Pipelines: |
|
CC @htuch for another use case for access logging at the start of the stream. |
Signed-off-by: Ohad Vano <ohadvano@gmail.com>
Signed-off-by: Ohad Vano <ohadvano@gmail.com>
Signed-off-by: Ohad Vano <ohadvano@gmail.com>
|
I think this would be needed more generally for HTTP streams, but that can be done in a future PR. @kyessenov what is your take on the API? |
|
@htuch I thought I should handle this for HTTP case in a separate PR. I'll try get to it in next week but that's on my mind. |
|
API looks fine to me. Would be better to share the proto, but not that important. |
api/envoy/extensions/filters/network/tcp_proxy/v3/tcp_proxy.proto
Outdated
Show resolved
Hide resolved
| setup(1, config); | ||
| raiseEventUpstreamConnected(0); | ||
|
|
||
| EXPECT_EQ(access_log_data_, "127.0.0.1:80"); |
There was a problem hiding this comment.
I can't tell if this is a connection open or closed access log. Is this really the first log line?
There was a problem hiding this comment.
Yes, the fact that it is equal before calling on filter_.reset() makes sure that this was flushed on connection open. Other tests for example are checking this only after calling filter_.reset(). I also tried seeing what happens if I don't call "config.set_flush_access_log_on_connected(true);" and that check did throw.
Signed-off-by: Ohad Vano <ohadvano@gmail.com>
Signed-off-by: Ohad Vano <ohadvano@gmail.com>
Signed-off-by: Ohad Vano <ohadvano@gmail.com>
Signed-off-by: Ohad Vano <ohadvano@gmail.com>
Signed-off-by: Ohad Vano <ohadvano@gmail.com>
kyessenov
changed the title
Commit Message: Add on-upstream-connected access log flush for TCP Proxy.
Additional Description: None
Risk Level: Low
Testing: Unit test, integration tests
Docs Changes: Access logs
Release Notes: None
Platform Specific Features: None
Resolves #26058