detect: refactor the detect package

This refactors the detect package with the goal of making it more
similar to otel's `autoexport` package and splitting out the additional
functionality used by buildkit, like the trace recorder and delegated
tracer, to more explicit processors rather than implicit through
`autoexport`.

This removes the global variables for the trace provider and meter
provider along with the global variable for the exporters. This is
replaced with functions that create the exporters. The delegated tracer
has been removed from detect and moved into the normal tracing util
package. This is still used by the command line to send delegated
traces, but it's an explicit exporter that's added rather than implicit.

Some functions have been renamed mostly to force dependent packages to
change their usage rather than have a chance at incorrect usage because
the semantics changed.

Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>

cmd/buildctl/common/common.go

@@ -12,7 +12,7 @@ import (
 	"time"
 
 	"github.com/moby/buildkit/client"
-	"github.com/moby/buildkit/util/tracing/detect"
+	"github.com/moby/buildkit/util/tracing/delegated"
 	"github.com/pkg/errors"
 	"github.com/urfave/cli"
 	"go.opentelemetry.io/otel/trace"
@@ -68,16 +68,10 @@ func ResolveClient(c *cli.Context) (*client.Client, error) {
 	ctx := CommandContext(c)
 	var opts []client.ClientOpt
 	if span := trace.SpanFromContext(ctx); span.SpanContext().IsValid() {
-		opts = append(opts, client.WithTracerProvider(span.TracerProvider()))
-
-		exp, _, err := detect.Exporter()
-		if err != nil {
-			return nil, err
-		}
-
-		if td, ok := exp.(client.TracerDelegate); ok {
-			opts = append(opts, client.WithTracerDelegate(td))
-		}
+		opts = append(opts,
+			client.WithTracerProvider(span.TracerProvider()),
+			client.WithTracerDelegate(delegated.DefaultExporter),
+		)
 	}
 
 	if caCert != "" {

cmd/buildctl/common/trace.go

@@ -5,20 +5,28 @@ import (
 	"os"
 
 	"github.com/moby/buildkit/util/appcontext"
+	"github.com/moby/buildkit/util/tracing/delegated"
 	"github.com/moby/buildkit/util/tracing/detect"
 	"github.com/urfave/cli"
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
+	sdktrace "go.opentelemetry.io/otel/sdk/trace"
 	"go.opentelemetry.io/otel/trace"
 )
 
 func AttachAppContext(app *cli.App) error {
 	ctx := appcontext.Context()
 
-	tp, err := detect.TracerProvider()
+	exp, err := detect.NewSpanExporter(ctx)
 	if err != nil {
 		return err
 	}
+
+	tp := sdktrace.NewTracerProvider(
+		sdktrace.WithResource(detect.Resource()),
+		sdktrace.WithBatcher(exp),
+		sdktrace.WithBatcher(delegated.DefaultExporter),
+	)
 	tracer := tp.Tracer("")
 
 	var span trace.Span
@@ -60,7 +68,7 @@ func AttachAppContext(app *cli.App) error {
 		if span != nil {
 			span.End()
 		}
-		return detect.Shutdown(context.TODO())
+		return tp.Shutdown(context.TODO())
 	}
 	return nil
 }

cmd/buildctl/main.go

@@ -16,7 +16,6 @@ import (
 	"github.com/moby/buildkit/util/appdefaults"
 	"github.com/moby/buildkit/util/profiler"
 	"github.com/moby/buildkit/util/stack"
-	_ "github.com/moby/buildkit/util/tracing/detect/delegated"
 	_ "github.com/moby/buildkit/util/tracing/detect/jaeger"
 	_ "github.com/moby/buildkit/util/tracing/env"
 	"github.com/moby/buildkit/version"

cmd/buildkitd/main.go

@@ -23,6 +23,7 @@ import (
 	sddaemon "github.com/coreos/go-systemd/v22/daemon"
 	"github.com/docker/docker/pkg/reexec"
 	"github.com/gofrs/flock"
+	"github.com/hashicorp/go-multierror"
 	"github.com/moby/buildkit/cache/remotecache"
 	"github.com/moby/buildkit/cache/remotecache/azblob"
 	"github.com/moby/buildkit/cache/remotecache/gha"
@@ -63,7 +64,9 @@ import (
 	"github.com/urfave/cli"
 	"go.etcd.io/bbolt"
 	"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
+	"go.opentelemetry.io/otel/exporters/prometheus"
 	"go.opentelemetry.io/otel/propagation"
+	sdkmetric "go.opentelemetry.io/otel/sdk/metric"
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
 	tracev1 "go.opentelemetry.io/proto/otlp/collector/trace/v1"
 	"golang.org/x/sync/errgroup"
@@ -217,6 +220,7 @@ func main() {
 	app.Flags = append(app.Flags, appFlags...)
 	app.Flags = append(app.Flags, serviceFlags()...)
 
+	var closers []func(ctx context.Context) error
 	app.Action = func(c *cli.Context) error {
 		// TODO: On Windows this always returns -1. The actual "are you admin" check is very Windows-specific.
 		// See https://github.com/golang/go/issues/28804#issuecomment-505326268 for the "short" version.
@@ -259,15 +263,17 @@ func main() {
 			}
 		}
 
-		tp, err := detect.TracerProvider()
+		tp, err := newTracerProvider(ctx)
 		if err != nil {
 			return err
 		}
+		closers = append(closers, tp.Shutdown)
 
-		mp, err := detect.MeterProvider()
+		mp, err := newMeterProvider(ctx)
 		if err != nil {
 			return err
 		}
+		closers = append(closers, mp.Shutdown)
 
 		statsHandler := tracing.ServerStatsHandler(
 			otelgrpc.WithTracerProvider(tp),
@@ -375,8 +381,13 @@ func main() {
 		return err
 	}
 
-	app.After = func(_ *cli.Context) error {
-		return detect.Shutdown(context.TODO())
+	app.After = func(_ *cli.Context) (err error) {
+		for _, c := range closers {
+			if e := c(context.TODO()); e != nil {
+				err = multierror.Append(err, e)
+			}
+		}
+		return err
 	}
 
 	profiler.Attach(app)
@@ -737,13 +748,19 @@ func newController(c *cli.Context, cfg *config.Config) (*control.Controller, err
 		return nil, err
 	}
 
-	tc, _, err := detect.Exporter()
-	if err != nil {
+	tc := make(tracing.MultiSpanExporter, 0, 2)
+	if detect.Recorder != nil {
+		tc = append(tc, detect.Recorder)
+	}
+
+	if exp, err := detect.NewSpanExporter(context.TODO()); err != nil {
 		return nil, err
+	} else if !detect.IsNoneSpanExporter(exp) {
+		tc = append(tc, exp)
 	}
 
 	var traceSocket string
-	if tc != nil {
+	if len(tc) > 0 {
 		traceSocket = cfg.OTEL.SocketPath
 		if err := runTraceController(traceSocket, tc); err != nil {
 			return nil, err
@@ -942,9 +959,45 @@ type traceCollector struct {
 }
 
 func (t *traceCollector) Export(ctx context.Context, req *tracev1.ExportTraceServiceRequest) (*tracev1.ExportTraceServiceResponse, error) {
-	err := t.exporter.ExportSpans(ctx, transform.Spans(req.GetResourceSpans()))
-	if err != nil {
+	if err := t.exporter.ExportSpans(ctx, transform.Spans(req.GetResourceSpans())); err != nil {
 		return nil, err
 	}
 	return &tracev1.ExportTraceServiceResponse{}, nil
 }
+
+func newTracerProvider(ctx context.Context) (*sdktrace.TracerProvider, error) {
+	opts := []sdktrace.TracerProviderOption{
+		sdktrace.WithResource(detect.Resource()),
+		sdktrace.WithSyncer(detect.Recorder),
+	}
+
+	if exp, err := detect.NewSpanExporter(ctx); err != nil {
+		return nil, err
+	} else if !detect.IsNoneSpanExporter(exp) {
+		opts = append(opts, sdktrace.WithBatcher(exp))
+	}
+	return sdktrace.NewTracerProvider(opts...), nil
+}
+
+func newMeterProvider(ctx context.Context) (*sdkmetric.MeterProvider, error) {
+	opts := []sdkmetric.Option{
+		sdkmetric.WithResource(detect.Resource()),
+	}
+
+	if r, err := prometheus.New(); err != nil {
+		// Log the error but do not fail if we could not configure the prometheus metrics.
+		bklog.G(context.Background()).
+			WithError(err).
+			Error("failed prometheus metrics configuration")
+	} else {
+		opts = append(opts, sdkmetric.WithReader(r))
+	}
+
+	if exp, err := detect.NewMetricExporter(ctx); err != nil {
+		return nil, err
+	} else if !detect.IsNoneMetricExporter(exp) {
+		r := sdkmetric.NewPeriodicReader(exp)
+		opts = append(opts, sdkmetric.WithReader(r))
+	}
+	return sdkmetric.NewMeterProvider(opts...), nil
+}

go.mod

@@ -85,7 +85,6 @@ require (
 	go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.21.0
 	go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.21.0
 	go.opentelemetry.io/otel/exporters/prometheus v0.42.0
-	go.opentelemetry.io/otel/metric v1.21.0
 	go.opentelemetry.io/otel/sdk v1.21.0
 	go.opentelemetry.io/otel/sdk/metric v1.21.0
 	go.opentelemetry.io/otel/trace v1.21.0
@@ -165,6 +164,7 @@ require (
 	github.com/vishvananda/netns v0.0.4 // indirect
 	go.opencensus.io v0.24.0 // indirect
 	go.opentelemetry.io/otel/exporters/otlp/otlpmetric v0.42.0 // indirect
+	go.opentelemetry.io/otel/metric v1.21.0 // indirect
 	golang.org/x/text v0.14.0 // indirect
 	golang.org/x/tools v0.14.0 // indirect
 	google.golang.org/genproto v0.0.0-20231016165738-49dd2c1f3d0b // indirect

util/tracing/detect/delegated/delegated.go → util/tracing/delegated/delegated.go

@@ -4,27 +4,24 @@ import (
 	"context"
 	"sync"
 
-	"github.com/moby/buildkit/util/tracing/detect"
+	"github.com/moby/buildkit/client"
 	sdktrace "go.opentelemetry.io/otel/sdk/trace"
 )
 
 const maxBuffer = 256
 
-var exp = &Exporter{}
-
-func init() {
-	detect.Register("delegated", detect.TraceExporterDetector(func() (sdktrace.SpanExporter, error) {
-		return exp, nil
-	}), 100)
-}
+var DefaultExporter = &Exporter{}
 
 type Exporter struct {
 	mu        sync.Mutex
 	exporters []sdktrace.SpanExporter
 	buffer    []sdktrace.ReadOnlySpan
 }
 
-var _ sdktrace.SpanExporter = &Exporter{}
+var (
+	_ sdktrace.SpanExporter = (*Exporter)(nil)
+	_ client.TracerDelegate = (*Exporter)(nil)
+)
 
 func (e *Exporter) ExportSpans(ctx context.Context, ss []sdktrace.ReadOnlySpan) error {
 	e.mu.Lock()