Repository navigation
fix(dns): stop naming DNS resolvers after the next TLS host on a reused fd - #372
Merged
Merged
Conversation
…ed fd A UDP DNS socket has no close event, so the connection built from its tuple stayed tracked on its pid and fd after the socket closed. The next socket the application opened usually got the same fd: the connection to the address it had just resolved. That connection's TLS ClientHello took the stale entry and recorded the resolver's address under the TLS server name. A resolver with a non-private address, such as one in a cloud provider's service range, then went by whichever host had last been connected to. Every new name minted a new destination for each queried domain, in the counter and in the 12-bucket histogram, so DNS series grew with the number of hosts a container connected to. DNS queries now take their connection from their own socket tuple and are never tracked. A tracked entry whose destination is not the event's socket is treated as an earlier socket's, both by DNS queries and by the ClientHello path.
There was a problem hiding this comment.
Code Review
This pull request introduces changes to prevent stale connection tracking when file descriptors are reused, particularly for DNS over UDP and TLS ClientHello. It adds a dst field to ActiveConnection and introduces an isSocket helper to verify if a tracked connection matches the socket information of an L7 event. Comprehensive unit tests are also added to validate these scenarios. The review feedback suggests two key improvements: optimizing the isSocket check by comparing destination ports first to avoid expensive IP parsing on the hot path, and generalizing the stale connection check across all protocols to prevent incorrect reuse of connections.
RamanKharchee
approved these changes
Oct 8, 2026
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
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.
Summary
DNS series were labelled with the wrong destination. Queries to a resolver came out as
destination="<some TLS host>:53", one value for every host the container had recently connected to. Each new value created a new series for every domain, in both the counter and the histogram. On a busy node this pushed/metricspast a scraper's response size limit, and the scraper dropped that node's metrics entirely.This PR fixes the attribution, so each resolver has one destination again.
Engineering detail
Cause. eBPF passes UDP DNS through with the socket's tuple, and
createConnectionFromSocketInfostored a connection for it inconnectionsByPidFd. A UDP socket has no close event, so that entry outlived the socket. The application's next socket usually got the same fd number: the TCP connection to the address it had just resolved. L7 events are handled as they arrive, and connection events later. So the new connection's TLS ClientHello found the stale DNS entry, and the SNI path (which skips the timestamp checks) publishedip2fqdn[resolver IP] = <SNI host>. It also renamed the DNS entry's key to that host. This only matters whenIsIpExternal(resolver)is true, for example a cluster whose service CIDR lies in a public range.NewDestinationKeythen named every later query's destination after the resolver's latest "FQDN".The
--max-fqdns-per-containercap (#371) does not bound this, because the multiplier is the destination, not the domain.Fix.
ActiveConnection.dstrecords the socket's pre-NAT destination, from the open event or the tuple.isSocket(si)compares it with an L7 event's tuple, which is read from the fd when the event happens. It returns true when there is nothing to compare.connectionFromSocketInfo) when there is no tracked entry, or when the entry is a different socket. Those conns now live for one event; the cost is two BPF map lookups per query.Tests (
containers/socket_connection_test.go, which CI excludes, seeci.yml; run locally in a Linux container):TestDNSResolverKeepsItsNameAcrossReusedFd: a DNS query on fd 7, then a ClientHello on fd 7 to the resolved address, then another query. The resolver keeps its address and has no FQDN.TestEarlierSocketOnFdIsNotUsed: a stale entry on the fd, in each direction.TestConnectionIsSocket: IPv4-mapped, port mismatch, no tuple.Mutation check: disabling the DNS branch fails 2 tests, and disabling the ClientHello check fails 1.
Review follow-up (9d4e951):
isSocketcompares the port before it parses the address. Its result is unchanged for any parseable tuple. The e2e below ran on 86a77f8, before this commit; the unit tests cover the reordered check.CI-equivalent checks. In a Linux container with Go 1.26.5: gofmt, goimports,
go vet ./..., golangci-lint v2.13.2 (0 issues) andgo test ./...(including/containers) all pass.Local e2e: in Docker Desktop (kernel 6.10, arm64), I built this branch and
mainand ran each twice as the agent. The client was a Python container started with--dns 8.8.8.8. Three times over, it resolved 5 public HTTPS hosts and opened a TLS connection to each.main:ip_to_fqdn{ip="8.8.8.8"}was a TLS host (www.cloudflare.com), and the client's DNS series had 5–6 destinations (api.github.com:53,example.com:53, …) on both runs. The published 0.1.9 image behaves the same.ip_to_fqdnentry for 8.8.8.8, and every DNS series hasdestination="8.8.8.8:53". That is 5 series, one per domain, on both runs.