Skip to content

fix: lightweight HTTP/2 parsing to reduce CPU on busy nodes#227

Merged
mayankpande88 merged 3 commits into
mainfrom
fix-node-agent-oom
Mar 14, 2026
Merged

fix: lightweight HTTP/2 parsing to reduce CPU on busy nodes#227
mayankpande88 merged 3 commits into
mainfrom
fix-node-agent-oom

Conversation

@mayankpande88

Copy link
Copy Markdown
Contributor

Summary

  • HTTP/2 frame parsing was consuming 70% of CPU (profiled via pprof on a pod with 47 containers). Most HTTP/2 traffic (gRPC, K8s API, service mesh) doesn't need payload capture — only status codes for L7 metrics.
  • Adds lightweight mode to Http2Parser: skips DATA payload accumulation and RequestHeaders map for non-LLM traffic, auto-upgrades to full mode when :authority matches a known LLM provider
  • Eliminates per-Parse() map allocations (statuses/grpcStatuses) by reusing maps with clear() — ~15% CPU savings
  • Caps activeRequests at 100 per connection and parsers at 50 per container
  • Fixes parser GC leak for connectionless parsers and removes duplicate GetActiveStreamsForLLM() call

What's preserved

  • L7 metrics (status codes) — HEADERS frames always fully decoded
  • LLM detection — auto-upgrades to full mode when LLM host detected
  • gRPC status tracking — always decoded from HEADERS/Trailers
  • Trace correlation — Http2Request still emits method/path/status

Test plan

  • Deploy to dev cluster and verify L7 metrics still captured for HTTP/2 services
  • Verify LLM tracking still works (Gemini/OpenAI calls captured)
  • Compare CPU usage via pprof: expect 60-70% reduction on HTTP/2 hot path
  • Check cAdvisor CPU: 4 pods at 1 core should drop to ~0.2-0.3 cores
  • Verify no memory leak in googleHTTP2Parsers over time

HTTP/2 frame parsing was consuming 70% of CPU (profiled). Most HTTP/2
traffic (gRPC, K8s API, service mesh) doesn't need payload capture —
only status codes for L7 metrics.

- Add lightweight mode to Http2Parser: skips DATA payload accumulation
  and RequestHeaders map allocation for non-LLM traffic
- Auto-upgrade to full mode when :authority matches an LLM provider
- Reuse statuses/grpcStatuses maps across Parse() calls (clear instead
  of alloc) — eliminates ~15% CPU from per-call map allocation
- Cap activeRequests at 100 per connection to prevent orphan stream growth
- Cap HTTP/2 parsers at 50 per container
- Fix parser GC leak: connectionless parsers now cleaned up when pid dies
- Eliminate duplicate GetActiveStreamsForLLM() call per event
- Skip LLM stream tracker entirely when parser is lightweight
@gemini-code-assist

Copy link
Copy Markdown

Summary of Changes

Hello, 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 focuses on enhancing the efficiency of HTTP/2 parsing, particularly in environments with high HTTP/2 traffic. By introducing a lightweight parsing mode and optimizing memory management, it aims to drastically reduce CPU utilization and prevent resource exhaustion, while preserving essential L7 metrics and LLM detection capabilities.

Highlights

  • Lightweight HTTP/2 Parsing: Introduced a lightweight HTTP/2 parsing mode to significantly reduce CPU consumption (up to 70%) on busy nodes by skipping payload capture for non-LLM traffic.
  • Memory Allocation Optimization: Optimized memory allocation by reusing statuses and grpcStatuses maps, leading to approximately 15% CPU savings.
  • Resource Limits: Implemented limits on active HTTP/2 requests per connection (100) and parsers per container (50) to prevent unbounded memory growth.
  • Garbage Collection Fixes: Corrected a garbage collection leak for connectionless HTTP/2 parsers and removed a redundant call to GetActiveStreamsForLLM().

🧠 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
  • containers/container.go
    • Defined a constant maxHTTP2ParsersPerContainer to limit the number of HTTP/2 parsers.
    • Configured newly created Http2Parser instances to operate in lightweight mode and assigned an LLM host checker.
    • Adjusted LLM stream tracking to only activate for non-lightweight parsers.
    • Refined the garbage collection mechanism for HTTP/2 parsers to correctly handle connectionless instances.
  • containers/llm.go
    • Added a helper function isLLMRelevantHost to identify LLM provider hosts.
  • ebpftracer/l7/http2.go
    • Introduced maxActiveRequests to cap concurrent HTTP/2 streams per connection.
    • Modified Http2Parser to include Lightweight mode and an LLMHostChecker callback.
    • Implemented map reuse for statuses and grpcStatuses to reduce allocations.
    • Applied the maxActiveRequests limit when creating new HTTP/2 requests.
    • Added logic to dynamically switch from lightweight to full parsing mode upon detecting an LLM-relevant :authority header.
    • Conditionalized DATA frame payload accumulation based on the Lightweight flag.
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.

