Skip to content

redis parser: avoid string conversions and reduce allocations on hot path#1481

Merged
mariomac merged 3 commits into
open-telemetry:mainfrom
rafaelroquetto:lb_redis
Mar 6, 2026
Merged

redis parser: avoid string conversions and reduce allocations on hot path#1481
mariomac merged 3 commits into
open-telemetry:mainfrom
rafaelroquetto:lb_redis

Conversation

@rafaelroquetto
Copy link
Copy Markdown
Contributor

Reduce unnecessary allocations and string conversions in the Redis
detection pipeline:

  • parseRedisRequest now takes []byte instead of string, eliminating
    the buffer-to-string copy at both TCP call sites
    (string(requestBuffer.UnsafeView())). The Go Redis uprobe path also
    drops the string(event.Buf[:]) wrapper.

  • split.Iterator is made generic (Iterator[T string | []byte]) with
    two typed constructors: NewStringIterator and NewBytesIterator. The
    bytes variant is used by parseRedisRequest to iterate lines without
    any intermediate allocation. All existing string call sites are updated
    to NewStringIterator.

  • redisErrorCodes [...]string replaced with a redisErrors struct array
    pairing each []byte prefix with its pre-trimmed string code,
    eliminating the per-iteration []byte(code) conversion in
    getRedisError. Description is now trimmed before string conversion
    (string(bytes.Trim(...))), reducing the size of the allocation.

  • getRedisDB replaces strings.ToUpper with strings.EqualFold for
    the SELECT/QUIT check, avoiding an allocation for the uppercased copy.

Checklist

@rafaelroquetto rafaelroquetto requested a review from a team as a code owner March 5, 2026 21:10
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 5, 2026

Codecov Report

❌ Patch coverage is 88.57143% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.99%. Comparing base (db77da0) to head (7a34330).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
pkg/ebpf/common/redis_detect_transform.go 87.50% 1 Missing and 1 partial ⚠️
pkg/ebpf/common/tcp_detect_transform.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1481      +/-   ##
==========================================
+ Coverage   43.98%   43.99%   +0.01%     
==========================================
  Files         313      313              
  Lines       34353    34354       +1     
==========================================
+ Hits        15110    15115       +5     
+ Misses      18260    18255       -5     
- Partials      983      984       +1     
Flag Coverage Δ
integration-test 21.47% <0.00%> (-0.01%) ⬇️
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.30% <0.00%> (+<0.01%) ⬆️
oats-test 0.00% <0.00%> (ø)
unittests 45.01% <88.57%> (+<0.01%) ⬆️

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

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR optimizes the Redis detection/parsing hot path by reducing allocations and avoiding unnecessary string conversions, while extending the internal split iterator to support both string and []byte inputs.

Changes:

  • Updated Redis request parsing to operate on []byte end-to-end (TCP and Go uprobe paths), avoiding buffer-to-string copies.
  • Refactored split.Iterator into a generic iterator with NewStringIterator and NewBytesIterator, and updated all call sites accordingly.
  • Optimized Redis error extraction and DB selection logic to reduce per-iteration conversions/allocations.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/internal/split/splititerator.go Makes Iterator generic over string/[]byte and adds typed constructors.
pkg/internal/split/splititerator_test.go Updates string iterator tests and adds coverage for the new bytes iterator.
pkg/export/attributes/env.go Switches env var parsing to NewStringIterator.
pkg/ebpf/common/http/sqlpp.go Switches SQL++ parsing helpers to NewStringIterator.
pkg/ebpf/common/tcp_detect_transform.go Removes request/response string(...) conversions when parsing Redis frames.
pkg/ebpf/common/redis_detect_transform.go Updates Redis parsing to consume []byte, refactors error-prefix handling, and removes ToUpper allocation.
pkg/ebpf/common/redis_detect_transform_test.go Updates Redis parsing tests to pass []byte inputs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread pkg/ebpf/common/redis_detect_transform.go Outdated
Comment thread pkg/ebpf/common/redis_detect_transform.go Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread pkg/internal/split/splititerator.go
Comment thread pkg/internal/split/splititerator.go
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

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.

🚀

@mariomac mariomac merged commit 42381a5 into open-telemetry:main Mar 6, 2026
66 of 74 checks passed
@rafaelroquetto rafaelroquetto deleted the lb_redis branch March 6, 2026 15:16
@MrAlias MrAlias mentioned this pull request Mar 6, 2026
@MrAlias MrAlias added this to the v0.7.0 milestone Mar 16, 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.

4 participants