Repository navigation
fix: log each agent message once, and skip untracked sockets before the map lookup - #358
Merged
Merged
Conversation
TLS capture could fail at attach time, in the kernel, or in the agent with
nothing to show for it at default verbosity. This adds counters for each
stage and fixes the losses they revealed.
Visibility:
- node_agent_tls_attach_total{lib,result} for every attach attempt, and
one log line per binary and outcome instead of one per process.
- node_agent_tls_plaintext_dropped_total{reason}: TLS plaintext a library
hook saw but could not attribute to a socket.
- node_agent_l7_ringbuf_drops_total: L7 events lost to a full ring buffer.
- node_agent_l7_events_dropped_total{reason,protocol,tls}: events dropped
in the agent before protocol parsing.
Fixes:
- Register a process on its first socket event, so its first connection
is tracked and its TLS probes attach before its first request.
- Give a connection built from an event's socket tuple the event's
timestamp, and let a newer event replace the closed predecessor on a
reused fd. Both cases used to be dropped as stale.
- Key the cache of binaries that cannot be probed by file identity, not
by path, which is only meaningful inside one container.
- Re-attach after a process execs a different binary.
- Only a write that starts with a TLS record may claim pending OpenSSL
plaintext, and that write's ciphertext now counts toward bytes sent.
- Guard Process state shared between the instrumentation goroutine, the
event loop and the scrape path.
Collect read c.pythonStats and c.nodejsStats without c.lock while the registry's event loop updates them under it.
… tuples A connection created from an L7 event's socket tuple, because the event arrived before the connection's open event, skipped the filters and the NAT resolution onConnectionOpen applies. With these connections now kept (previous commit), that tracked traffic to ignored destinations and labelled requests with a service's virtual IP as their actual destination. - Both paths now share connectionKey: the same port, loopback, ignored workload and connection filters, and the same destination key. - The socket-tuple path takes the actual destination from the kernel's conntrack-derived actual_destinations map, as open events do. - An exec is re-checked on every new socket, not only when a throttle shared with the periodic sweep allows it. - The go_fd_unknown help text says it includes TLS over in-memory connections, which have no socket.
… wrapped Go connections (#354) * feat(tls): attribute TLS plaintext losses to the container and binary node_agent_tls_plaintext_dropped_total says how much TLS plaintext the kernel could not attribute to a socket, but not which process it came from, so a loss could not be traced to a workload without a debug build. - The TLS library uprobes also count losses per process and reason in an LRU map. The syscall programs are left alone. - The agent reads and clears those counts every stats tick, and drains a process's entries when it exits, since a short-lived process is gone before the next tick. - container_tls_plaintext_dropped_total{reason} counts them per container. - The first loss per binary and reason is logged once as a warning, naming the executable and what the reason usually means. * fix(ebpf): find the socket behind wrapped Go connections The Go TLS probe found a connection's socket only if crypto/tls ran directly over *net.TCPConn, or over a wrapper embedding the connection at offset 0, one level deep. Proxies and scrapers wrap deeper. One reverse proxy's TLS connections are three wrappers deep, each embedding the next at offset 0. A scraper's wrapper puts an int32 counter before the embedded net.Conn, at offset 8. All of their TLS plaintext was dropped. The probe now descends up to four levels, taking the embedded net.Conn at offset 0 or 8. A level is accepted when its type is the binary's *net.TCPConn itab. Otherwise, as in stripped binaries, or PIE binaries whose symbol table holds the itab unrelocated, the fd it yields is accepted only if it names an IPv4 or IPv6 socket of the process. gRPC's syscallConn has the same shape and goes through the same walk. TLS over in-memory connections still has no socket and is still counted as go_fd_unknown. * fix(tls): log drops of unknown processes once per container, not once per node An unknown process has no executable identity, so its log key was the zero value and the first such warning on a node silenced the rest. Also name the per-pid drop map key type.
…es it HTTP/2 parsers are keyed by pid+fd alone. When a recycled fd got a new connection, either from its open event or from an L7 event that arrived first, the new connection inherited the previous one's parser and its HPACK dynamic table. Its headers then failed to decode, or decoded wrong. The parser is now dropped when the fd's connection is replaced. A parser already tagged with the new connection's timestamp is kept: it was created for this connection by an L7 event that beat the open event.
A level of the walk that was not a *net.TCPConn was still read as one. For a
wrapper struct the first word is an itab pointer, so the "Sysfd" read was the
itab's type hash, and when a value read that way named another live socket of
the process, the socket check accepted it: the session's plaintext went to
that connection, and it was marked TLS, every time, for that binary. On
kernels without BTF the socket check was skipped, so all four levels were
trusted unchecked.
A level found without the itab must now point to a net.netFD whose family is
AF_INET or AF_INET6 and whose sotype is SOCK_STREAM. The check reads only the
application's memory, so it holds without kernel BTF. Where the kernel's
struct offsets are known, the fd must also be a SOCK_STREAM socket of that
family. The netFD offsets come from DWARF and default to 56 and 64, which
Go 1.17 through 1.26 share. The walk no longer reads the gRPC syscallConn
itab, so its discovery is gone.
New metrics make the kernel-dependent part visible:
- node_agent_go_tls_fd_resolved_total{method,depth}: method is itab, socket
(netFD check plus kernel check) or shape (netFD check only, no BTF)
- node_agent_ebpf_info{program_variant,btf,socket_offsets}
- node_agent_ebpf_program_instructions and
node_agent_ebpf_program_verified_instructions, per program
TestProgramsLoad loads the variant the running kernel gets and logs each
program's size and verifier cost. TestGoTLSFdWalk runs TLS over a plain, a
wrapped and a decoy connection and checks which socket each one is
attributed to. Both need root and VM=1.
…onnection takes its fd A parser without a connection timestamp was created by events from a socket the kernel was not tracking, such as a connection older than the agent. A connection opening on its fd is tracked, and the kernel stamps every event of a tracked connection with its timestamp, so the parser cannot be the new connection's. It was kept, and the new connection inherited its HPACK table.
…dropped (#355) * feat(tls): attach Go TLS probes at exec, and log where L7 events are dropped A process's Go TLS probes were attached on its first connection, after its connect event had been read. That was late for short-lived programs: many exited first, and the rest had made their first requests unprobed. A program that a wrapper exec'd kept probes for the wrapper's binary until a throttled re-check noticed the change. - A sched_process_exec tracepoint reports execs. The agent attaches Go TLS probes for the new image at once, and drops those of the old one. OpenSSL is still attached on the first connection, because the loader maps libssl after the exec. - Process events are read every 10 ms, like connect events, instead of every 100 ms. - The periodic executable re-check stays as a fallback for lost exec events, at a 10 s interval. - Every L7 event dropped before parsing is logged once per container, reason and protocol, with the pid, fd and destination, so the counter's reasons can be traced to a workload. * fix(containers): report L7 events on non-IP sockets as no_ip_socket Events on sockets without an IP tuple, such as gRPC over a Unix socket, were counted as unknown_connection, which reads as a capture loss. The agent does not track those sockets, so they now get their own reason. * chore(ebpf): regenerate ebpf.go after rebasing onto the review fixes
…he map lookup klog.SetOutput hands every severity the same writer, and klog writes a message to its own severity's writer and to every lower one's, so each warning reached the log twice and each error three times. one_output writes it once. createConnectionFromSocketInfo parsed the socket tuple and looked up the kernel's NAT map before applying the connection filters. A TLS server's accepted sockets have a client's ephemeral port as their destination, are never tracked, and came through this path on every read and write the server made. The port filter is now checked first.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces performance and logging improvements. In containers/container.go, a port filtering check is added early in createConnectionFromSocketInfo to skip parsing and kernel map lookups for skipped ports, optimizing performance for common cases like TLS server connections. In main.go, the one_output flag is set to true for klog to prevent duplicate log messages when a single writer is used for all log severities. I have no feedback to provide.
main's tree equals the #353 branch this was built on, so this keeps the branch's tree and only records the merge.
blue4209211
approved these changes
Oct 6, 2026
RamanKharchee
approved these changes
Oct 6, 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
3 participants
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
Two small fixes found while profiling the agent under TLS load:
klog.SetOutputgives every severity the same writer, and klog writes a message to its own severity's writer and to every lower one's. So each warning was logged twice and each error three times. klog'sone_outputflag writes it once.createConnectionFromSocketInfoparsed the socket tuple and looked up the kernel's NAT map before applying the connection filters. A TLS server's accepted sockets have a client's ephemeral port as their destination, are never tracked, and came through this path on every read and write the server made. The port filter now runs first;connectionKeystill applies it, so nothing else changes.Engineering detail
Local e2e: built agent images from this branch and its parent and ran each in a local Docker VM (kernel 6.10) behind a Go HTTPS/2 server serving 8 parallel 64 MB downloads, with a 20s CPU profile mid-load. Parent: 14 warning lines, 7 distinct (every warning twice);
createConnectionFromSocketInfo8.8% of agent CPU, 7.8% of it in the NAT map lookup. This branch: every warning line distinct, andcreateConnectionFromSocketInfoabsent from the profile. golangci-lint and thecontainerstests pass.