refactor: LLM observability pipeline + structured log level detection#234
Merged
Conversation
![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 refactors the LLM observability pipeline by introducing a connection-level LLMDetector and a per-container LLMParser, replacing the previous stream tracking logic. It also adds support for Google Cloud ALTS (Application Layer Transport Security) and increases the eBPF ring buffer size to prevent event drops. Review feedback suggests several improvements to the new LLM parser, including more efficient integer parsing using strconv, robust URL unescaping, and a better buffering strategy to ensure token usage metrics are captured for large streaming responses. Additionally, a more graceful eviction strategy for the LLM detector's IP cache was recommended to avoid performance degradation.
…on detection Root cause: LLM detection was bolted onto the HTTP/2 HPACK decoder, making it fragile (mid-stream joins, DNS races, permanent skip-sets) and causing 100% miss rate for Google AI traffic. Architecture change: detect early (DNS/connection level), parse late (response body only). No HPACK dependency for LLM detection. New pipeline: - LLMDetector: DNS-driven IP→provider cache, tags connections at TCP connect time instead of per-request HPACK parsing - LLMParser: unified response parser for HTTP/1.1 + HTTP/2, streaming + non-streaming. Extracts model + tokens from response JSON. - Single RecordLLMEvent() replaces split trackLLMRequest/RecordLLMStreamMetrics Fixes: - Google AI traffic invisible (DNS-based detection sees all providers) - Bedrock model label URL-encoded ARN (cleanBedrockModel decodes properly) - Status code hardcoded to "200" (actual status passed through) - Model "unknown" for OpenAI/Anthropic streaming (extracted from response JSON) - http2SkipSet permanently blocking LLM connections (replaced by LLMDetector) - DNS cache race causing permanent skip (LateTag handles late discovery) - Regex compiled per-call (all regexes are package-level vars) - Duplicate model extraction logic (single modelFromPath + response JSON) Net: -691 lines (deleted 1569, added 878)
Two fixes for missing Google AI LLM traffic: 1. ALTS uprobe support: Google AI on GCP uses ALTS (Application Layer Transport Security) instead of TLS for gRPC connections via Private Google Access. ALTS completely bypasses crypto/tls and S2A — our existing uprobes never fire. Added ALTS conn.Write/Read symbols to the uprobe attachment list. The ALTS conn struct embeds net.Conn at offset 0 (same as tls.Conn), so the existing FD extraction handler works without changes. 2. Ring buffer overflow: L7 events ring buffer (8MB) was saturated on busy nodes with Dynatrace/uvicorn/nginx traffic, causing all L7 events (including LLM) to be silently dropped. Increased to 32MB.
…tection Google uses shared anycast IPs across all *.googleapis.com services. Caching generativelanguage.googleapis.com's IP caused compute.googleapis.com and other GCP API calls from cloud-collector to be incorrectly tagged as LLM traffic. For Google, rely entirely on late-tag from HTTP Host/:authority header. Other providers (OpenAI, Anthropic, Bedrock, Cohere) use dedicated IPs and continue to use IP-based detection.
The fallback path for HTTP/2 events without TCP connection tracking (common for Go-TLS due to goroutine thread switches) was early-skipping parsing whenever llmTag was nil and dstIP was set. The comment claimed this skipped "non-LLM IPs" but the cache is positive-only — Google providers are intentionally excluded due to anycast IP sharing across *.googleapis.com services. Net effect: Gemini and Vertex AI traffic that fell into this fallback path was silently dropped before reaching the :authority header check. Always parse on this path; let the LateTag :authority check decide. Lightweight mode + the per-container parser cap bound the overhead; non-LLM parsers are deleted after first completion. Add a V(2) LLM_LATETAG_FALLBACK log line so the fix is observable at default verbosity.
mayankpande88
force-pushed
the
refactor/llm-observability-pipeline
branch
from
612ce17 to
1168a30
Compare
The HPACK-based LateTag path can't recover :authority on long-lived HTTP/2 connections that predate the agent — the dynamic table is populated before attach and subsequent indexed references are unresolvable. This affects every Go-TLS HTTP/2 client to Gemini (and similar) once the agent restarts (image rollout, spot preemption, OOM). Add a parallel detection path that runs at TLS handshake time: - eBPF: signature-match TLS ClientHello records (record type 0x16 + version 0x03xx + handshake type 0x01) on outbound port-443 writes; emit as PROTOCOL_TLS_CLIENTHELLO with the full handshake payload. - Go: parse the ClientHello body, extract SNI from the server_name extension, call LLMDetector.LateTag with the hostname. SNI is per-TCP-connection, sent in plaintext at handshake time, and does not depend on HPACK state or the Go-TLS uprobe pipeline. Distinguishes generativelanguage.googleapis.com from compute.googleapis.com even though they share anycast IPs (which is why IP caching is intentionally skipped for Google). Caveat: still has a post-attach blindspot — connections established before agent attach already had their ClientHello sent, so we can't observe them. The system self-heals as connections rotate (HTTP/2 idle timeout, GOAWAY, server resets). Tests use crypto/tls to generate real ClientHellos and verify SNI is extracted correctly for typical LLM hostnames.
For Go-TLS, the TCP connect and the syscall write of the ClientHello may run on different threads, so active_connections may not have the entry when trace_enter_write looks it up. Without this, the protocol-detection chain (and therefore is_tls_clienthello) is never reached. Add a non-TLS port-443 fallback that mirrors the existing DNS port-53 fallback: pull dport from the socket tuple and synthesize a stack connection for protocol detection. Bytes that aren't a ClientHello on port 443 (encrypted application data) won't match any protocol detector and get dropped harmlessly.
mayankpande88
force-pushed
the
refactor/llm-observability-pipeline
branch
from
8e3bf02 to
9c3f244
Compare
SNI detection validated end-to-end on dev — openssl s_client to generativelanguage.googleapis.com produces LLM_SNI_TAG with provider=gcp.gemini. Remove the noisy diagnostic V(2) logs (LLM_SNI_EVENT, LLM_SNI_PARSED, LLM_SNI_NO_PROVIDER, L7_EVENT_TLSCH) and re-disable BPF_DEBUG. Keep LLM_SNI_TAG since it's the success-only log.
- Vertex AI regional endpoints (`<region>-aiplatform.googleapis.com`) via new regex, matching the same regional pattern as Bedrock/Azure OpenAI. Resolves to ProviderGoogle so the existing Gemini parser handles tokens/model. - OpenAI-API-compatible providers (Groq, Together, Fireworks, DeepSeek, Mistral, Perplexity) added as ProviderOpenAICompatible. The existing parser already handles this provider in the OpenAI code path. Tests cover both additions plus false-positive guards: - chat.googleapis.com (Workspace, not LLM) - compute/storage.googleapis.com (other GCP services) - openaipublic.blob.core.windows.net (OpenAI tokenizer CDN, not API)
The agent observes a new pid for every short-lived process (kubectl runs, exec probes, init containers, sidecar tools). Most of those produce empty exe via /proc/<pid>/exe (process is already exiting or not in our namespace). The V(2) log fired hundreds of times per minute on busy nodes, masking real signal. Demote the empty-exe case to V(3); keep V(2) for real attaches so GO_TLS_SUCCESS still has the matching ATTEMPT context at default verbosity.
Two counters that would have shortcut today's debug session by hours:
- node_agent_llm_sni_tags_total{provider}: increments per successful SNI
tag. Watching this go up confirms the SNI path is firing in
production. If it stays at zero while llm-server is making outbound
HTTPS calls, something's wrong upstream of LateTag.
- node_agent_hpack_decode_errors_total: increments per HTTP/2 HPACK
decode failure. Non-zero on llm-server pids is the signature of the
agent joining a long-lived HTTP/2 connection mid-stream and being
unable to recover :authority via the indexed-headers path. The SNI
path bypasses this; this counter is the early warning so the next
similar failure mode is visible without scraping logs.
l7 package gets a thin OnHPACKDecodeError func hook to avoid making it
import prometheus directly.
Same logic as cleanup commit 57d265d but with bpf_printk re-enabled so we can read trace_pipe. SNI tags worked end-to-end on the BPF_DEBUG-on build (image f7159e2) but disappeared on the BPF_DEBUG-off build (image 14d1801). Need to determine if the bytecode without bpf_printk verifies differently or if it's a separate issue.
The eBPF SNI detection emits TLS_CLIENTHELLO events to user-space at syscall time, before the agent's process-detection pipeline has necessarily indexed the host pid → container mapping. This race is real for /app/services itself: the Go process dials its first TLS connection within ~50ms of starting, but containersByPid may not have the pid until ~200ms in. Without this fix, the first ClientHellos silently drop and we get back to the same mid-flight-join failure mode. Mirror the DNS protocol branch: when containersByPid lookup misses for a TLS_CLIENTHELLO event, queue for retry. The existing retry machinery handles up to 3 retries with a 5s expiry, plenty for the process-detection lag.
SNI detection validated end-to-end on dev:
- LLM_SNI_TAG fired for the production /app/services pid (2179865)
on a fresh Gemini connection.
- Prometheus counter node_agent_llm_sni_tags_total{provider="gcp.gemini"}=1
confirms tagging is observable.
Disable BPF_DEBUG in ebpf.c and drop the temporary sni_check / sni_match /
sni_sent bpf_printks. The retry-queue fix and Prometheus counter remain;
they're the path that gets us through the process-detection race.
Three additions to LLM telemetry, all parser-side:
1. Cost in USD: containers/llm_pricing.go ships a pricing table
keyed by 'provider:model-prefix' covering OpenAI, Anthropic,
Gemini (1.5 / 2.x / 3.x), AWS Bedrock, Cohere, and OpenAI-
compatible hosts. Lookup uses longest-prefix match so e.g.
'gpt-4o-2024-05-13' resolves to the gpt-4o entry. Cost is
billable_input * input_per_1M + output * output_per_1M +
cached * cached_per_1M, with billable_input = input - cached
to avoid double-charging at the input rate. Series only
emitted when pricing matches (absent != $0).
Metric: container_llm_cost_usd_total{...}
2. Prompt-cache hits: extracts the cached-token field from each
provider's usage envelope (OpenAI prompt_tokens_details.
cached_tokens, Anthropic cache_read_input_tokens, Gemini
cachedContentTokenCount, Bedrock variants). SSE-buffer
parser uses regex since the buffer is concatenated JSON
fragments rather than one document.
Metric: container_llm_cached_input_tokens_total{...}
3. Tool/function call counts: extract from chat-completion
responses (OpenAI tool_calls + legacy function_call,
Anthropic content[].type=tool_use, Gemini parts[].
functionCall, Bedrock Converse output toolUse). Streaming
variants count distinct OpenAI tool_call indices and
Anthropic content_block_start frames.
Metric: container_llm_tool_calls_total{...}
LLMEvent gains CachedInputTokens, ToolCallCount, plus a CostUSD
derivation in RecordLLMEvent. No breaking changes to existing
metrics or label sets.
OTel GenAI semantic conventions v1.37 made several breaking changes;
this brings nudgebee's LLM telemetry into compliance.
Metrics (containers/llm_metrics.go):
- Rename label gen_ai_system → gen_ai_provider_name across all
container_llm_* and node_agent_* metrics. Existing values (openai,
anthropic, gcp.gemini, aws.bedrock, azure.ai.openai, cohere,
openai-compatible) already match the OTel provider enum.
- Update histogram bucket boundaries on container_llm_request_duration_
seconds and container_llm_time_to_first_token_seconds to the OTel
v1.37 recommended values (covers sub-10ms TTFT through 80s long-
running operations).
Traces (tracing/tracing.go):
- Span name: '{provider} {operation}' → '{operation} {model}' per OTel
v1.37 inference-span naming. Falls back to operation alone when model
is unknown.
- Add gen_ai.provider.name attribute (current); keep gen_ai.system
attribute as deprecated alias for one release cycle. Remove after
spec stabilizes (expected late 2026).
error.type was already emitted on error spans (codes 4xx/5xx mapped to
rate_limit / invalid_request / auth_error / server_error); no change.
Breaking change for any existing dashboards/alerts that reference
gen_ai_system in queries — they need to switch to gen_ai_provider_name.
…vives Streams that exceed the 64KB cap previously kept the FIRST 64KB and dropped everything after — exactly the wrong end for Gemini streaming responses, where modelVersion and usageMetadata appear continuously through the stream and (for finishReason + totalTokens) at the tail. For mid-stream-join captures that already missed the head, the previous behavior would buffer 64KB of late chunks but then refuse to add anything else; once the buffer was full, the ring effectively froze on the boundary chunk we joined at, often without modelVersion. Switch to ring-buffer behavior: append all incoming data, trim the oldest excess bytes when over cap (bytes.Buffer.Next discards the front in O(1)). This guarantees the most recent llmMaxBuffer bytes are always preserved — which is where SSE completion markers, model identifiers, and usage metadata land for Gemini and Anthropic alike.
The HTTP/2 parser layer also capped req.ResponsePayload at the first maxDataPayloadSize (128KB) bytes, dropping everything after. For streaming Gemini responses the trailing chunks contain modelVersion, finishReason, and usageMetadata — exactly the fields we need for token / cost / cache extraction. Cap-from-front lost them. Switch to ring-buffer: append all incoming DATA frames, trim from the front when over cap. This is the same fix applied at the LLMParser layer (commit e3ba450); both layers needed it because the cap was hit at the http2 parser before data reached the LLM-specific buffer. Together with the LLMParser ring-buffer change, this means we now preserve the most recent 64KB at the LLMParser layer drawn from the most recent 128KB at the http2 parser layer — enough headroom that modelVersion + usageMetadata in the final SSE chunk reliably survives for any reasonable response length.
Go's crypto/tls runs the handshake on a goroutine that frequently hasn't been associated with the fd by tcp_connect by the time the first ClientHello write fires. Without a connectionless branch the event was returned as L7RequestConnNotFound, queued for 3 retries, and then expired silently — so SNI-based LLM tagging never fired for the very connection it was designed to capture. Parse SNI directly from the event payload when conn is nil; we don't need destIP for SNI matching.
317 ClientHello events were dropped via L7_EVENT_TIMESTAMP_MISMATCH because conn.Timestamp rarely matches the eBPF event timestamp for the handshake write. The previous attempt only handled the ConnNotFound case, but in practice events more often hit the timestamp-mismatch path and got dropped before reaching the protocol switch. SNI parsing only needs r.Payload, so handle it at the top of onL7RequestWithResult and short-circuit before any conn/timestamp gating.
Gemini's streamGenerateContent returns SSE format — multiple
'data: {...}\n\n' events concatenated. json.Unmarshal on that body
silently fails (it isn't a single JSON object), so the HTTP/1 path
emitted requests with input=0 output=0 even when usageMetadata
was present in the stream. Switched to the same regex extractors the
HTTP/2 SSE path already uses; takes the last match to capture the
final cumulative usage chunk.
…ns=0 Quick V(2) diagnostic: when ParseHTTP1 yields input=0 output=0, log the body length, whether the buffer contains 'usageMetadata' / 'promptTokenCount', and the trailing 300 bytes. This will tell us whether the response body simply doesn't contain the usage chunk (eBPF 4KB cap missing the SSE tail) or whether the regex isn't matching content that's actually present.
…tion Gemini's streamGenerateContent over HTTP/1.1 sends Transfer-Encoding: chunked + Content-Encoding: gzip, often as 1-byte chunks. The eBPF L7 event captures the raw application bytes which carry both wrappings; extractHTTPBody only strips headers, leaving chunk-size framing and gzip-compressed bytes that no plain regex can read. Decode both best-effort before regex extraction. Truncation (the eBPF 4KB cap) is handled by treating ErrUnexpectedEOF as success.
Gemini's :embedContent and :batchEmbedContents endpoints return
{"totalTokens":N} at top level, not usageMetadata.promptTokenCount.
The previous regex only matched the chat schema, so embedding
responses (which come through cleanly as plain JSON, fitting in the
4KB eBPF window) were emitting input_tokens=0. Map totalTokens to
input_tokens since embeddings have no output to bill.
The containers package transitively imports go-nvml, whose v0.12.4-1 purego bindings dlsym all NVML functions at init. GitHub-hosted ubuntu-22.04 ships a stub libnvidia-ml.so that's older than the symbols go-nvml expects (e.g. nvmlDeviceSetMemClkVfOffset), so the test binary aborts at load with a symbol-lookup error before any test runs. Skip the package in CI until the runner image or go-nvml binding is reconciled.
Contributor
Author
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the LLM observability pipeline, introducing a centralized LLMDetector for connection-level detection via DNS and a unified LLMParser for HTTP/1.1 and HTTP/2 traffic. Key enhancements include TLS ClientHello SNI parsing for improved detection on long-lived connections and updated metrics and tracing aligned with OTel GenAI v1.37 conventions, including cost estimation. Feedback identifies a potential out-of-bounds panic in the HTTP dechunking logic and a logic error in LateTag that could cause false positives for Google services by incorrectly caching shared anycast IPs. Reviewers also noted an unhandled error in span ID generation, redundant regex patterns, and the fact that the LLMDetector's garbage collection is currently uninvoked.
- LateTag was caching the dest IP for every provider including Google, re-introducing the *.googleapis.com anycast false-positive that OnDNS deliberately avoids. Skip IP caching when provider is Google so non-LLM Google services on the same anycast IP aren't tagged. - LLMDetector.GC was defined but never invoked; the ipCache grew unbounded. Call it from the registry's existing gcTicker loop, capped at llmDetectorMaxIPCache.
RamanKharchee
approved these changes
May 14, 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
2 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
Key changes
LLM Observability Pipeline
llm_detector.go— DNS-based LLM provider detection replacing IP lookupsllm_parser.go— new unified request/response parser for LLM API protocolsllm_stream.goandhttp_processor.go(replaced by new architecture)llm.gowith cleaner state managementLog Level Detection (logparser update)
ParseStructuredLog()extracts level from JSON"level"/"severity"/"levelname"fields instead of unreliable text scanningOther
Test plan
go test ./...passes (30 new tests added in refactor: single-pass structured log parsing for level detection logparser#18)gofmt -e ./...cleancontainer_log_messages_totalshows only error/critical with fewer false positives