Skip to content
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 in otlplog transforms #5196

Open
MrAlias opened this issue Apr 11, 2024 · 4 comments
Open

Use sync.Pools in otlplog transforms #5196

MrAlias opened this issue Apr 11, 2024 · 4 comments
Labels
area:logs Part of OpenTelemetry logs pkg:exporter:otlp Related to the OTLP exporter package

Comments

@MrAlias
Copy link
Contributor

MrAlias commented Apr 11, 2024

Could we use sync.Pools to reduce the amount of heap allocation?
E.g. ResourceLogs could return a functions that would return the pooled maps and slices back to their pools.

If you think that it is possible and it is a good idea then this should be tracked as a separate issue.

Originally posted by @pellared in #5191 (comment)

@MrAlias MrAlias added area:logs Part of OpenTelemetry logs pkg:exporter:otlp Related to the OTLP exporter package labels Apr 11, 2024
@hiroyaonoe
Copy link

I want to work on this!
Should I update ScopeLogs as well?

func ScopeLogs(records []log.Record) []*lpb.ScopeLogs {

@MrAlias
Copy link
Contributor Author

MrAlias commented Apr 18, 2024

I want to work on this! Should I update ScopeLogs as well?

func ScopeLogs(records []log.Record) []*lpb.ScopeLogs {

I would start by only pooling the maps.

@hiroyaonoe hiroyaonoe removed their assignment May 9, 2024
@MrAlias MrAlias self-assigned this May 17, 2024
pellared pushed a commit that referenced this issue May 22, 2024
Part of #5196 

### Benchmarks

```console
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp/internal/transform
cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
               │   old.txt   │               new.txt               │
               │   sec/op    │   sec/op     vs base                │
ResourceLogs-8   6.033µ ± 4%   5.249µ ± 8%  -13.00% (p=0.000 n=10)

               │    old.txt    │               new.txt                │
               │     B/op      │     B/op      vs base                │
ResourceLogs-8   10.602Ki ± 0%   8.299Ki ± 0%  -21.72% (p=0.000 n=10)

               │  old.txt   │              new.txt              │
               │ allocs/op  │ allocs/op   vs base               │
ResourceLogs-8   188.0 ± 0%   178.0 ± 0%  -5.32% (p=0.000 n=10)
```
@MrAlias MrAlias removed their assignment May 30, 2024
@MrAlias
Copy link
Contributor Author

MrAlias commented Jul 18, 2024

This needs to be applied at to the template to address both HTTP and gRPC: https://github.com/open-telemetry/opentelemetry-go/tree/main/internal/shared/otlp/otlplog/transform

@MrAlias MrAlias changed the title Use sync.Pools in otlploghttp transforms Use sync.Pools in otlplog transforms Jul 18, 2024
@kenanfarukcakir
Copy link

kenanfarukcakir commented Aug 15, 2024

Hi @MrAlias, I am working on an application that collects logs from a kubernetes node and we plan to export these logs to OpenTelemetry Collector using go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp. When the log frequency increases, I've observed a dramatic increase in the cpu usage of my application.

Got the cpu profile, and it seems related to this issue.
Screenshot 2024-08-15 at 14 43 34
Screenshot 2024-08-15 at 14 43 52

Seems like too much allocation is made and that keeps the GC under pressure.
I'd like to work on this and appreciate any direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:logs Part of OpenTelemetry logs pkg:exporter:otlp Related to the OTLP exporter package
Projects
Status: Backlog
Development

No branches or pull requests

3 participants