fix: reduce memory usage, fix race conditions, and prevent corrupted eBPF spans by mayankpande88 · Pull Request #206 · nudgebee/node-agent

mayankpande88 · 2026-03-07T09:06:40Z

Summary

Memory optimizations: Auto-tune GOMEMLIMIT from cgroup limit, reduce LRU cache/buffer sizes, cache stripped Go binary paths, clean up HTTP/2 HPACK parsers in GC
Race condition fixes: Protect connectionStats, logSamples, restarts/oomKills, getListens(), and processes map accesses in Collect() and onConnectionOpen() with proper locking
FD leak fixes: Close file handles on error in tail_reader.go, cap partial line buffer at 64KB
Corrupted timestamp fix: eBPF probes occasionally produce uint64 durations with high bit set, which become negative when cast to int64. These create spans where end_time < start_time, causing ClickHouse batch rejections, OTel collector retry accumulation, and OOMKill (165 restarts). Fixed with safeDuration() validation at parse time and defense-in-depth checks in span creation.

Test plan

Deploy to staging cluster and verify node-agent pods stay under memory limit (1Gi)
Verify OTel collector stops restarting (no more corrupted spans)
Check ClickHouse for absence of negative-duration spans
Verify Prometheus metrics still emit correctly (race condition fixes don't cause deadlocks)
Monitor for any regression in L7/HTTP2/DNS metric collection

…alls

The previous version had a corrupted sensitive_patterns.json that caused "cannot unmarshal object into Go value of type []SensitivePattern" errors, breaking sensitive data detection in log parsing.

Memory optimizations: - Auto-tune GOMEMLIMIT to 90% of cgroup memory limit - Reduce connectionStats LRU cache from 8192 to 4096 - Reduce event channel buffer from 10000 to 2000 - Cap ip2fqdn map at 10000 entries - Reduce label interner max from 10000 to 5000 - Reduce LLM stream limits (maxActiveStreams 1000->200, buffer 1MB->64KB) - Cache stripped Go binary paths to avoid repeated uprobe attach attempts - Reduce GC interval from 10min to 5min - Clean up HTTP/2 HPACK parsers for dead connections in gc() Race condition fixes in Container.Collect(): - Snapshot connectionStats LRU under RLock (not concurrent-safe) - Call getListens/getProxiedListens under RLock - Protect logSamples map reads/writes with lock - Read restarts/oomKills under RLock - Read c.processes under RLock in onConnectionOpen before lock scope File descriptor leak fixes in logs/tail_reader.go: - Close file on Stat/Seek failure in NewTailReader - Close old file before setting nil in moved() - Cap partial line buffer at 64KB to prevent unbounded growth - Fix prefix accumulation bug (was replacing instead of appending)

eBPF kernel probes occasionally produce garbage duration values where the uint64 has the high bit set. Casting to time.Duration (int64) produces negative durations, creating spans where end_time < start_time. These poison spans cause ClickHouse batch rejections, OTel collector retry accumulation, and eventual OOMKill (165 restarts observed). Source-level fix (eBPF event parsing): - Add safeDuration() that returns 0 for values >= 1 hour or == 0 - Replace all uint64->time.Duration casts in perf/ringbuf readers HTTP/2 parser fix: - Add safeKernelDuration() for kernel timestamp subtraction - Prevents unsigned underflow when events arrive out of order Defense-in-depth (span creation): - createSpan: drop spans with duration <= 0 or > 1 hour - LLMRequest: drop spans with zero/invalid timestamps or > 1 hour

gemini-code-assist · 2026-03-07T09:07:05Z

Summary of Changes

Hello, I'm Gemini Code Assist¹! 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 stability and efficiency of the system by addressing critical memory consumption issues, resolving race conditions in concurrent data access, and preventing data corruption originating from eBPF probes. The changes aim to reduce the agent's memory footprint, improve its resilience against OOMKills, and ensure the integrity of collected telemetry data, particularly for L7 and connection tracing.

Highlights

Memory Optimizations: Implemented auto-tuning of GOMEMLIMIT based on cgroup limits, reduced LRU cache and buffer sizes across various components, cached stripped Go binary paths to avoid redundant ELF scanning, and added cleanup for HTTP/2 HPACK parsers during garbage collection.
Race Condition Fixes: Addressed several race conditions by protecting concurrent accesses to shared data structures such as connectionStats, logSamples, restarts/oomKills counters, getListens(), and the processes map with appropriate locking mechanisms.
File Descriptor Leak Fixes: Ensured file handles are properly closed on error paths in tail_reader.go and capped the partial line buffer at 64KB to prevent excessive memory consumption.
Corrupted eBPF Span Prevention: Introduced validation for eBPF-derived durations to prevent the creation of corrupted spans where end_time < start_time, which previously led to ClickHouse rejections, OTel collector restarts, and OOMKills.

🧠 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

common/log_parser_test.go
- Updated NewParser instantiation to include a defaultPatternsPerLevel argument.
- Modified an example AWS access key in a test log entry.
containers/container.go
- Imported the path/filepath package.
- Reduced the gcInterval from 10 minutes to 5 minutes.
- Decreased connectionStatsCacheSize from 8192 to 4096.
- Added read-locks around restarts and oomKills access in the Collect method.
- Implemented read-locks and snapshotting for getListens(), getProxiedListens(), and connectionStats within Collect() to ensure thread safety.
- Introduced read and write locks for logSamples access in Collect().
- Added a read-lock for c.processes access in onConnectionOpen().
- Updated NewParser calls to pass flags.LogPatternsPerContainer.
- Added logic to clean up HTTP/2 parsers for closed or dead connections during garbage collection.
- Refined log path resolution in resolveFd by using filepath.Clean and stricter prefix checks.
containers/l7.go
- Reduced the stringInterner cache size from 10000 to 5000 entries.
- Adjusted the cache clearing threshold from 5000 to 2000 entries.
containers/llm_stream.go
- Decreased maxActiveStreams from 1000 to 200.
- Reduced maxBufferPerStream from 1MB to 64KB.
containers/registry.go
- Reduced the events channel buffer size from 10000 to 2000.
- Introduced maxIP2FQDNEntries constant to limit the size of the ip2fqdn map.
- Added a check to prevent unbounded growth of the ip2fqdn map.
- Reduced maxPendingEvents for L7 event retries from 1000 to 500.
ebpftracer/l7/http2.go
- Added a safeKernelDuration helper function to validate eBPF kernel timestamps and prevent negative or excessively long durations.
- Reduced http2DecoderGcInterval from 10 minutes to 2 minutes.
- Applied safeKernelDuration when calculating r.Duration for HTTP/2 requests.
ebpftracer/tls.go
- Imported the sync package.
- Introduced strippedGoExeCache (a sync.Map) to cache paths of Go binaries known to be stripped of TLS symbols, avoiding repeated ELF scanning.
- Adjusted logging verbosity for Go TLS attachment attempts.
- Implemented logic to check and populate strippedGoExeCache to optimize TLS uprobe attachment for Go applications.
ebpftracer/tracer.go
- Added a safeDuration helper function to validate eBPF-provided nanosecond durations, returning 0 for corrupted or excessive values.
- Applied safeDuration when parsing Duration for L7 events and connection events from eBPF data.
go.mod
- Updated the github.com/nudgebee/logparser module to a newer version.
- Added a new dependency: github.com/jaeyo/go-drain3.
go.sum
- Updated checksums for github.com/nudgebee/logparser.
- Added checksums for the new dependency github.com/jaeyo/go-drain3.
logs/tail_reader.go
- Ensured file handles are closed on error paths in NewTailReader and moved methods.
- Introduced maxPrefixLen (64KB) to cap the buffer used for partial log lines, preventing memory bloat.
main.go
- Imported the fmt package.
- Added logic to automatically set GOMEMLIMIT to 90% of the cgroup memory limit if not already set, improving Go runtime memory management.
- Implemented readCgroupMemoryLimit function to detect and parse memory limits from cgroup v1 and v2.
tracing/tracing.go
- Added validation to createSpan to discard spans with non-positive or excessively long durations (over 1 hour).
- Introduced timestamp and duration validation in LLMRequest to ensure valid LLM stream information before creating spans.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a significant set of improvements, focusing on memory optimization, fixing race conditions and file descriptor leaks, and handling corrupted eBPF timestamps. The changes are comprehensive and well-targeted.

The memory optimizations, such as auto-tuning GOMEMLIMIT, reducing cache sizes, and caching stripped Go binary paths, are well-implemented and should significantly reduce the agent's memory footprint. The race condition fixes using read-write locks are crucial for stability, and the snapshotting pattern used in Collect() is a good approach to minimize lock contention. The file descriptor leak fixes in tail_reader.go are also correctly implemented.

I have two main points of feedback:

A potential race condition in the locking logic for logSamples in containers/container.go.
A suggestion to improve the robustness of integer parsing in the new readCgroupMemoryLimit function in main.go.

Overall, this is a great set of changes that will improve the agent's performance and reliability. Please see the detailed comments for my suggestions.

- Combine connectionStats RLock snapshotting with enrichedKey aggregation - Keep stripped binary cache check and add debug logging from main

- Use double-checked locking for logSamples to avoid redundant writes - Use strconv.ParseInt instead of fmt.Sscanf for stricter cgroup parsing

* fix: reduce memory usage, fix race conditions, and prevent corrupted eBPF spans (#206) * fix: add missing patternsPerLevelLimit param to logparser.NewParser calls * fix: update logparser dep, sanitize log paths, replace test credential * fix: update logparser dep to fix sensitive pattern loading The previous version had a corrupted sensitive_patterns.json that caused "cannot unmarshal object into Go value of type []SensitivePattern" errors, breaking sensitive data detection in log parsing. * fix: reduce memory usage, fix race conditions, and fd leaks Memory optimizations: - Auto-tune GOMEMLIMIT to 90% of cgroup memory limit - Reduce connectionStats LRU cache from 8192 to 4096 - Reduce event channel buffer from 10000 to 2000 - Cap ip2fqdn map at 10000 entries - Reduce label interner max from 10000 to 5000 - Reduce LLM stream limits (maxActiveStreams 1000->200, buffer 1MB->64KB) - Cache stripped Go binary paths to avoid repeated uprobe attach attempts - Reduce GC interval from 10min to 5min - Clean up HTTP/2 HPACK parsers for dead connections in gc() Race condition fixes in Container.Collect(): - Snapshot connectionStats LRU under RLock (not concurrent-safe) - Call getListens/getProxiedListens under RLock - Protect logSamples map reads/writes with lock - Read restarts/oomKills under RLock - Read c.processes under RLock in onConnectionOpen before lock scope File descriptor leak fixes in logs/tail_reader.go: - Close file on Stat/Seek failure in NewTailReader - Close old file before setting nil in moved() - Cap partial line buffer at 64KB to prevent unbounded growth - Fix prefix accumulation bug (was replacing instead of appending) * fix: validate eBPF durations to prevent corrupted spans causing OTel OOM eBPF kernel probes occasionally produce garbage duration values where the uint64 has the high bit set. Casting to time.Duration (int64) produces negative durations, creating spans where end_time < start_time. These poison spans cause ClickHouse batch rejections, OTel collector retry accumulation, and eventual OOMKill (165 restarts observed). Source-level fix (eBPF event parsing): - Add safeDuration() that returns 0 for values >= 1 hour or == 0 - Replace all uint64->time.Duration casts in perf/ringbuf readers HTTP/2 parser fix: - Add safeKernelDuration() for kernel timestamp subtraction - Prevents unsigned underflow when events arrive out of order Defense-in-depth (span creation): - createSpan: drop spans with duration <= 0 or > 1 hour - LLMRequest: drop spans with zero/invalid timestamps or > 1 hour * fix: address review comments - Use double-checked locking for logSamples to avoid redundant writes - Use strconv.ParseInt instead of fmt.Sscanf for stricter cgroup parsing * fix: gofmt alignment in llm_stream.go * fix: use debug.SetMemoryLimit instead of runtime.SetMemoryLimit * chore(deps): bump go.opentelemetry.io/otel/sdk from 1.37.0 to 1.40.0 (#205) Bumps [go.opentelemetry.io/otel/sdk](https://github.com/open-telemetry/opentelemetry-go) from 1.37.0 to 1.40.0. - [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases) - [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md) - [Commits](open-telemetry/opentelemetry-go@v1.37.0...v1.40.0) --- updated-dependencies: - dependency-name: go.opentelemetry.io/otel/sdk dependency-version: 1.40.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * fix: resolve IP-based destinations to FQDN via HTTP headers (#215) * fix: resolve IP-based destinations to FQDN via HTTP headers and eBPF dport extraction Two complementary fixes for the ip_to_fqdn resolution gap where external destinations show raw IPs instead of FQDNs in metrics: 1. Go: Use HTTP Host header and HTTP/2 :authority pseudo-header to resolve IP-based DestinationKeys to FQDNs when DNS capture fails (due to Go goroutine migration breaking pid-based eBPF tracking). Extracts migrateConnectionKeyToFQDN helper from migrateConnectionKeyIfNeeded. 2. eBPF: Extract destination port from connect() sockaddr arguments so non-TCP connections (UDP DNS) get proper dport in active_connections. Previously sys_exit_connect created entries with dport=0 for UDP, preventing DNS protocol detection for port 53. * fix: address review feedback - return ip2fqdn for HTTP/2 and add early return guard * fix: reduce memory footprint by removing unused labels and shrinking eBPF maps (#217) * fix: reduce memory footprint by removing unused labels and shrinking eBPF maps - Remove 7 unused labels from TCP/L7 metrics (src_region, src_az, destination_workload_region, destination_workload_az, actual_destination_region, actual_destination_az, actual_destination_instance). These are derivable server-side via container_info joins. - Reduce MAX_CONNECTIONS from 1M to 100K (saves ~100MB kernel memory) - Reduce MAX_PAYLOAD_SIZE from 8192 to 4096 - Reduce active_l7_requests max_entries from 8192 to 4096 - Reduce connectionStatsCacheSize from 4096 to 2048 * fix: sync MaxPayloadSize Go constant with eBPF MAX_PAYLOAD_SIZE (4096) Without this, the Go agent reads L7 response data at the wrong offset, breaking all L7 protocol monitoring. * fix: prevent OOMKills by lowering GOMEMLIMIT and reducing memory allocations (#218) - Lower GOMEMLIMIT from 90% to 60% of cgroup limit. The cgroup OOM killer counts kernel memory (~80MB for eBPF maps) and page cache (~200MB from /proc reads) which GOMEMLIMIT doesn't control. - Fix Event struct Payload/Response arrays: use MaxPayloadSize (4096) instead of hardcoded 8192. Saves ~16MB in the 2000-entry event channel buffer. - Reduce l7_events ring buffer from 16MB to 8MB. With 4KB payloads, 8MB still holds ~2000 events. * fix: detect Go applications on listen events for container_application_type (#216) attachTlsUprobes (which detects Go binaries via buildinfo) was only called on outbound connection events. Go servers that only listen for incoming connections were never detected as golang apps. Add the same call on ListenOpen events so server-only Go processes are properly identified. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * fix: eliminate scrape deadlock, Prometheus wrapping allocs, and K8s informer memory (#219) * fix: eliminate WrapRegistererWith allocation storm on Prometheus scrapes Removes the 3-layer WrapRegistererWith wrapping chain for container metrics that caused ~200MB of transient allocations per scrape on busy nodes. Container metrics now embed const labels directly, and L7 CounterVec/HistogramVec use ConstLabels on their opts for zero per-scrape overhead. ip2fqdn and LLM metrics retain minimal wrapping. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: use NodeConstLabels struct instead of magic indices, fix variable shadowing Addresses PR review: replaces []string with named struct fields for node-level const labels. Also fixes variable shadowing where loop variable `c` in GetSensitiveCounters range shadowed the Container receiver, causing a compile error. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: reduce K8s informer memory by stripping cached objects and removing redundant API calls - Add SetTransform functions to all 9 informers to strip unneeded fields (ManagedFields, Annotations, Spec.Containers, Status.Conditions, etc.) before objects enter the informer cache, reducing memory by ~80%. - Remove getFullClusterSnapshot() which did redundant List() API calls for all resource types at startup (informers already do their own initial List). Replace with factory.WaitForCacheSync() + updateIpMapping() to eliminate the double-fetch memory spike. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: push-model metrics to eliminate c.lock deadlock on scrapes Root cause: Collect() held c.lock.RLock() for extended periods while reading connectionStats, failedConnectionAttempts, activeConnections, and logSamples. Event handlers (onConnectionOpen, onL7Request, etc.) need c.lock.Lock() — creating a deadlock when a pending writer blocks recursive readers. Additionally, Registry.Collect() sent to trafficStatsUpdateCh (consumed by the event handler goroutine), so if the event handler was blocked on c.lock.Lock(), Registry.Collect() blocked too → Gather() never returns → zombie goroutines accumulate. Changes: - New TCPMetrics struct (CounterVec/GaugeVec) updated by event handlers at event time, not scrape time. Collect() calls tcpMetrics.collect(ch) without any c.lock — same pattern as existing L7Stats. - Remove connectionStats LRU cache (no longer needed for accumulation) - Remove failedConnectionAttempts map (push directly to CounterVec) - onConnectionOpen/onRetransmission push metrics before acquiring c.lock - updateConnectionTrafficStats pushes byte deltas to TCPMetrics - Active connections gauge refreshed periodically by event handler - Move updateStatsFromEbpfMapsIfNecessary() from Registry.Collect() into event handler ticker — eliminates trafficStatsUpdateCh deadlock - Remove trafficStatsUpdateCh/nodejsStatsUpdateCh/pythonStatsUpdateCh channels — event handler calls container methods directly - logSamples changed from map[string]string to sync.Map for lock-free LoadOrStore (eliminates c.lock.Lock() in Collect for log samples) - Restarts/OOMKills pushed to TCPMetrics counter at event time Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: use single containerCollector instead of per-container registration prometheus.Registry.Unregister does not work for unchecked collectors (empty Describe). It only searches collectorsByID, never the uncheckedCollectors slice. This meant every container replacement (process restart within same pod) leaked the old Container object in the registry — Gather() collected duplicate metrics and returned HTTP 500. Register a single containerCollector on the raw registry that iterates containersById under RLock. Individual container Register/Unregister calls removed entirely. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Raman Kumar <raman.kharche@nudgebee.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

mayankpande88 added 5 commits February 17, 2026 16:03

fix: add missing patternsPerLevelLimit param to logparser.NewParser c…

281e60f

…alls

fix: update logparser dep, sanitize log paths, replace test credential

eef39b9

fix: update logparser dep to fix sensitive pattern loading

060e11d

The previous version had a corrupted sensitive_patterns.json that caused "cannot unmarshal object into Go value of type []SensitivePattern" errors, breaking sensitive data detection in log parsing.

gemini-code-assist Bot reviewed Mar 7, 2026

View reviewed changes

Comment thread containers/container.go

Comment thread main.go

mayankpande88 added 4 commits March 7, 2026 14:44

merge: resolve conflicts with main

0693cde

- Combine connectionStats RLock snapshotting with enrichedKey aggregation - Keep stripped binary cache check and add debug logging from main

fix: address review comments

46eaeeb

- Use double-checked locking for logSamples to avoid redundant writes - Use strconv.ParseInt instead of fmt.Sscanf for stricter cgroup parsing

fix: gofmt alignment in llm_stream.go

4d98ff5

fix: use debug.SetMemoryLimit instead of runtime.SetMemoryLimit

f85f5a0

blue4209211 approved these changes Mar 7, 2026

View reviewed changes

blue4209211 merged commit c6e450d into main Mar 7, 2026
1 check passed

blue4209211 deleted the fix/memory-and-timestamp-fixes branch March 7, 2026 10:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: reduce memory usage, fix race conditions, and prevent corrupted eBPF spans#206

fix: reduce memory usage, fix race conditions, and prevent corrupted eBPF spans#206
blue4209211 merged 9 commits into
mainfrom
fix/memory-and-timestamp-fixes

mayankpande88 commented Mar 7, 2026

Uh oh!

gemini-code-assist Bot commented Mar 7, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

mayankpande88 commented Mar 7, 2026

Summary

Test plan

Uh oh!

gemini-code-assist Bot commented Mar 7, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants