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
Use sync.Pools for ResourceLogs in otelloghttp transforms #5268
base: main
Are you sure you want to change the base?
Conversation
e4f5cfa
to
646a771
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5268 +/- ##
======================================
Coverage 84.3% 84.4%
======================================
Files 258 265 +7
Lines 17042 17483 +441
======================================
+ Hits 14382 14759 +377
- Misses 2361 2413 +52
- Partials 299 311 +12
|
Please show how the performance is improved with this change with benchmarks. |
LogRecords: pbLogRecords[2:], | ||
}}, | ||
}} | ||
out, free = ResourceLogs(records[2:]) |
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.
free
is never called here.
@@ -18,18 +19,26 @@ import ( | |||
"go.opentelemetry.io/otel/sdk/log" | |||
) | |||
|
|||
// ResourceLogs returns an slice of OTLP ResourceLogs generated from records. | |||
func ResourceLogs(records []log.Record) []*lpb.ResourceLogs { | |||
var resourceLogsPool = sync.Pool{ |
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.
These pool values escape this package. It is likely a better design to have pooled values passed to ResourceLogs
instead so leaks are not incurred.
I used
sync.Pools
to pool the slice[]*lpb.ResourceLogs
. The functionfree
, whichResourceLogs
returns, puts the slice into the pools.Also, I updated the test
TestResourceLogs
for the data integrity.I referred to https://github.com/open-telemetry/opentelemetry-go-contrib/blob/59ebbee5c207bb6034462a75da61b79668fe9232/bridges/otelslog/handler.go#L346