Skip to content

perf: [durable_buffer] Avoid unnecessary copy of OTLP bytes in OtlpBytesAdapter::new()#2726

Open
gscalderon wants to merge 4 commits intoopen-telemetry:mainfrom
gscalderon:gscalderon/perf/zero-copy-otlp-bytes-adapter
Open

perf: [durable_buffer] Avoid unnecessary copy of OTLP bytes in OtlpBytesAdapter::new()#2726
gscalderon wants to merge 4 commits intoopen-telemetry:mainfrom
gscalderon:gscalderon/perf/zero-copy-otlp-bytes-adapter

Conversation

@gscalderon
Copy link
Copy Markdown

Replace BinaryArray::from_vec() which deep-copies the entire OTLP payload with a zero-copy construction using Buffer::from(bytes::Bytes). The clone_bytes() call is just an Arc refcount bump, and Buffer::from(Bytes) wraps the data without copying, eliminating a full memcpy on the ingest hot path.

Change Summary

  • Zero-copy wrapping: Use Buffer::from(bytes::Bytes) instead of BinaryArray::from_vec() to avoid deep-copying the OTLP payload into Arrow.
  • Bounds check: Added explicit i32::try_from guard on payload length for a clear error on oversized payloads.
  • New tests: test_otlp_bytes_adapter_zero_copy (pointer equality assertion) and test_otlp_bytes_adapter_empty_payload.

What issue does this PR close?

How are these changes tested?

  • Existing tests (test_otlp_bytes_adapter, test_extract_otlp_bytes) verify correctness is preserved.
  • New test_otlp_bytes_adapter_zero_copy asserts the Arrow buffer points to the same memory as the original OtlpProtoBytes (no copy).
  • New test_otlp_bytes_adapter_empty_payload verifies empty payload handling.

Are there any user-facing changes?

No. This is a transparent performance improvement with no API or behavior changes.

@gscalderon gscalderon requested a review from a team as a code owner April 21, 2026 22:51
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Apr 21, 2026

CLA Signed

The committers listed above are authorized under a signed CLA.

@github-actions github-actions Bot added the rust Pull requests that update Rust code label Apr 21, 2026
Replace BinaryArray::from_vec() which deep-copies the entire OTLP payload
with a zero-copy construction using Buffer::from(bytes::Bytes). The
clone_bytes() call is just an Arc refcount bump, and Buffer::from(Bytes)
wraps the data without copying, eliminating a full memcpy on the ingest
hot path.

Fixes open-telemetry#2703

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@gscalderon gscalderon force-pushed the gscalderon/perf/zero-copy-otlp-bytes-adapter branch from 338905e to 7169a58 Compare April 21, 2026 22:56
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

❌ Patch coverage is 76.47059% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.87%. Comparing base (ebf2403) to head (3fe0fc3).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2726   +/-   ##
=======================================
  Coverage   87.87%   87.87%           
=======================================
  Files         638      638           
  Lines      244608   244641   +33     
=======================================
+ Hits       214942   214972   +30     
- Misses      29142    29145    +3     
  Partials      524      524           
Components Coverage Δ
otap-dataflow 89.90% <76.47%> (+<0.01%) ⬆️
query_abstraction 80.61% <ø> (ø)
query_engine 90.74% <ø> (ø)
otel-arrow-go 51.92% <ø> (ø)
quiver 92.27% <ø> (ø)
🚀 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

@AaronRM AaronRM left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks @gscalderon !

Copy link
Copy Markdown
Member

@lalitb lalitb left a comment

Choose a reason for hiding this comment

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

nice.

@jmacd jmacd enabled auto-merge April 22, 2026 18:34
Copy link
Copy Markdown
Contributor

@utpilla utpilla left a comment

Choose a reason for hiding this comment

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

Welcome and thanks for the first PR! 🎉

Use higher-level BinaryArray::new(offsets, data_buffer, None) instead of
ArrayData::builder, reducing boilerplate while preserving zero-copy behavior.
auto-merge was automatically disabled April 22, 2026 22:54

Head branch was pushed to by a user without write access

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rust Pull requests that update Rust code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

perf: [durable_buffer] Avoid unnecessary copy of OTLP bytes in OtlpBytesAdapter::new()

5 participants