Footnotes

  1. 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 gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces significant performance optimizations for HTTP/2 parsing, primarily by adding a lightweight parsing mode that avoids unnecessary payload processing for non-LLM traffic. This should substantially reduce CPU usage on busy nodes as intended. The changes also include other good optimizations like reusing maps to reduce allocations and capping resource usage to prevent unbounded memory growth. However, I've found a high-severity memory leak in the updated garbage collection logic for HTTP/2 parsers. While fixing a leak for connectionless parsers, it seems to have introduced a new one for parsers of closed connections in long-running processes. I've also left a medium-severity comment about a minor code duplication that could be refactored for better maintainability.

Comment thread containers/container.go
Comment thread ebpftracer/l7/http2.go Outdated
…END_STREAM

- Fix parser GC: closed connection parsers in long-running processes were
  not cleaned up. Now uses heuristic: if pid is alive but no connection
  exists, clean up parsers with zero active requests and no partial data.
- Add HasPartialData() to Http2Parser for safe GC decisions.
- Deduplicate END_STREAM handling: extract it before lightweight/full branch
  instead of duplicating in both paths.
processHTTP2WithoutConnection produces zero L7 metrics (no connection =
no DestinationKey). Its only purpose is LLM detection, yet it was parsing
every HTTP/2 frame from gRPC/K8s API traffic — consuming 70% of CPU on
busy nodes.

Two-phase skip-set approach:
1. If dstIP is known, check DNS cache — skip immediately if not LLM.
2. Otherwise, parse first event per pid:fd in lightweight mode to extract
   :authority. If not LLM, add to skip-set and never parse that fd again.

Also adds GC for skip-set entries when the pid dies.
@mayankpande88 mayankpande88 merged commit 3042078 into main Mar 14, 2026
1 check passed
@mayankpande88 mayankpande88 deleted the fix-node-agent-oom branch March 14, 2026 11:34
mayankpande88 added a commit that referenced this pull request Apr 9, 2026
* fix: update logparser to v0.0.0-20260310062405-9d6258bdd771

