Skip to content

buildctl: ignore trace shutdown deadline#6918

Closed
crazy-max wants to merge 1 commit into
moby:masterfrom
crazy-max:fix-buildctl-trace-shutdown
Closed

buildctl: ignore trace shutdown deadline#6918
crazy-max wants to merge 1 commit into
moby:masterfrom
crazy-max:fix-buildctl-trace-shutdown

Conversation

@crazy-max

@crazy-max crazy-max commented Jul 1, 2026

Copy link
Copy Markdown
Member

fixes #6912

Trace shutdown runs after the command action has already completed. The existing 50ms shutdown context can expire while the OpenTelemetry batch processor is flushing delegated spans over a slow connhelper transport.

Ignore only context deadline errors from tracer shutdown so telemetry cleanup cannot turn a successful command into a non-zero exit. Continue returning other tracer shutdown errors and existing app.After errors.

@crazy-max crazy-max added this to the v0.32.0 milestone Jul 1, 2026
@crazy-max crazy-max force-pushed the fix-buildctl-trace-shutdown branch from 91d4be6 to d145fae Compare July 1, 2026 15:38
@crazy-max crazy-max requested a review from tonistiigi July 1, 2026 15:39
@crazy-max crazy-max marked this pull request as ready for review July 1, 2026 15:39
@crazy-max crazy-max force-pushed the fix-buildctl-trace-shutdown branch from d145fae to cd7a223 Compare July 2, 2026 13:23
@crazy-max crazy-max changed the title buildctl: make trace shutdown best effort buildctl: ignore trace shutdown deadline Jul 2, 2026
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
@crazy-max crazy-max force-pushed the fix-buildctl-trace-shutdown branch from cd7a223 to cdda59c Compare July 3, 2026 13:48

@tonistiigi tonistiigi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this needs quick unblock (not sure how serious issue), we can include this. But regarding the actual issue I don't think this is right. #6757 that added this doesn't look correct at all and we could consider reverting it if there isn't a quick solution. The 50ms hardcoded timeout should never be applied in buildctl delegated traces. This has nothing to do with "stuck OTEL provider" case that the original issue for #6757 was pointing to. We should always prioritize saving delegated traces instead of dropping them with some timeout. If sending delegated traces fails then this also means build failure and nonzero exit code.

Reading #6747 again, I'm not even sure why we took this on. Why are we trying to guess with some crazy hack how BuildKit is supposed to ignore errors in misconfigured user setup.

@crazy-max crazy-max removed this from the v0.32.0 milestone Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

2 participants