Merge pull request docker#2373 from jsternberg/docker-cli-meter-provider

metricutil: switch to using the cli meter provider

commands/build.go

@@ -266,11 +266,8 @@ func (o *buildOptionsHash) String() string {
 }
 
 func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions) (err error) {
-	mp, err := metricutil.NewMeterProvider(ctx, dockerCli)
-	if err != nil {
-		return err
-	}
-	defer mp.Report(context.Background())
+	mp := dockerCli.MeterProvider(ctx)
+	defer metricutil.Shutdown(ctx, mp)
 
 	ctx, end, err := tracing.TraceCurrentCommand(ctx, "build")
 	if err != nil {

go.mod

@@ -14,7 +14,7 @@ require (
 	github.com/containerd/typeurl/v2 v2.1.1
 	github.com/creack/pty v1.1.18
 	github.com/distribution/reference v0.5.0
-	github.com/docker/cli v26.0.0+incompatible
+	github.com/docker/cli v26.0.1-0.20240401150816-155dc5e4e406+incompatible
 	github.com/docker/cli-docs-tool v0.7.0
 	github.com/docker/docker v26.0.0+incompatible
 	github.com/docker/go-units v0.5.0
@@ -42,11 +42,11 @@ require (
 	github.com/tonistiigi/fsutil v0.0.0-20240301111122-7525a1af2bb5
 	github.com/zclconf/go-cty v1.14.1
 	go.opentelemetry.io/otel v1.21.0
-	go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v0.42.0
-	go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v0.42.0
+	go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v0.42.0 // indirect
+	go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v0.42.0 // indirect
 	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/sdk v1.21.0 // indirect
+	go.opentelemetry.io/otel/sdk/metric v1.21.0 // indirect
 	go.opentelemetry.io/otel/trace v1.21.0
 	golang.org/x/mod v0.14.0
 	golang.org/x/sync v0.6.0

go.sum

@@ -121,8 +121,8 @@ github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
 github.com/denisenkom/go-mssqldb v0.0.0-20191128021309-1d7a30a10f73/go.mod h1:xbL0rPBG9cCiLr28tMa8zpbdarY27NDyej4t/EjAShU=
 github.com/distribution/reference v0.5.0 h1:/FUIFXtfc/x2gpa5/VGfiGLuOIdYa1t65IKK2OFGvA0=
 github.com/distribution/reference v0.5.0/go.mod h1:BbU0aIcezP1/5jX/8MP0YiH4SdvB5Y4f/wlDRiLyi3E=
-github.com/docker/cli v26.0.0+incompatible h1:90BKrx1a1HKYpSnnBFR6AgDq/FqkHxwlUyzJVPxD30I=
-github.com/docker/cli v26.0.0+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8=
+github.com/docker/cli v26.0.1-0.20240401150816-155dc5e4e406+incompatible h1:+BQSfkbpbEDXXfHmFfmbfstUBFjhffsTkfa0iudOFLc=
+github.com/docker/cli v26.0.1-0.20240401150816-155dc5e4e406+incompatible/go.mod h1:JLrzqnKDaYBop7H2jaqPtU4hHvMKP+vjCwu2uszcLI8=
 github.com/docker/cli-docs-tool v0.7.0 h1:M2Da98Unz2kz3A5d4yeSGbhyOge2mfYSNjAFt01Rw0M=
 github.com/docker/cli-docs-tool v0.7.0/go.mod h1:zMjqTFCU361PRh8apiXzeAZ1Q/xupbIwTusYpzCXS/o=
 github.com/docker/distribution v2.7.1+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w=

util/metricutil/metric.go

@@ -2,218 +2,23 @@ package metricutil
 
 import (
 	"context"
-	"fmt"
-	"net/url"
-	"path"
-	"time"
 
-	"github.com/docker/buildx/util/confutil"
 	"github.com/docker/buildx/version"
 	"github.com/docker/cli/cli/command"
-	"github.com/pkg/errors"
 	"go.opentelemetry.io/otel"
-	"go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc"
 	"go.opentelemetry.io/otel/metric"
-	"go.opentelemetry.io/otel/metric/noop"
-	sdkmetric "go.opentelemetry.io/otel/sdk/metric"
-	"go.opentelemetry.io/otel/sdk/metric/metricdata"
-	"golang.org/x/sync/errgroup"
 )
 
-const (
-	otelConfigFieldName = "otel"
-	reportTimeout       = 2 * time.Second
-)
-
-// MeterProvider holds a MeterProvider for metric generation and the configured
-// exporters for reporting metrics from the CLI.
-type MeterProvider struct {
-	metric.MeterProvider
-	reader    *sdkmetric.ManualReader
-	exporters []sdkmetric.Exporter
-}
-
-// NewMeterProvider configures a MeterProvider from the CLI context.
-func NewMeterProvider(ctx context.Context, cli command.Cli) (*MeterProvider, error) {
-	var exps []sdkmetric.Exporter
-
-	if exp, err := dockerOtelExporter(cli); err != nil {
-		return nil, err
-	} else if exp != nil {
-		exps = append(exps, exp)
-	}
-
-	if confutil.IsExperimental() {
-		// Expose the user-facing metric exporter only if the experimental flag is set.
-		if exp, err := detectOtlpExporter(ctx); err != nil {
-			return nil, err
-		} else if exp != nil {
-			exps = append(exps, exp)
-		}
-	}
-
-	if len(exps) == 0 {
-		// No exporters are configured so use a noop provider.
-		return &MeterProvider{
-			MeterProvider: noop.NewMeterProvider(),
-		}, nil
-	}
-
-	reader := sdkmetric.NewManualReader(
-		sdkmetric.WithTemporalitySelector(deltaTemporality),
-	)
-	mp := sdkmetric.NewMeterProvider(
-		sdkmetric.WithResource(Resource()),
-		sdkmetric.WithReader(reader),
-	)
-	return &MeterProvider{
-		MeterProvider: mp,
-		reader:        reader,
-		exporters:     exps,
-	}, nil
-}
-
-// Report exports metrics to the configured exporter. This should be done before the CLI
-// exits.
-func (m *MeterProvider) Report(ctx context.Context) {
-	if m.reader == nil {
-		// Not configured.
-		return
-	}
-
-	ctx, cancel := context.WithTimeout(ctx, reportTimeout)
-	defer cancel()
-
-	var rm metricdata.ResourceMetrics
-	if err := m.reader.Collect(ctx, &rm); err != nil {
-		// Error when collecting metrics. Do not send any.
-		otel.Handle(err)
-		return
-	}
-
-	var eg errgroup.Group
-	for _, exp := range m.exporters {
-		exp := exp
-		eg.Go(func() error {
-			if err := exp.Export(ctx, &rm); err != nil {
-				otel.Handle(err)
-			}
-			_ = exp.Shutdown(ctx)
-			return nil
-		})
-	}
-
-	// Can't report an error because we don't allow it to.
-	_ = eg.Wait()
-}
-
-// dockerOtelExporter reads the CLI metadata to determine an OTLP exporter
-// endpoint for docker metrics to be sent.
-//
-// This location, configuration, and usage is hard-coded as part of
-// sending usage statistics so this metric reporting is not meant to be
-// user facing.
-func dockerOtelExporter(cli command.Cli) (sdkmetric.Exporter, error) {
-	endpoint, err := otelExporterOtlpEndpoint(cli)
-	if endpoint == "" || err != nil {
-		return nil, err
-	}
-
-	// Parse the endpoint. The docker config expects the endpoint to be
-	// in the form of a URL to match the environment variable, but this
-	// option doesn't correspond directly to WithEndpoint.
-	//
-	// We pretend we're the same as the environment reader.
-	u, err := url.Parse(endpoint)
-	if err != nil {
-		return nil, errors.Errorf("docker otel endpoint is invalid: %s", err)
-	}
-
-	var opts []otlpmetricgrpc.Option
-	switch u.Scheme {
-	case "unix":
-		// Unix sockets are a bit weird. OTEL seems to imply they
-		// can be used as an environment variable and are handled properly,
-		// but they don't seem to be as the behavior of the environment variable
-		// is to strip the scheme from the endpoint, but the underlying implementation
-		// needs the scheme to use the correct resolver.
-		//
-		// We'll just handle this in a special way and add the unix:// back to the endpoint.
-		opts = []otlpmetricgrpc.Option{
-			otlpmetricgrpc.WithEndpoint(fmt.Sprintf("unix://%s", path.Join(u.Host, u.Path))),
-			otlpmetricgrpc.WithInsecure(),
-		}
-	case "http":
-		opts = []otlpmetricgrpc.Option{
-			// Omit the scheme when using http or https.
-			otlpmetricgrpc.WithEndpoint(path.Join(u.Host, u.Path)),
-			otlpmetricgrpc.WithInsecure(),
-		}
-	default:
-		opts = []otlpmetricgrpc.Option{
-			// Omit the scheme when using http or https.
-			otlpmetricgrpc.WithEndpoint(path.Join(u.Host, u.Path)),
-		}
-	}
-
-	// Hardcoded endpoint from the endpoint.
-	exp, err := otlpmetricgrpc.New(context.Background(), opts...)
-	if err != nil {
-		return nil, err
-	}
-	return exp, nil
-}
-
-// otelExporterOtlpEndpoint retrieves the OTLP endpoint used for the docker reporter
-// from the current context.
-func otelExporterOtlpEndpoint(cli command.Cli) (string, error) {
-	meta, err := cli.ContextStore().GetMetadata(cli.CurrentContext())
-	if err != nil {
-		return "", err
-	}
-
-	var otelCfg interface{}
-	switch m := meta.Metadata.(type) {
-	case command.DockerContext:
-		otelCfg = m.AdditionalFields[otelConfigFieldName]
-	case map[string]interface{}:
-		otelCfg = m[otelConfigFieldName]
-	}
-
-	if otelCfg == nil {
-		return "", nil
-	}
-
-	otelMap, ok := otelCfg.(map[string]interface{})
-	if !ok {
-		return "", errors.Errorf(
-			"unexpected type for field %q: %T (expected: %T)",
-			otelConfigFieldName,
-			otelCfg,
-			otelMap,
-		)
-	}
-
-	// keys from https://opentelemetry.io/docs/concepts/sdk-configuration/otlp-exporter-configuration/
-	endpoint, _ := otelMap["OTEL_EXPORTER_OTLP_ENDPOINT"].(string)
-	return endpoint, nil
-}
-
-// deltaTemporality sets the Temporality of every instrument to delta.
-//
-// This isn't really needed since we create a unique resource on each invocation,
-// but it can help with cardinality concerns for downstream processors since they can
-// perform aggregation for a time interval and then discard the data once that time
-// period has passed. Cumulative temporality would imply to the downstream processor
-// that they might receive a successive point and they may unnecessarily keep state
-// they really shouldn't.
-func deltaTemporality(_ sdkmetric.InstrumentKind) metricdata.Temporality {
-	return metricdata.DeltaTemporality
-}
-
 // Meter returns a Meter from the MetricProvider that indicates the measurement
 // comes from buildx with the appropriate version.
 func Meter(mp metric.MeterProvider) metric.Meter {
 	return mp.Meter(version.Package,
 		metric.WithInstrumentationVersion(version.Version))
 }
+
+// Shutdown invokes Shutdown on the MeterProvider and then reports any error to the OTEL handler.
+func Shutdown(ctx context.Context, mp command.MeterProvider) {
+	if err := mp.Shutdown(ctx); err != nil {
+		otel.Handle(err)
+	}
+}