Picks up fix/json-log-pattern-hashing (PR #16) which stabilizes
JSON log pattern hashing by extracting only message fields.

* fix: configurable sensitive log detection with optimized logparser (#224)

* fix: update logparser and add configurable sensitive log detection flags

- Update logparser to v0.0.0-20260313095946-b22168b95d9c with optimized
  sensitive parsing (anchor pre-filter, confidence tiers, singleton cache)
- Add SENSITIVE_LOG_SAMPLE_RATE, SENSITIVE_LOG_MIN_CONFIDENCE,
  SENSITIVE_LOG_MAX_DETECTIONS flags for fine-grained control
- Update all NewParser callsites to use SensitiveConfig struct
- Remove local replace directive

* fix: deduplicate SensitiveConfig and align flag naming

- Extract SensitiveConfig into a single variable reused across all 3 NewParser calls
- Rename flag/env var to sensitive-log-max-detections-per-container / SENSITIVE_LOG_MAX_DETECTIONS_PER_CONTAINER to match variable name

* fix: cap HTTP/2 DATA payload accumulation to prevent OOM (#226)

Http2Parser.Parse appends DATA frame payloads to RequestPayload and
ResponsePayload with no size limit. On nodes with high-throughput
HTTP/2 traffic (envoy-gateway, gRPC proxies, benchmark-server), these
grow unbounded — confirmed via pprof: 149MB (62% of heap) consumed by
Http2Parser on a pod that OOMs after ~22h.

Cap both payloads at 128KB per stream. They are only used for LLM
provider detection and trace spans which need at most a few KB.
END_STREAM tracking is unaffected so stream completion still works.

* fix: graceful shutdown, L7 panic protection, and bounded strippedGoExeCache (#225)

- Replace os.Exit(0) in signal handler with context cancellation so deferred
  cleanup (cr.Close, profiling.Stop, resolver.StopWatching) runs on shutdown.
  Previously leaked eBPF programs, uprobes, and perf buffers on every restart.
  Use http.Server.Shutdown for graceful HTTP server termination. (fixes #207)

- Wrap L7 event processing with recover() to prevent a single malformed packet
  from crashing the entire agent. Covers both processL7Event and pending L7
  retry paths. (fixes #210)

- Replace unbounded sync.Map with LRU cache (cap 1000) for strippedGoExeCache
  to prevent slow memory growth on CI/CD nodes with many unique Go binaries.
  hashicorp/golang-lru is already a dependency. (fixes #214)

* fix: lightweight HTTP/2 parsing to reduce CPU on busy nodes (#227)

* fix: lightweight HTTP/2 parsing to reduce CPU on busy nodes

HTTP/2 frame parsing was consuming 70% of CPU (profiled). Most HTTP/2
traffic (gRPC, K8s API, service mesh) doesn't need payload capture —
only status codes for L7 metrics.

- Add lightweight mode to Http2Parser: skips DATA payload accumulation
  and RequestHeaders map allocation for non-LLM traffic
- Auto-upgrade to full mode when :authority matches an LLM provider
- Reuse statuses/grpcStatuses maps across Parse() calls (clear instead
  of alloc) — eliminates ~15% CPU from per-call map allocation
- Cap activeRequests at 100 per connection to prevent orphan stream growth
- Cap HTTP/2 parsers at 50 per container
- Fix parser GC leak: connectionless parsers now cleaned up when pid dies
- Eliminate duplicate GetActiveStreamsForLLM() call per event
- Skip LLM stream tracker entirely when parser is lightweight

* fix: address PR review — GC leak for closed connections, deduplicate END_STREAM

- Fix parser GC: closed connection parsers in long-running processes were
  not cleaned up. Now uses heuristic: if pid is alive but no connection
  exists, clean up parsers with zero active requests and no partial data.
- Add HasPartialData() to Http2Parser for safe GC decisions.
- Deduplicate END_STREAM handling: extract it before lightweight/full branch
  instead of duplicating in both paths.

* fix: skip-set for non-LLM HTTP/2 traffic to eliminate CPU waste

processHTTP2WithoutConnection produces zero L7 metrics (no connection =
no DestinationKey). Its only purpose is LLM detection, yet it was parsing
every HTTP/2 frame from gRPC/K8s API traffic — consuming 70% of CPU on
busy nodes.

Two-phase skip-set approach:
1. If dstIP is known, check DNS cache — skip immediately if not LLM.
2. Otherwise, parse first event per pid:fd in lightweight mode to extract
   :authority. If not LLM, add to skip-set and never parse that fd again.

Also adds GC for skip-set entries when the pid dies.

* fix: prevent runtime crash from uint64-to-int overflow in event readers (#228)

* fix: prevent runtime crash from uint64-to-int overflow in event readers

Both perf and ring buffer L7 event readers convert eBPF-reported payload
sizes from uint64 to int without safe clamping. If eBPF sends corrupted
data with high-bit-set sizes (e.g. 0x8000000000000001), int() produces
a negative value that bypasses the MaxPayloadSize cap, causing
make([]byte, negative) to crash the reader goroutine.

Also upgrades cilium/ebpf v0.20.0 → v0.21.0 which includes ring buffer
and memory management fixes relevant to Go 1.24.

Fixes runtime crashes: "fatal error: stack not a power of 2" observed on
12/19 pods after ~7 hours of operation.

* fix: revert cilium/ebpf upgrade, keep clampSize fix only

cilium/ebpf v0.21.0 has breaking API changes (removed Sym, RewriteConstants,
MapName) that affect transitive dependencies. Revert to v0.20.0 and keep
just the uint64-to-int overflow fix.

* fix: evict stale CounterVec label combinations to prevent series explosion (#231)

CounterVec entries for closed TCP connections were never removed,
causing series count to grow linearly with pod uptime. On a 40h pod,
89% of emitted counter series were stale (85/95 destinations in a
single container had no active connection). At customer scale this
accumulated to 989K series (54% of all Prometheus series).

Track label combinations pushed to each CounterVec and periodically
delete entries whose destinations are no longer in activeConnections.
Eviction runs every gcInterval (5min) from the event handler goroutine,
taking mu.Lock to exclude concurrent collect() calls.

* chore: main to test (#237)

* deps: update logparser with structured log level detection (#235)

Updates github.com/nudgebee/logparser to include single-pass
structured log parsing that extracts level from JSON/logfmt fields
instead of unreliable text scanning (nudgebee/logparser#18).

Fixes false positive log level classification where INFO logs
containing "error"/"fatal" in message content were misclassified
as ERROR/CRITICAL.

* chore(deps): bump google.golang.org/grpc from 1.76.0 to 1.79.3 (#229)

Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.76.0 to 1.79.3.
- [Release notes](https://github.com/grpc/grpc-go/releases)
- [Commits](grpc/grpc-go@v1.76.0...v1.79.3)

---
updated-dependencies:
- dependency-name: google.golang.org/grpc
  dependency-version: 1.79.3
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix: prevent OOM crash from L7 protocol misidentification (#236)

The ClickHouse eBPF detector matched non-ClickHouse TCP payloads using
a 3-byte heuristic (0x01, 0x00, 0x01), causing ch-go to decode garbage
varint lengths and attempt 228TB allocations — a fatal OOM that bypasses
defer/recover.

3-layer defense:

Layer 0 — eBPF detection hardening:
  Remove non-port-gated is_clickhouse_query() call. ClickHouse native
  protocol detection now only on ports 9000/8123.

Layer 1 — Bounded parsing:
  Wrap ch-go proto.NewReader with io.LimitReader(payload size). Varint
  lengths exceeding actual payload now return io.EOF instead of OOM.

Layer 2 — Userspace validation:
  Add structural checks before invoking external libraries:
  - ClickHouse: validate query code byte + query ID length
  - Postgres: reject unknown frame types (Q/B/P/C only)
  - Zookeeper: validate opcode in known range

Layer 3 — Protocol reclassification:
  Track consecutive parse failures per connection. After 3 failures,
  override the cached protocol to stop further misidentified parsing.
  Logs a warning for observability.

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Shiv <3078106+blue4209211@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants