-
Notifications
You must be signed in to change notification settings - Fork 502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid vec allocation during each export for BatchLogProcessor - Part 2 #2488
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2488 +/- ##
=====================================
Coverage 77.8% 77.8%
=====================================
Files 123 123
Lines 22867 22887 +20
=====================================
+ Hits 17804 17824 +20
Misses 5063 5063 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay to special case to let BatchProcessor avoid alloc. Given no public api change, lets merge.
For batch export, we create an additional vec just to hold the references to
LogRecord
andInstrumentationScope
. This is needed because the constructor forLogBatch
only accepts a slice of tuples that are references toLogRecord
andInstrumentationScope
.BatchLogProcessor
already has a vec of ownedLogRecord
andInstrumentationScope
. This PR is an attempt to reuse that vec.Changes
LogBatch
which accepts a shared reference to a slice of boxed tuple of ownedLogRecord
andInstrumentationScope
.Merge requirement checklist
CHANGELOG.md
files updated for non-trivial, user-facing changes