Skip to content

Commit e468373

Browse files
authored
Merge pull request moby#3909 from nicks/nicks/tracing
tracing: fix buildx tracing delegation
2 parents 61e6f28 + 9fd591a commit e468373

File tree

5 files changed

+48
-34
lines changed

5 files changed

+48
-34
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: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,6 @@ func getExporter() (sdktrace.SpanExporter, error) {
7979
return nil, err
8080
}
8181

82-
if exp != nil {
83-
exp = &threadSafeExporterWrapper{
84-
exporter: exp,
85-
}
86-
}
87-
8882
if Recorder != nil {
8983
Recorder.SpanExporter = exp
9084
exp = Recorder

util/tracing/detect/jaeger/jaeger.go

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package jaeger
22

33
import (
4+
"context"
45
"net"
56
"os"
67
"strings"
8+
"sync"
79

810
"github.com/moby/buildkit/util/tracing/detect"
911
"go.opentelemetry.io/otel/exporters/jaeger"
@@ -48,7 +50,14 @@ func jaegerExporter() (sdktrace.SpanExporter, error) {
4850
epo = jaeger.WithAgentEndpoint(jaeger.WithAgentHost(host), jaeger.WithAgentPort(port))
4951
}
5052

51-
return jaeger.New(epo)
53+
exp, err := jaeger.New(epo)
54+
if err != nil {
55+
return nil, err
56+
}
57+
58+
return &threadSafeExporterWrapper{
59+
exporter: exp,
60+
}, nil
5261
}
5362

5463
func envOr(key, defaultValue string) string {
@@ -57,3 +66,22 @@ func envOr(key, defaultValue string) string {
5766
}
5867
return defaultValue
5968
}
69+
70+
// We've received reports that the Jaeger exporter is not thread-safe,
71+
// so wrap it in a mutex.
72+
type threadSafeExporterWrapper struct {
73+
mu sync.Mutex
74+
exporter sdktrace.SpanExporter
75+
}
76+
77+
func (tse *threadSafeExporterWrapper) ExportSpans(ctx context.Context, spans []sdktrace.ReadOnlySpan) error {
78+
tse.mu.Lock()
79+
defer tse.mu.Unlock()
80+
return tse.exporter.ExportSpans(ctx, spans)
81+
}
82+
83+
func (tse *threadSafeExporterWrapper) Shutdown(ctx context.Context) error {
84+
tse.mu.Lock()
85+
defer tse.mu.Unlock()
86+
return tse.exporter.Shutdown(ctx)
87+
}

util/tracing/detect/threadsafe.go

Lines changed: 0 additions & 26 deletions
This file was deleted.

0 commit comments

Comments
 (0)