Skip to content

Commit

Permalink
metrics: add build command duration metric
Browse files Browse the repository at this point in the history
This adds a build duration metric for the build command with attributes
related to the buildx driver, the error type (if any), and which options
were used to perform the build from a subset of the options.

This also refactors some of the utility methods used by the git tool to
determine filepaths into its own separate package so they can be reused
in another place.

Also adds a test to ensure the resource is initialized correctly and
doesn't error. The otel handler logging message is suppressed on buildx
invocations so we never see the error if there's a problem with the
schema url. It's so easy to mess up the schema url when upgrading OTEL
that we need a proper test to make sure we haven't broken the
functionality.

Signed-off-by: Jonathan A. Sternberg <[email protected]>
  • Loading branch information
jsternberg committed Feb 13, 2024
1 parent 082d5d7 commit a60f069
Show file tree
Hide file tree
Showing 16 changed files with 254 additions and 87 deletions.
33 changes: 4 additions & 29 deletions build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@ import (
"bufio"
"bytes"
"context"
"crypto/rand"
_ "crypto/sha256" // ensure digests can be computed
"encoding/base64"
"encoding/hex"
"encoding/json"
"fmt"
"io"
Expand All @@ -26,9 +24,11 @@ import (
"github.com/distribution/reference"
"github.com/docker/buildx/builder"
"github.com/docker/buildx/driver"
"github.com/docker/buildx/util/confutil"
"github.com/docker/buildx/util/desktop"
"github.com/docker/buildx/util/dockerutil"
"github.com/docker/buildx/util/imagetools"
"github.com/docker/buildx/util/osutil"
"github.com/docker/buildx/util/progress"
"github.com/docker/buildx/util/resolver"
"github.com/docker/buildx/util/waitmap"
Expand Down Expand Up @@ -389,7 +389,7 @@ func toSolveOpt(ctx context.Context, node builder.Node, multiDriver bool, opt Op
if p, err := filepath.Abs(sharedKey); err == nil {
sharedKey = filepath.Base(p)
}
so.SharedKey = sharedKey + ":" + tryNodeIdentifier(configDir)
so.SharedKey = sharedKey + ":" + confutil.TryNodeIdentifier(configDir)
}

if opt.Pull {
Expand Down Expand Up @@ -1148,7 +1148,7 @@ func LoadInputs(ctx context.Context, d *driver.DriverHandle, inp Inputs, pw prog
target.LocalDirs["context"] = inp.ContextPath
}
}
case isLocalDir(inp.ContextPath):
case osutil.IsLocalDir(inp.ContextPath):
target.LocalDirs["context"] = inp.ContextPath
switch inp.DockerfilePath {
case "-":
Expand Down Expand Up @@ -1465,31 +1465,6 @@ func handleLowercaseDockerfile(dir, p string) string {
return p
}

var nodeIdentifierMu sync.Mutex

func tryNodeIdentifier(configDir string) (out string) {
nodeIdentifierMu.Lock()
defer nodeIdentifierMu.Unlock()
sessionFile := filepath.Join(configDir, ".buildNodeID")
if _, err := os.Lstat(sessionFile); err != nil {
if os.IsNotExist(err) { // create a new file with stored randomness
b := make([]byte, 8)
if _, err := rand.Read(b); err != nil {
return out
}
if err := os.WriteFile(sessionFile, []byte(hex.EncodeToString(b)), 0600); err != nil {
return out
}
}
}

dt, err := os.ReadFile(sessionFile)
if err == nil {
return string(dt)
}
return
}

func noPrintFunc(opt map[string]Options) bool {
for _, v := range opt {
if v.PrintFunc != nil {
Expand Down
19 changes: 6 additions & 13 deletions build/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"strings"

"github.com/docker/buildx/util/gitutil"
"github.com/docker/buildx/util/osutil"
"github.com/moby/buildkit/client"
specs "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
Expand Down Expand Up @@ -46,9 +47,9 @@ func getGitAttributes(ctx context.Context, contextPath string, dockerfilePath st
if filepath.IsAbs(contextPath) {
wd = contextPath
} else {
wd, _ = filepath.Abs(filepath.Join(getWd(), contextPath))
wd, _ = filepath.Abs(filepath.Join(osutil.GetWd(), contextPath))
}
wd = gitutil.SanitizePath(wd)
wd = osutil.SanitizePath(wd)

gitc, err := gitutil.New(gitutil.WithContext(ctx), gitutil.WithWorkingDir(wd))
if err != nil {
Expand Down Expand Up @@ -104,7 +105,7 @@ func getGitAttributes(ctx context.Context, contextPath string, dockerfilePath st
dockerfilePath = filepath.Join(wd, "Dockerfile")
}
if !filepath.IsAbs(dockerfilePath) {
dockerfilePath = filepath.Join(getWd(), dockerfilePath)
dockerfilePath = filepath.Join(osutil.GetWd(), dockerfilePath)
}
if r, err := filepath.Rel(root, dockerfilePath); err == nil && !strings.HasPrefix(r, "..") {
res["label:"+DockerfileLabel] = r
Expand All @@ -124,21 +125,13 @@ func getGitAttributes(ctx context.Context, contextPath string, dockerfilePath st
if err != nil {
continue
}
if lp, err := getLongPathName(dir); err == nil {
if lp, err := osutil.GetLongPathName(dir); err == nil {
dir = lp
}
dir = gitutil.SanitizePath(dir)
dir = osutil.SanitizePath(dir)
if r, err := filepath.Rel(root, dir); err == nil && !strings.HasPrefix(r, "..") {
so.FrontendAttrs["vcs:localdir:"+k] = r
}
}
}, nil
}

func getWd() string {
wd, _ := os.Getwd()
if lp, err := getLongPathName(wd); err == nil {
return lp
}
return wd
}
9 changes: 0 additions & 9 deletions build/git_unix.go

This file was deleted.

6 changes: 0 additions & 6 deletions build/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"bytes"
"context"
"net"
"os"
"strings"

"github.com/docker/buildx/driver"
Expand Down Expand Up @@ -34,11 +33,6 @@ func IsRemoteURL(c string) bool {
return false
}

func isLocalDir(c string) bool {
st, err := os.Stat(c)
return err == nil && st.IsDir()
}

func isArchive(header []byte) bool {
for _, m := range [][]byte{
{0x42, 0x5A, 0x68}, // bzip2
Expand Down
97 changes: 97 additions & 0 deletions commands/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ package commands
import (
"bytes"
"context"
"crypto/sha256"
"encoding/base64"
"encoding/csv"
"encoding/hex"
"encoding/json"
"fmt"
"io"
Expand All @@ -13,6 +15,8 @@ import (
"path/filepath"
"strconv"
"strings"
"sync"
"time"

"github.com/containerd/console"
"github.com/docker/buildx/build"
Expand All @@ -28,9 +32,11 @@ import (
"github.com/docker/buildx/store/storeutil"
"github.com/docker/buildx/util/buildflags"
"github.com/docker/buildx/util/cobrautil"
"github.com/docker/buildx/util/confutil"
"github.com/docker/buildx/util/desktop"
"github.com/docker/buildx/util/ioset"
"github.com/docker/buildx/util/metricutil"
"github.com/docker/buildx/util/osutil"
"github.com/docker/buildx/util/progress"
"github.com/docker/buildx/util/tracing"
"github.com/docker/cli-docs-tool/annotation"
Expand All @@ -52,6 +58,8 @@ import (
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric"
"google.golang.org/grpc/codes"
)

Expand Down Expand Up @@ -211,6 +219,56 @@ func (o *buildOptions) toDisplayMode() (progressui.DisplayMode, error) {
return progress, nil
}

func buildMetricAttributes(dockerCli command.Cli, b *builder.Builder, options *buildOptions) attribute.Set {
aliasName := os.Getenv("DOCKER_CLI_BUILD_ALIAS")
if aliasName == "" {
aliasName = "buildx"
}
return attribute.NewSet(
attribute.String("command.name", aliasName),
attribute.String("driver.name", options.builder),
attribute.String("driver.type", b.Driver),
attribute.Stringer("build.options.hash", &buildOptionsHash{
buildOptions: options,
configDir: confutil.ConfigDir(dockerCli),
}),
)
}

// buildOptionsHash computes a hash for the buildOptions when the String method is invoked.
// This is done so we can delay the computation of the hash until needed by OTEL using
// the fmt.Stringer interface.
type buildOptionsHash struct {
*buildOptions
configDir string
result string
resultOnce sync.Once
}

func (o *buildOptionsHash) String() string {
o.resultOnce.Do(func() {
target := o.target
contextPath := o.contextPath
dockerfile := o.dockerfileName
if dockerfile == "" {
dockerfile = "Dockerfile"
}

if contextPath != "-" && osutil.IsLocalDir(contextPath) {
contextPath = osutil.ToAbs(contextPath)
}
salt := confutil.TryNodeIdentifier(o.configDir)

h := sha256.New()
for _, s := range []string{target, contextPath, dockerfile, salt} {
_, _ = io.WriteString(h, s)
h.Write([]byte{0})
}
o.result = hex.EncodeToString(h.Sum(nil))
})
return o.result
}

func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions) (err error) {
mp, err := metricutil.NewMeterProvider(ctx, dockerCli)
if err != nil {
Expand Down Expand Up @@ -279,6 +337,8 @@ func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions)
return err
}

attributes := buildMetricAttributes(dockerCli, b, &options)
done := timeBuildCommand(mp, attributes)
var resp *client.SolveResponse
var retErr error
if isExperimental() {
Expand All @@ -290,6 +350,8 @@ func runBuild(ctx context.Context, dockerCli command.Cli, options buildOptions)
if err := printer.Wait(); retErr == nil {
retErr = err
}

done(retErr)
if retErr != nil {
return retErr
}
Expand Down Expand Up @@ -930,3 +992,38 @@ func maybeJSONArray(v string) []string {
}
return []string{v}
}

// timeBuildCommand will start a timer for timing the build command. It records the time when the returned
// function is invoked into a metric.
func timeBuildCommand(mp metric.MeterProvider, attrs attribute.Set) func(err error) {
meter := metricutil.Meter(mp)
counter, _ := meter.Float64Counter("command.time",
metric.WithDescription("Measures the duration of the build command."),
metric.WithUnit("ms"),
)

start := time.Now()
return func(err error) {
dur := float64(time.Since(start)) / float64(time.Millisecond)
extraAttrs := attribute.NewSet()
if err != nil {
extraAttrs = attribute.NewSet(
attribute.String("error.type", otelErrorType(err)),
)
}
counter.Add(context.Background(), dur,
metric.WithAttributeSet(attrs),
metric.WithAttributeSet(extraAttrs),
)
}
}

// otelErrorType returns an attribute for the error type based on the error category.
// If nil, this function returns an invalid attribute.
func otelErrorType(err error) string {
name := "generic"
if errors.Is(err, context.Canceled) {
name = "canceled"
}
return name
}
34 changes: 34 additions & 0 deletions util/confutil/node.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package confutil

import (
"crypto/rand"
"encoding/hex"
"os"
"path/filepath"
"sync"
)

var nodeIdentifierMu sync.Mutex

func TryNodeIdentifier(configDir string) (out string) {
nodeIdentifierMu.Lock()
defer nodeIdentifierMu.Unlock()
sessionFile := filepath.Join(configDir, ".buildNodeID")
if _, err := os.Lstat(sessionFile); err != nil {
if os.IsNotExist(err) { // create a new file with stored randomness
b := make([]byte, 8)
if _, err := rand.Read(b); err != nil {
return out
}
if err := os.WriteFile(sessionFile, []byte(hex.EncodeToString(b)), 0600); err != nil {
return out
}
}
}

dt, err := os.ReadFile(sessionFile)
if err == nil {
return string(dt)
}
return
}
3 changes: 2 additions & 1 deletion util/gitutil/gitutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"path/filepath"
"strings"

"github.com/docker/buildx/util/osutil"
"github.com/pkg/errors"
)

Expand Down Expand Up @@ -70,7 +71,7 @@ func (c *Git) RootDir() (string, error) {
if err != nil {
return "", err
}
return SanitizePath(root), nil
return osutil.SanitizePath(root), nil
}

func (c *Git) GitDir() (string, error) {
Expand Down
17 changes: 0 additions & 17 deletions util/gitutil/path.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import (
"os"
"os/exec"
"path/filepath"
"regexp"
"strings"

"github.com/moby/sys/mountinfo"
)
Expand Down Expand Up @@ -42,18 +40,3 @@ func gitPath(wd string) (string, error) {
}
return exec.LookPath("git")
}

var windowsPathRegex = regexp.MustCompile(`^[A-Za-z]:[\\/].*$`)

func SanitizePath(path string) string {
// If we're running in WSL, we need to convert Windows paths to Unix paths.
// This is because the git binary can be invoked through `git.exe` and
// therefore returns Windows paths.
if os.Getenv("WSL_DISTRO_NAME") != "" && windowsPathRegex.MatchString(path) {
unixPath := strings.ReplaceAll(path, "\\", "/")
drive := strings.ToLower(string(unixPath[0]))
rest := filepath.Clean(unixPath[3:])
return filepath.Join("/mnt", drive, rest)
}
return filepath.Clean(path)
}
Loading

0 comments on commit a60f069

Please sign in to comment.