Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

metricutil: switch to using the cli meter provider #2373

Merged
merged 1 commit into from
Apr 4, 2024
Merged

metricutil: switch to using the cli meter provider #2373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions commands/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 5 additions & 5 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want the change backported to the 26.0 branch so that we don't have to force consumers to depend on master? (Haven't checked if we can)

cc @vvoland @laurazard

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is possible to backport docker/cli#4889, I think so yes

github.com/docker/cli-docs-tool v0.7.0
github.com/docker/docker v26.0.0-rc1+incompatible
github.com/docker/go-units v0.5.0
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
209 changes: 7 additions & 202 deletions util/metricutil/metric.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
45 changes: 0 additions & 45 deletions util/metricutil/otlp.go
View file
Open in desktop

This file was deleted.

53 changes: 0 additions & 53 deletions util/metricutil/resource.go
View file
Open in desktop

This file was deleted.

Loading