-
Notifications
You must be signed in to change notification settings - Fork 542
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
otelslog is not reusing the pooled buffer #5879
Labels
Milestone
Comments
pellared
added
bug
Something isn't working
bridge: slog
Related to the slog bridge
labels
Jul 9, 2024
This was referenced Jul 9, 2024
pellared
added a commit
to open-telemetry/opentelemetry-go
that referenced
this issue
Jul 11, 2024
Add comment which is already in `BytesValue`, `SliceValue`, `MapValue`. Maybe it would help mitigating issues like open-telemetry/opentelemetry-go-contrib#5879.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I noticed that this
opentelemetry-go-contrib/bridges/otelslog/handler.go
Line 357 in c80e464
"kills" the whole optimization gained from usage of
sync.Pool
becausebuf.data[:0:0]
makesbuf.data
to never be really reused.Demo: #5878
At the same time changing to
buf.data[:0]
does not work because of the recursive nature of the conversion.We would need to call
free()
when everything is converted - e.g. between these lines:opentelemetry-go-contrib/bridges/otelslog/handler.go
Lines 164 to 165 in c80e464
The other idea is to only use the pool for "top level attributes" (only in
convertRecord
).However, the
Logger.Emit
documentation says:This would mean that we cannot pool the attributes as it can be held by the implementation.
The text was updated successfully, but these errors were encountered: