Skip to content

metrics: add build duration metric #2185

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

Closed
jsternberg wants to merge 1 commit into from
Closed

metrics: add build duration metric #2185

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
53 changes: 50 additions & 3 deletions build/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"bytes"
"context"
"crypto/rand"
"crypto/sha256"
_ "crypto/sha256" // ensure digests can be computed
"encoding/base64"
"encoding/hex"
Expand Down Expand Up @@ -55,6 +56,8 @@ import (
specs "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/trace"
"golang.org/x/sync/errgroup"
)
Expand Down Expand Up @@ -469,11 +472,11 @@ func toSolveOpt(ctx context.Context, node builder.Node, multiDriver bool, opt Op
return &so, releaseF, nil
}

func Build(ctx context.Context, nodes []builder.Node, opt map[string]Options, docker *dockerutil.Client, configDir string, w progress.Writer) (resp map[string]*client.SolveResponse, err error) {
return BuildWithResultHandler(ctx, nodes, opt, docker, configDir, w, nil)
func Build(ctx context.Context, nodes []builder.Node, opt map[string]Options, docker *dockerutil.Client, configDir string, w progress.Writer, mp metric.MeterProvider) (resp map[string]*client.SolveResponse, err error) {
return BuildWithResultHandler(ctx, nodes, opt, docker, configDir, w, mp, nil)
}

func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[string]Options, docker *dockerutil.Client, configDir string, w progress.Writer, resultHandleFunc func(driverIndex int, rCtx *ResultHandle)) (resp map[string]*client.SolveResponse, err error) {
func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[string]Options, docker *dockerutil.Client, configDir string, w progress.Writer, mp metric.MeterProvider, resultHandleFunc func(driverIndex int, rCtx *ResultHandle)) (resp map[string]*client.SolveResponse, err error) {
if len(nodes) == 0 {
return nil, errors.Errorf("driver required for build")
}
Expand Down Expand Up @@ -686,6 +689,14 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[s
return err
}

buildAttributes := attribute.NewSet(
driverAttribute(dp),
buildIdentityAttribute(so),
buildRefAttribute(so),
)
pw, record := progress.Metrics(mp, pw,
metric.WithAttributeSet(buildAttributes))

frontendInputs := make(map[string]*pb.Definition)
for key, st := range so.FrontendInputs {
def, err := st.Marshal(ctx)
Expand Down Expand Up @@ -785,6 +796,7 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[s
} else {
rr, err = c.Build(ctx, *so, "buildx", buildFunc, ch)
}

if desktop.BuildBackendEnabled() && node.Driver.HistoryAPISupported(ctx) {
buildRef := fmt.Sprintf("%s/%s/%s", node.Builder, node.Name, so.Ref)
if err != nil {
Expand Down Expand Up @@ -842,6 +854,9 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[s
}
}
}

// Record the result of the build.
record(ctx)
return nil
})
}
Expand Down Expand Up @@ -1523,3 +1538,35 @@ func ReadSourcePolicy() (*spb.Policy, error) {

return &pol, nil
}

// driverAttribute is a utility to retrieve the backend attribute from a resolvedNode.
func driverAttribute(dp *resolvedNode) attribute.KeyValue {
driverName := dp.Node().Driver.Factory().Name()
return attribute.String("driver", driverName)
}

// buildIdentityAttribute is a utility to retrieve the build id attribute from the solve options.
// This value should be consistent between builds.
func buildIdentityAttribute(so *client.SolveOpt) attribute.KeyValue {
vcs := so.FrontendAttrs["vcs:source"]
target := so.FrontendAttrs["target"]
context := so.FrontendAttrs["context"]
filename := so.FrontendAttrs["filename"]

buildID := ""
if vcs != "" || target != "" || context != "" || filename != "" {
h := sha256.New()
for _, s := range []string{vcs, target, context, filename} {
_, _ = io.WriteString(h, s)
h.Write([]byte{0})
}
buildID = hex.EncodeToString(h.Sum(nil))
Copy link
Member

@tonistiigi tonistiigi Jan 25, 2024

Choose a reason for hiding this comment

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

I think we need to add something unique to node into this mix. Eg. see tryNodeIdentifier. Otherwise, this metric can leak data with a simple plaintext attack.

Copy link
Collaborator Author

@jsternberg jsternberg Jan 26, 2024

Choose a reason for hiding this comment

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

I think we need to be able to correlate the same parameters between different machines. I believe the node identifier is unique between machines. Would there be a different salt parameter we could use for this purpose that would still allow us to match build identities across machines?

}
return attribute.String("build.identity", buildID)
}

