fix: IP-to-FQDN resolution, OpenSSL 3.x TLS capture, and log cleanup#200
Conversation
DestinationKey is set at TCP connection time, but DNS may not be cached yet due to per-CPU perf buffer race conditions. This causes all metric labels (destination, destination_workload_name) to show raw IPs instead of FQDNs for external services. Add enrichDestinationKey() that checks the DNS cache at metric emission time and substitutes the FQDN when available. Applied to all TCP metrics (bytes, connections, retransmits) and all L7 protocol metrics (HTTP, Postgres, Redis, etc).
…metrics Multiple IPs resolving to the same FQDN (e.g., Google's shared IPs for monitoring.googleapis.com) caused duplicate metric errors when enriched to the same label set. Aggregate stats by enriched DestinationKey before emitting TCP metrics.
Root cause fix for duplicate metric errors. When a TCP connection opens before DNS is cached (per-CPU perf buffer race), the DestinationKey stores the raw IP. Later connections get an FQDN-based key. Both enrich to the same FQDN at emit time, causing "collected before with same label values" errors. migrateConnectionKeyIfNeeded updates conn.DestinationKey in-place and migrates connectionStats from the old IP-based key to the FQDN-based key. Called from updateConnectionTrafficStats and onL7RequestWithResult. Collect() aggregation kept as safety net for stale entries.
Replace full K8s API objects with minimal structs (MinimalOwnerInfo, MinimalService, MinimalNode) to reduce memory ~10x per resource. Add PodNameIndex for O(1) ResolvePodOwner instead of O(pods) scan. Deduplicate service resolution into resolveServiceWorkload(). Simplify getControllerOfOwner from 7 switch cases to unified lookup.
Remove 3 unused functions (ParseHTTPResponse, ParseHttpResponse, ParseHostFromHttpRequest). Fix parseRequest() to call ParseHTTPRequest once instead of both ParseHttp + ParseHTTPRequest. Pass pre-parsed method/path to observe() instead of re-parsing payload a third time. Fix pre-existing RawPath bug in ParseHTTPRequest URL construction.
…unds checks - Fix concurrent map access in getMounts/getListens/ping (copy c.processes under lock) - Fix conn.Closed written outside lock in onConnectionClose - Add missing CronJob informer handler for owner chain resolution - Fix InstanceMeta missing Instance field in initial snapshot - Fix storeWorkloadsIP check-then-act race with mutex - Clean stale ClusterIPs on Service update and old pod IPs on Pod update - Add bounds checks in memcached/redis eBPF parsers to prevent OOB reads - Remove misleading defer req.Body.Close() in ParseHTTPRequest
In OpenSSL 3.x, the SSL struct was split into ssl_st (base) and ssl_connection_st (connection data). rbio/wbio moved from offset 16/24 to 80/88. The eBPF code was reading garbage pointers for the BIO, causing FD extraction to fail (CONN_NOT_FOUND with invalid FDs like 2868022864). HTTP payload was captured correctly since it comes from function parameters, but the connection couldn't be matched. Adds GET_FD_V3 macro that reads rbio/wbio at the correct offsets for OpenSSL 3.x (verified against OpenSSL 3.5.4 with a test program).
Processes like the notification server load both psycopg2's bundled libssl-*.so and the system /usr/lib/libssl.so.3. The agent was picking the first match from /proc/pid/maps (psycopg2's), but Python's ssl module uses the system lib for HTTPS. This caused SSL uprobes to fire on the wrong library, missing Slack/Teams API calls.
Per-event hot-path logs (L7_EVENT_REGISTRY, CONTAINER_FOUND, TIMESTAMP_MISMATCH) were at V(2), producing ~15K lines/min with KLOG_V=2. Raised to V(5). Per-request logs (HTTP2_COMPLETED, LLM_TRACK) raised to V(4). Occasional logs raised to V(3). TLS per-process debug logs moved from unconditional Infof to V(2)/V(3).
Summary of ChangesHello @mayankpande88, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the agent's network observability capabilities by implementing robust IP-to-FQDN resolution for external destinations in metrics and enabling TLS traffic capture for OpenSSL 3.x. It also includes a substantial rewrite of the Kubernetes IP resolver for improved efficiency and accuracy. Furthermore, the PR addresses several stability and performance concerns through race condition fixes, HTTP parsing deduplication, and a comprehensive reduction in log verbosity for hot-path events. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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 significant refactoring and improvements to the Kubernetes IP resolver and L7 traffic monitoring components. Key changes include optimizing memory usage in the IP resolver by introducing minimal structs for Kubernetes resources (like MinimalOwnerInfo, MinimalService, MinimalNode) and updating the clusterSnapshot to use these. It also enhances the IP resolver's functionality by adding a PodNameIndex for faster lookups, incorporating CronJob watching, and improving service IP handling with a new mutex for thread safety. In L7 traffic monitoring, the changes address duplicate metrics by aggregating connection statistics based on an 'enriched' destination key that resolves IP addresses to FQDNs using a DNS cache, and by migrating connection keys from IP to FQDN when DNS information becomes available. The LLM (Large Language Model) tracking mechanism has been streamlined by removing internal aggregation maps and directly emitting metrics, along with adjustments to logging verbosity across several components. Additionally, the HTTP request parsing logic was made more robust, and thread safety was improved in various Container and L7Stats methods by adding mutexes and read locks to prevent concurrent map access issues. The eBPF openssl probes were updated to support OpenSSL 3.x by correctly identifying rbio/wbio offsets, and minor fixes were applied to memcached and redis eBPF response parsing for improved robustness. Finally, the logic for detecting SSL libraries in processes was refined to prefer system libraries over bundled ones.
- Rename L7 labels to match TCP: destination_region → destination_workload_region, destination_az → destination_workload_az, destination_instance → actual_destination_instance - Add missing actual_destination_region and actual_destination_az labels to L7 metrics - Fix L7 destination_workload_region/az to use destinationWorkload (was using actualDestWorkload) - Remove dead fields from ConnectionKey (srcWorkload, dstWorkload, actualDestWorkload — never set) - Remove dead fields from ActiveConnection (dstWorkload, actualDestWorkload — set but never read) - Remove redundant DNS re-enrichment in trace creation (migrateConnectionKeyIfNeeded already handles it)
2 participants
Summary
Test plan