Skip to content

Commit 12ff8cc

Browse files
committed
tracing: fix buildx tracing delegation
before this change, the threadsafe wrapper would hide the delegate interface, which breaks the ability to send traces. after this change, tracing works again! fixes docker/buildx#1847 Signed-off-by: Nick Santos <[email protected]>
1 parent 61e6f28 commit 12ff8cc

File tree

4 files changed

+64
-4
lines changed

4 files changed

+64
-4
lines changed

util/tracing/detect/delegated/delegated.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package jaeger
1+
package delegated
22

33
import (
44
"context"
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package delegated_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/moby/buildkit/client"
7+
"github.com/moby/buildkit/util/tracing/detect"
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
func TestDetectPreservesDelegateInterface(t *testing.T) {
13+
exp, err := detect.Exporter()
14+
require.NoError(t, err)
15+
16+
_, ok := exp.(client.TracerDelegate)
17+
assert.True(t, ok, "delegated tracer expected to fulfill client.TracerDelegate interface")
18+
}

util/tracing/detect/detect.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,7 @@ func getExporter() (sdktrace.SpanExporter, error) {
8080
}
8181

8282
if exp != nil {
83-
exp = &threadSafeExporterWrapper{
84-
exporter: exp,
85-
}
83+
exp = makeThreadSafe(exp)
8684
}
8785

8886
if Recorder != nil {

util/tracing/detect/threadsafe.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,27 @@ import (
77
sdktrace "go.opentelemetry.io/otel/sdk/trace"
88
)
99

10+
// Duplicated from buildkit/client.
11+
// Part of the external interface of our tracer.
12+
// Needed to ensure that we maintain this interface.
13+
type tracerDelegate interface {
14+
sdktrace.SpanExporter
15+
16+
SetSpanExporter(ctx context.Context, exp sdktrace.SpanExporter) error
17+
}
18+
19+
func makeThreadSafe(exp sdktrace.SpanExporter) sdktrace.SpanExporter {
20+
d, isDelegate := exp.(tracerDelegate)
21+
if isDelegate {
22+
return &threadSafeDelegateWrapper{
23+
exporter: d,
24+
}
25+
}
26+
return &threadSafeExporterWrapper{
27+
exporter: exp,
28+
}
29+
}
30+
1031
// threadSafeExporterWrapper wraps an OpenTelemetry SpanExporter and makes it thread-safe.
1132
type threadSafeExporterWrapper struct {
1233
mu sync.Mutex
@@ -24,3 +45,26 @@ func (tse *threadSafeExporterWrapper) Shutdown(ctx context.Context) error {
2445
defer tse.mu.Unlock()
2546
return tse.exporter.Shutdown(ctx)
2647
}
48+
49+
type threadSafeDelegateWrapper struct {
50+
mu sync.Mutex
51+
exporter tracerDelegate
52+
}
53+
54+
func (tse *threadSafeDelegateWrapper) ExportSpans(ctx context.Context, spans []sdktrace.ReadOnlySpan) error {
55+
tse.mu.Lock()
56+
defer tse.mu.Unlock()
57+
return tse.exporter.ExportSpans(ctx, spans)
58+
}
59+
60+
func (tse *threadSafeDelegateWrapper) Shutdown(ctx context.Context) error {
61+
tse.mu.Lock()
62+
defer tse.mu.Unlock()
63+
return tse.exporter.Shutdown(ctx)
64+
}
65+
66+
func (tse *threadSafeDelegateWrapper) SetSpanExporter(ctx context.Context, exp sdktrace.SpanExporter) error {
67+
tse.mu.Lock()
68+
defer tse.mu.Unlock()
69+
return tse.exporter.SetSpanExporter(ctx, exp)
70+
}

0 commit comments

Comments
 (0)