buildctl: ignore trace shutdown deadline#6918
Conversation
91d4be6 to
d145fae
Compare
d145fae to
cd7a223
Compare
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
cd7a223 to
cdda59c
Compare
tonistiigi
left a comment
There was a problem hiding this comment.
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.
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.