Skip to content

Commit d145fae

Browse files
committed
buildctl: make trace shutdown best effort
Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com>
1 parent 2d85d54 commit d145fae

2 files changed

Lines changed: 102 additions & 1 deletion

File tree

cmd/buildctl/common/trace.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/moby/buildkit/util/tracing/detect"
1111
"github.com/pkg/errors"
1212
"github.com/urfave/cli/v3"
13+
"go.opentelemetry.io/otel"
1314
"go.opentelemetry.io/otel/attribute"
1415
"go.opentelemetry.io/otel/codes"
1516
sdktrace "go.opentelemetry.io/otel/sdk/trace"
@@ -85,7 +86,10 @@ func AttachAppContext(app *cli.Command) error {
8586
ctx, cancel := context.WithTimeoutCause(appcontext.Shutdown(), exportTimeout, errors.WithStack(context.DeadlineExceeded))
8687
defer cancel()
8788

88-
return tp.Shutdown(ctx)
89+
if err := tp.Shutdown(ctx); err != nil {
90+
otel.Handle(err)
91+
}
92+
return nil
8993
}
9094
return nil
9195
}

cmd/buildctl/common/trace_test.go

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
package common
2+
3+
import (
4+
"context"
5+
"errors"
6+
"testing"
7+
8+
"github.com/moby/buildkit/util/tracing/detect"
9+
"github.com/stretchr/testify/require"
10+
"github.com/urfave/cli/v3"
11+
sdktrace "go.opentelemetry.io/otel/sdk/trace"
12+
)
13+
14+
var errTraceShutdown = errors.New("trace shutdown failed")
15+
16+
func TestTraceShutdown(t *testing.T) {
17+
for _, tc := range []struct {
18+
name string
19+
exp sdktrace.SpanExporter
20+
}{
21+
{
22+
name: "deadline",
23+
exp: shutdownDeadlineExporter{},
24+
},
25+
{
26+
name: "error",
27+
exp: shutdownErrorExporter{},
28+
},
29+
} {
30+
t.Run(tc.name, func(t *testing.T) {
31+
withTraceExporter(t, tc.name, tc.exp)
32+
33+
app := traceTestApp(t, nil)
34+
require.NoError(t, app.Run(context.Background(), []string{"buildctl", "noop"}))
35+
})
36+
}
37+
}
38+
39+
func TestTraceAfterError(t *testing.T) {
40+
withTraceExporter(t, "noop", shutdownErrorExporter{})
41+
42+
errAfter := errors.New("after failed")
43+
app := traceTestApp(t, func(context.Context, *cli.Command) error {
44+
return errAfter
45+
})
46+
require.ErrorIs(t, app.Run(context.Background(), []string{"buildctl", "noop"}), errAfter)
47+
}
48+
49+
func withTraceExporter(t *testing.T, name string, exp sdktrace.SpanExporter) {
50+
t.Helper()
51+
52+
exporterName := "buildctl-common-test-" + name
53+
detect.Register(exporterName, detect.TraceExporterDetector(func() (sdktrace.SpanExporter, error) {
54+
return exp, nil
55+
}), 0)
56+
t.Setenv("OTEL_TRACES_EXPORTER", exporterName)
57+
}
58+
59+
func traceTestApp(t *testing.T, after cli.AfterFunc) *cli.Command {
60+
t.Helper()
61+
62+
app := &cli.Command{
63+
Name: "buildctl",
64+
After: after,
65+
Commands: []*cli.Command{
66+
{
67+
Name: "noop",
68+
Action: func(context.Context, *cli.Command) error {
69+
return nil
70+
},
71+
},
72+
},
73+
}
74+
require.NoError(t, AttachAppContext(app))
75+
return app
76+
}
77+
78+
type shutdownDeadlineExporter struct{}
79+
80+
func (shutdownDeadlineExporter) ExportSpans(context.Context, []sdktrace.ReadOnlySpan) error {
81+
return nil
82+
}
83+
84+
func (shutdownDeadlineExporter) Shutdown(ctx context.Context) error {
85+
<-ctx.Done()
86+
return context.Cause(ctx)
87+
}
88+
89+
type shutdownErrorExporter struct{}
90+
91+
func (shutdownErrorExporter) ExportSpans(context.Context, []sdktrace.ReadOnlySpan) error {
92+
return nil
93+
}
94+
95+
func (shutdownErrorExporter) Shutdown(context.Context) error {
96+
return errTraceShutdown
97+
}

0 commit comments

Comments
 (0)