Skip to content

Replace largeBuffer []byte with chunked LargeBuffer#1431

Merged
grcevski merged 2 commits into
open-telemetry:mainfrom
rafaelroquetto:large_buffers
Mar 3, 2026
Merged

Replace largeBuffer []byte with chunked LargeBuffer#1431
grcevski merged 2 commits into
open-telemetry:mainfrom
rafaelroquetto:large_buffers

Conversation

@rafaelroquetto
Copy link
Copy Markdown
Contributor

The previous largeBuffer type was a plain struct{ buf []byte }. When a TCP payload spans multiple ring-buffer events, each largeBufferActionAppend call did append(lb.buf, chunk...), which copies the entire accumulated buffer every time it grows past capacity.

Replace it with LargeBuffer, a chunked store that keeps each ring-buffer chunk as an independent allocation. Appending is always a single copy into a freshly allocated slice plus one pointer append into the chunk index. Previously written data is never moved. Reading is handled by LargeBufferReader (cursor-based, io.Reader-compatible) for sequential parsers, and by UnsafeViewAt/UnsafeView (zero-copy within a chunk, scratch-backed across boundaries) for parsers that need all bytes at once.

Parsers that only need the full payload are changed to take *LargeBuffer and call UnsafeView() directly, eliminating the reader allocation at each call site. Parsers doing genuine sequential reads (handlePostgres, Kafka, http.ReadRequest/ReadResponse) keep *LargeBufferReader. The kafkaparser sub-package is migrated from raw []byte slices to a byteReader interface so it can consume a LargeBufferReader without importing ebpfcommon.

The new large_buffer_test.go covers construction, all cursor operations, random-access methods, scalar helpers, zero-alloc hot paths, and UnsafeView stale-slice semantics.


This is the foundation PR. Once merged, individual parsers can be migrated in follow-up PRs to take advantage of chunked reads, for example, avoiding the full-buffer materialisation in Postgres, or streaming Kafka frames directly from a LargeBufferReader without copying into an intermediate []byte first. For now, I tried to keep the code somewhat consistent.

I recommend reviewing pkg/ebpf/common/large_buffer.go as that contextualises the rest of the PR.

Checklist

@rafaelroquetto rafaelroquetto requested a review from a team as a code owner March 3, 2026 02:52
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 3, 2026

Codecov Report

❌ Patch coverage is 68.64065% with 233 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.86%. Comparing base (b6dda5f) to head (3208959).
⚠️ Report is 29 commits behind head on main.

Files with missing lines Patch % Lines
pkg/ebpf/common/large_buffer.go 70.65% 74 Missing and 7 partials ⚠️
pkg/internal/ebpf/kafkaparser/common.go 61.87% 38 Missing and 15 partials ⚠️
pkg/ebpf/common/sql_detect_postgres.go 40.00% 13 Missing and 2 partials ⚠️
pkg/ebpf/common/kafka_detect_transform.go 53.84% 11 Missing and 1 partial ⚠️
pkg/ebpf/common/sql_detect_mysql.go 0.00% 12 Missing ⚠️
pkg/ebpf/common/http_transform.go 72.97% 10 Missing ⚠️
pkg/ebpf/common/redis_detect_transform.go 25.00% 9 Missing ⚠️
pkg/ebpf/common/tcp_large_buffer.go 55.00% 8 Missing and 1 partial ⚠️
pkg/internal/ebpf/kafkaparser/fetch.go 85.45% 6 Missing and 2 partials ⚠️
pkg/ebpf/common/fast_cgi_detect_transform.go 75.86% 4 Missing and 3 partials ⚠️
... and 6 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1431      +/-   ##
==========================================
+ Coverage   43.74%   43.86%   +0.11%     
==========================================
  Files         312      313       +1     
  Lines       33911    34213     +302     
==========================================
+ Hits        14834    15007     +173     
- Misses      18122    18232     +110     
- Partials      955      974      +19     
Flag Coverage Δ
integration-test 21.60% <10.23%> (-0.18%) ⬇️
integration-test-arm 0.00% <0.00%> (ø)
integration-test-vm-x86_64-5.15.152 ?
integration-test-vm-x86_64-6.10.6 ?
k8s-integration-test 2.29% <0.00%> (-0.03%) ⬇️
oats-test 0.00% <0.00%> (ø)
unittests 44.89% <78.75%> (+0.26%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@mariomac mariomac left a comment

Choose a reason for hiding this comment

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

Loving the improvement. Thanks!

Comment thread pkg/ebpf/common/large_buffer.go
Comment thread pkg/ebpf/common/http_transform.go
Copy link
Copy Markdown
Contributor

@mmat11 mmat11 left a comment

Choose a reason for hiding this comment

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

lgtm!

Comment thread pkg/ebpf/common/kafka_detect_transform.go Outdated
@grcevski grcevski merged commit 226c057 into open-telemetry:main Mar 3, 2026
56 checks passed
@rafaelroquetto rafaelroquetto deleted the large_buffers branch March 3, 2026 21:22
@MrAlias MrAlias added this to the v0.6.0 milestone Mar 5, 2026
@MrAlias MrAlias mentioned this pull request Mar 5, 2026
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.

5 participants