feat(metrics): split L7 event counter by HTTP/2 frame direction - #318
Merged
Conversation
The pipeline counters from #317 localised the failure but not its cause. External HTTP/2, 10 minutes on dev: L7 events 1,155,643 internal control: 844,250 stream_created 53 8,052 response_status 0 5,478 completed 0 5,158 That is ~22,000 external events per stream created, against ~105 internally — a 200x difference — and response_status is exactly zero. Zero matters because :status decodes from the HPACK static table (index 8), which survives a degraded decoder; the internal control produces 5,478 of them on the same code path. Server-side HEADERS are therefore never decoded externally at all. Also ruled out by that run, so they need no further attention: parser cap drops were 0, and stale fd reuse was 3 events externally in 10 minutes. Two explanations remain for the ratio, and direction separates them cleanly: server-frame events scarce -> responses never reach the parser server-frame events plentiful -> the bytes are not HTTP/2 and the eBPF port-based detection (is_likely_http2_port, 443/8443, plus a frame-shape check) is over-matching TLS traffic The second would also recast node_agent_hpack_decode_errors_total as a symptom of feeding non-HTTP/2 bytes to an HPACK decoder rather than a cause. Adds a direction label ("client"/"server", "-" where inapplicable) to node_agent_l7_events_total. Measurement only.
There was a problem hiding this comment.
Code Review
This pull request introduces a new 'direction' label to the L7EventsTotal Prometheus metric to distinguish between client and server frames in HTTP/2 traffic, while using a default '-' value for other protocols. This is achieved by adding a helper function frameDirection in containers/container.go and updating the metric definition in containers/llm_metrics.go. I have no feedback to provide as the changes are clean and well-documented.
RamanKharchee
approved these changes
Sep 5, 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
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.
What #317 established
The pipeline counters localised the failure. External HTTP/2, 10 minutes on dev at rc.5:
Ruled out and needing no further attention: parser cap drops were
0, stale fd reuse was3externally in 10 minutes. Both candidates #317 was originally written for are dead.Two things stand out:
response_statusis exactly zero externally. Zero is the load-bearing detail::statusdecodes from the HPACK static table (index 8), which survives a degraded decoder, and the internal control produces 5,478 of them on the same code path. Server-side HEADERS are never decoded externally at all.What this PR separates
Two explanations remain for that ratio, and frame direction distinguishes them in one deploy:
is_likely_http2_port, 443/8443, plus a frame-shape check) is over-matching TLS trafficThe second would recast
node_agent_hpack_decode_errors_total(6.28M/12h) as a symptom of feeding non-HTTP/2 bytes to an HPACK decoder, rather than a cause — which would also explain why fixing genuine HPACK/truncation defects in #316 reduced the error count without restoring any decoding.Adds a
directionlabel (client/server,-where inapplicable) tonode_agent_l7_events_total.Status
Measurement only, no behaviour change.
gofmt,GOOS=linux gopls check, full CI suite pass.Stated plainly: four diagnoses have already been wrong here (mid-stream join, attach ordering, payload truncation, parser lifecycle). This PR does not assert a cause — it is the single cheapest measurement that discriminates between the two remaining ones, and either outcome is informative.