otel: Avoid excessive memory allocations if not configured#49078
otel: Avoid excessive memory allocations if not configured#49078cpuguy83 merged 3 commits intomoby:masterfrom
Conversation
b5bfa9c to
adbf9a2
Compare
|
FYI, looks like it doesn't need a 26.1 and 25.0 backport - the old buildkit's |
cmd/dockerd/daemon.go
Outdated
| log.G(ctx).WithError(err).Warn("Failed to initialize tracing, skipping") | ||
| } else if !detect.IsNoneSpanExporter(exp) { | ||
| opts = append(opts, sdktrace.WithBatcher(exp)) | ||
| return noop.NewTracerProvider(), nil |
There was a problem hiding this comment.
Curious, as the basic issue is quite similar; would it make sense to apply the workaround in the same location as we did for that previous OTEL memory leak, or is it not possible to set them up in the same location? (sorry didn't look in-depth, mostly considering that it would be nice to have such workarounds close together);
There was a problem hiding this comment.
This one is not really a workaround for the OTEL itself. It fixes our code which created and set a full-fledged tracer provider with no export target, rather than just NOT calling SetTracerProvider at all, or explicitly setting it to a noop provider.
adbf9a2 to
5089a4e
Compare
|
Hmmm, looks like this breaks EDIT: And With this PR, we're basically missing the Buildkit's which was attached to the tracer provider as a Line 399 in 5089a4e |
Use noop tracer provider if the OTEL exporter is not configured. This makes the OTEL tracing avoid doing unneeded memory allocations for spans which aren't going to be exported anywhere anyway. Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
5089a4e to
693c2f5
Compare
|
Missed a file / change somewhere @vvoland |
693c2f5 to
ce2a76f
Compare
Needed for Buildkit history Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
ce2a76f to
d8358eb
Compare
Use noop tracer provider if the OTEL exporter is not configured. This makes the OTEL tracing avoid doing unneeded memory allocations for spans which aren't going to be exported anywhere anyway.
- How to verify it
Before this patch, the RSS grows significantly over time, in proportion to the amount of running
docker logs -f.After the patch, the RSS stays around the same level.
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)