// buildRefAttribute is a utility to retrieve the build ref attribute from the solve options.
// This value should be unique to each build.
func buildRefAttribute(so *client.SolveOpt) attribute.KeyValue {
return attribute.String("build.ref", so.Ref)
}
10 changes: 4 additions & 6 deletions commands/bake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ import (
"github.com/docker/buildx/build"
"github.com/docker/buildx/builder"
"github.com/docker/buildx/localstate"
sdkmetric "github.com/docker/buildx/otel/sdk/metric"
"github.com/docker/buildx/util/buildflags"
"github.com/docker/buildx/util/cobrautil/completion"
"github.com/docker/buildx/util/confutil"
"github.com/docker/buildx/util/desktop"
"github.com/docker/buildx/util/dockerutil"
"github.com/docker/buildx/util/metrics"
"github.com/docker/buildx/util/progress"
"github.com/docker/buildx/util/tracing"
"github.com/docker/cli/cli/command"
Expand All @@ -43,13 +43,11 @@ type bakeOptions struct {
}

func runBake(ctx context.Context, dockerCli command.Cli, targets []string, in bakeOptions, cFlags commonFlags) (err error) {
mp, report, err := metrics.MeterProvider(dockerCli)
mp, err := sdkmetric.NewMeterProvider(ctx, dockerCli)
if err != nil {
return err
}
defer report()

recordVersionInfo(mp, "bake")
defer mp.Report(context.Background())

ctx, end, err := tracing.TraceCurrentCommand(ctx, "bake")
if err != nil {
Expand Down Expand Up @@ -235,7 +233,7 @@ func runBake(ctx context.Context, dockerCli command.Cli, targets []string, in ba
return err
}

resp, err := build.Build(ctx, nodes, bo, dockerutil.NewClient(dockerCli), confutil.ConfigDir(dockerCli), printer)
resp, err := build.Build(ctx, nodes, bo, dockerutil.NewClient(dockerCli), confutil.ConfigDir(dockerCli), printer, mp)
if err != nil {
return wrapBuildError(err, true)
}
Expand Down
50 changes: 9 additions & 41 deletions commands/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,15 @@ import (
controllererrors "github.com/docker/buildx/controller/errdefs"
controllerapi "github.com/docker/buildx/controller/pb"
"github.com/docker/buildx/monitor"
sdkmetric "github.com/docker/buildx/otel/sdk/metric"
"github.com/docker/buildx/store"
"github.com/docker/buildx/store/storeutil"
"github.com/docker/buildx/util/buildflags"
"github.com/docker/buildx/util/cobrautil"
"github.com/docker/buildx/util/desktop"
"github.com/docker/buildx/util/ioset"
"github.com/docker/buildx/util/metrics"
"github.com/docker/buildx/util/progress"
"github.com/docker/buildx/util/tracing"
"github.com/docker/buildx/version"
"github.com/docker/cli-docs-tool/annotation"
"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command"
Expand All @@ -53,8 +52,6 @@ import (
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric"
"google.golang.org/grpc/codes"
)
Expand Down Expand Up @@ -216,13 +213,11 @@ func (o *buildOptions) toDisplayMode() (progressui.DisplayMode, error) {
}

func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions) (err error) {
mp, report, err := metrics.MeterProvider(dockerCli)
mp, err := sdkmetric.NewMeterProvider(ctx, dockerCli)
if err != nil {
return err
}
defer report()

recordVersionInfo(mp, "build")
defer mp.Report(context.Background())

ctx, end, err := tracing.TraceCurrentCommand(ctx, "build")
if err != nil {
Expand Down Expand Up @@ -288,9 +283,9 @@ func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions)
var resp *client.SolveResponse
var retErr error
if isExperimental() {
resp, retErr = runControllerBuild(ctx, dockerCli, opts, options, printer)
resp, retErr = runControllerBuild(ctx, dockerCli, opts, options, printer, mp)
} else {
resp, retErr = runBasicBuild(ctx, dockerCli, opts, options, printer)
resp, retErr = runBasicBuild(ctx, dockerCli, opts, options, printer, mp)
}

if err := printer.Wait(); retErr == nil {
Expand Down Expand Up @@ -332,20 +327,20 @@ func getImageID(resp map[string]string) string {
return dgst
}

func runBasicBuild(ctx context.Context, dockerCli command.Cli, opts *controllerapi.BuildOptions, options buildOptions, printer *progress.Printer) (*client.SolveResponse, error) {
resp, res, err := cbuild.RunBuild(ctx, dockerCli, *opts, dockerCli.In(), printer, false)
func runBasicBuild(ctx context.Context, dockerCli command.Cli, opts *controllerapi.BuildOptions, options buildOptions, printer *progress.Printer, mp metric.MeterProvider) (*client.SolveResponse, error) {
resp, res, err := cbuild.RunBuild(ctx, dockerCli, *opts, dockerCli.In(), printer, mp, false)
if res != nil {
res.Done()
}
return resp, err
}

func runControllerBuild(ctx context.Context, dockerCli command.Cli, opts *controllerapi.BuildOptions, options buildOptions, printer *progress.Printer) (*client.SolveResponse, error) {
func runControllerBuild(ctx context.Context, dockerCli command.Cli, opts *controllerapi.BuildOptions, options buildOptions, printer *progress.Printer, mp metric.MeterProvider) (*client.SolveResponse, error) {
if options.invokeConfig != nil && (options.dockerfileName == "-" || options.contextPath == "-") {
// stdin must be usable for monitor
return nil, errors.Errorf("Dockerfile or context from stdin is not supported with invoke")
}
c, err := controller.NewController(ctx, options.ControlOptions, dockerCli, printer)
c, err := controller.NewController(ctx, options.ControlOptions, dockerCli, printer, mp)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -936,30 +931,3 @@ func maybeJSONArray(v string) []string {
}
return []string{v}
}

func recordVersionInfo(mp metric.MeterProvider, command string) {
// Still in the process of testing/stabilizing these counters.
if !isExperimental() {
return
}

meter := mp.Meter("github.com/docker/buildx",
metric.WithInstrumentationVersion(version.Version),
)

counter, err := meter.Int64Counter("docker.cli.count",
metric.WithDescription("Number of invocations of the docker buildx command."),
)
if err != nil {
otel.Handle(err)
}

counter.Add(context.Background(), 1,
metric.WithAttributes(
attribute.String("command", command),
attribute.String("package", version.Package),
attribute.String("version", version.Version),
attribute.String("revision", version.Revision),
),
)
}
3 changes: 2 additions & 1 deletion commands/debug/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"go.opentelemetry.io/otel/metric/noop"
)

// DebugConfig is a user-specified configuration for the debugger.
Expand Down Expand Up @@ -50,7 +51,7 @@ func RootCmd(dockerCli command.Cli, children ...DebuggableCmd) *cobra.Command {
}

ctx := context.TODO()
c, err := controller.NewController(ctx, controlOptions, dockerCli, printer)
c, err := controller.NewController(ctx, controlOptions, dockerCli, printer, noop.NewMeterProvider())
if err != nil {
return err
}
Expand Down
11 changes: 6 additions & 5 deletions controller/build/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/moby/buildkit/session/auth/authprovider"
"github.com/moby/buildkit/util/grpcerrors"
"github.com/pkg/errors"
"go.opentelemetry.io/otel/metric"
"google.golang.org/grpc/codes"
)

Expand All @@ -36,7 +37,7 @@ const defaultTargetName = "default"
// NOTE: When an error happens during the build and this function acquires the debuggable *build.ResultHandle,
// this function returns it in addition to the error (i.e. it does "return nil, res, err"). The caller can
// inspect the result and debug the cause of that error.
func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.BuildOptions, inStream io.Reader, progress progress.Writer, generateResult bool) (*client.SolveResponse, *build.ResultHandle, error) {
func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.BuildOptions, inStream io.Reader, progress progress.Writer, mp metric.MeterProvider, generateResult bool) (*client.SolveResponse, *build.ResultHandle, error) {
if in.NoCache && len(in.NoCacheFilter) > 0 {
return nil, nil, errors.Errorf("--no-cache and --no-cache-filter cannot currently be used together")
}
Expand Down Expand Up @@ -187,7 +188,7 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.Build
return nil, nil, err
}

resp, res, err := buildTargets(ctx, dockerCli, b.NodeGroup, nodes, map[string]build.Options{defaultTargetName: opts}, progress, generateResult)
resp, res, err := buildTargets(ctx, dockerCli, b.NodeGroup, nodes, map[string]build.Options{defaultTargetName: opts}, progress, mp, generateResult)
err = wrapBuildError(err, false)
if err != nil {
// NOTE: buildTargets can return *build.ResultHandle even on error.
Expand All @@ -201,22 +202,22 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.Build
// NOTE: When an error happens during the build and this function acquires the debuggable *build.ResultHandle,
// this function returns it in addition to the error (i.e. it does "return nil, res, err"). The caller can
// inspect the result and debug the cause of that error.
func buildTargets(ctx context.Context, dockerCli command.Cli, ng *store.NodeGroup, nodes []builder.Node, opts map[string]build.Options, progress progress.Writer, generateResult bool) (*client.SolveResponse, *build.ResultHandle, error) {
func buildTargets(ctx context.Context, dockerCli command.Cli, ng *store.NodeGroup, nodes []builder.Node, opts map[string]build.Options, progress progress.Writer, mp metric.MeterProvider, generateResult bool) (*client.SolveResponse, *build.ResultHandle, error) {
var res *build.ResultHandle
var resp map[string]*client.SolveResponse
var err error
if generateResult {
var mu sync.Mutex
var idx int
resp, err = build.BuildWithResultHandler(ctx, nodes, opts, dockerutil.NewClient(dockerCli), confutil.ConfigDir(dockerCli), progress, func(driverIndex int, gotRes *build.ResultHandle) {
resp, err = build.BuildWithResultHandler(ctx, nodes, opts, dockerutil.NewClient(dockerCli), confutil.ConfigDir(dockerCli), progress, mp, func(driverIndex int, gotRes *build.ResultHandle) {
mu.Lock()
defer mu.Unlock()
if res == nil || driverIndex < idx {
idx, res = driverIndex, gotRes
}
})
} else {
resp, err = build.Build(ctx, nodes, opts, dockerutil.NewClient(dockerCli), confutil.ConfigDir(dockerCli), progress)
resp, err = build.Build(ctx, nodes, opts, dockerutil.NewClient(dockerCli), confutil.ConfigDir(dockerCli), progress, mp)
}
if err != nil {
return nil, res, err
Expand Down
7 changes: 4 additions & 3 deletions controller/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import (
"github.com/docker/buildx/util/progress"
"github.com/docker/cli/cli/command"
"github.com/pkg/errors"
"go.opentelemetry.io/otel/metric"
)

func NewController(ctx context.Context, opts control.ControlOptions, dockerCli command.Cli, pw progress.Writer) (control.BuildxController, error) {
func NewController(ctx context.Context, opts control.ControlOptions, dockerCli command.Cli, pw progress.Writer, mp metric.MeterProvider) (control.BuildxController, error) {
var name string
if opts.Detach {
name = "remote"
Expand All @@ -23,9 +24,9 @@ func NewController(ctx context.Context, opts control.ControlOptions, dockerCli c
var c control.BuildxController
err := progress.Wrap(fmt.Sprintf("[internal] connecting to %s controller", name), pw.Write, func(l progress.SubLogger) (err error) {
if opts.Detach {
c, err = remote.NewRemoteBuildxController(ctx, dockerCli, opts, l)
c, err = remote.NewRemoteBuildxController(ctx, dockerCli, opts, l, mp)
} else {
c = local.NewLocalBuildxController(ctx, dockerCli, l)
c = local.NewLocalBuildxController(ctx, dockerCli, l, mp)
}
return err
})
Expand Down
11 changes: 9 additions & 2 deletions controller/local/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,19 @@ import (
"github.com/docker/cli/cli/command"
"github.com/moby/buildkit/client"
"github.com/pkg/errors"
"go.opentelemetry.io/otel/metric"
"go.opentelemetry.io/otel/metric/noop"
)

func NewLocalBuildxController(ctx context.Context, dockerCli command.Cli, logger progress.SubLogger) control.BuildxController {
func NewLocalBuildxController(ctx context.Context, dockerCli command.Cli, logger progress.SubLogger, mp metric.MeterProvider) control.BuildxController {
if mp == nil {
mp = noop.NewMeterProvider()
}
return &localController{
dockerCli: dockerCli,
ref: "local",
processes: processes.NewManager(),
mp: mp,
}
}

Expand All @@ -38,6 +44,7 @@ type localController struct {
ref string
buildConfig buildConfig
processes *processes.Manager
mp metric.MeterProvider

buildOnGoing atomic.Bool
}
Expand All @@ -48,7 +55,7 @@ func (b *localController) Build(ctx context.Context, options controllerapi.Build
}
defer b.buildOnGoing.Store(false)

resp, res, buildErr := cbuild.RunBuild(ctx, b.dockerCli, options, in, progress, true)
resp, res, buildErr := cbuild.RunBuild(ctx, b.dockerCli, options, in, progress, b.mp, true)
// NOTE: RunBuild can return *build.ResultHandle even on error.
if res != nil {
b.buildConfig = buildConfig{
Expand Down
Loading