udp_proxy: populate upstream host and address info for access logs#45583
Open
IssaAbuKalbein wants to merge 1 commit into
Open
udp_proxy: populate upstream host and address info for access logs#45583IssaAbuKalbein wants to merge 1 commit into
IssaAbuKalbein wants to merge 1 commit into
Conversation
IssaAbuKalbein
force-pushed
the
udp-tunneling-attempted-hosts
branch
2 times, most recently
from
b41cb0e to
bda303b
Compare
The UDP proxy did not populate all upstream information on the session StreamInfo that access loggers expect, so several %UPSTREAM_*% access log formatters were always empty for UDP sessions, unlike TCP proxy. This change fixes two distinct gaps: 1. Non-tunneling UDP -> UDP path (envoyproxy#44153): the upstream local and remote addresses were never set, so %UPSTREAM_LOCAL_ADDRESS% and %UPSTREAM_REMOTE_ADDRESS% (and their _WITHOUT_PORT / port variants) were always empty. The remote address is now recorded when the upstream host is selected in createUpstream(), and the local address once the socket is bound after the first datagram is sent upstream in writeUpstream(). 2. Tunneling (UDP-over-HTTP) path: the attempted-upstream-host tracking added in envoyproxy#43215 (HTTP router, TCP proxy and the non-tunneling UDP path) was not wired into TunnelingConnectionPoolImpl, so %UPSTREAM_HOSTS_ATTEMPTED% / %UPSTREAM_HOST_NAMES_ATTEMPTED_WITHOUT_PORT% were always empty for tunneled sessions even though %UPSTREAM_HOST% was populated. addUpstreamHostAttempted() is now called alongside setUpstreamHost() on both pool ready and pool failure. Adds unit test coverage for both paths in udp_proxy_filter_test. Fixes envoyproxy#44153. Follow-up to envoyproxy#43215. Signed-off-by: Issa Abu Kalbein <iabukalbein@microsoft.com>
IssaAbuKalbein
force-pushed
the
udp-tunneling-attempted-hosts
branch
from
bda303b to
3bfbad7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Commit Message: udp_proxy: populate upstream host and address info for access logs
Additional Description:
The UDP proxy did not populate all upstream information on the session
StreamInfothat access loggers expect, so several%UPSTREAM_*%access log formatters were always empty for UDP sessions, unlike TCP proxy. This PR fixes two distinct gaps:Non-tunneling UDP → UDP path (UDP listeners not logging upstream information #44153): the upstream local and remote addresses were never set, so
%UPSTREAM_LOCAL_ADDRESS%and%UPSTREAM_REMOTE_ADDRESS%(and their_WITHOUT_PORT/ port variants) were always empty. The remote address is now recorded when the upstream host is selected increateUpstream(), and the local address once the socket is bound after the first datagram is sent upstream inwriteUpstream().Tunneling (UDP-over-HTTP) path: the attempted-upstream-host tracking added in formatter: add access log formatters for tracking upstream hosts and connection IDs attempted #43215 (HTTP router, TCP proxy and the non-tunneling UDP path) was not wired into
TunnelingConnectionPoolImpl, so%UPSTREAM_HOSTS_ATTEMPTED%/%UPSTREAM_HOST_NAMES_ATTEMPTED_WITHOUT_PORT%were always empty for tunneled sessions even though%UPSTREAM_HOST%was populated.addUpstreamHostAttempted()is now called alongsidesetUpstreamHost()on both pool ready and pool failure.Risk Level: Low (only adds population of existing StreamInfo upstream fields; no behavior change to the data path).
Testing: Added unit tests in
udp_proxy_filter_testcovering both paths (UdpProxyFilterTest.UpstreamAddressAccessLog, andupstreamHostsAttempted()assertions inTunnelingConnectionPoolImplTest.PoolReady/PoolFailure).Docs Changes: N/A
Release Notes: Added (two
changelogs/current/bug_fixesentries).Fixes #44153