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.

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

c8d: Prometheus metrics #47555

Merged
merged 5 commits into from Mar 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion daemon/containerd/image_delete.go
Expand Up @@ -55,7 +55,14 @@ import (
// conflict will not be reported.
//
// TODO(thaJeztah): image delete should send prometheus counters; see https://github.com/moby/moby/issues/45268
func (i *ImageService) ImageDelete(ctx context.Context, imageRef string, force, prune bool) ([]imagetypes.DeleteResponse, error) {
func (i *ImageService) ImageDelete(ctx context.Context, imageRef string, force, prune bool) (response []imagetypes.DeleteResponse, retErr error) {
start := time.Now()
defer func() {
if retErr == nil {
dimages.ImageActions.WithValues("delete").UpdateSince(start)
}
}()

var c conflictType
if !force {
c |= conflictSoft
Expand Down
4 changes: 4 additions & 0 deletions daemon/containerd/image_history.go
Expand Up @@ -2,12 +2,14 @@ package containerd

import (
"context"
"time"

containerdimages "github.com/containerd/containerd/images"
"github.com/containerd/containerd/platforms"
"github.com/containerd/log"
"github.com/distribution/reference"
imagetype "github.com/docker/docker/api/types/image"
dimages "github.com/docker/docker/daemon/images"
"github.com/opencontainers/go-digest"
"github.com/opencontainers/image-spec/identity"
"github.com/pkg/errors"
Expand All @@ -16,6 +18,7 @@ import (
// ImageHistory returns a slice of HistoryResponseItem structures for the
// specified image name by walking the image lineage.
func (i *ImageService) ImageHistory(ctx context.Context, name string) ([]*imagetype.HistoryResponseItem, error) {
start := time.Now()
img, err := i.resolveImage(ctx, name)
if err != nil {
return nil, err
Expand Down Expand Up @@ -113,6 +116,7 @@ func (i *ImageService) ImageHistory(ctx context.Context, name string) ([]*imaget
}
}

dimages.ImageActions.WithValues("history").UpdateSince(start)
return history, nil
}

Expand Down
10 changes: 9 additions & 1 deletion daemon/containerd/image_pull.go
Expand Up @@ -6,6 +6,7 @@ import (
"io"
"os"
"strings"
"time"

"github.com/containerd/containerd"
cerrdefs "github.com/containerd/containerd/errdefs"
Expand All @@ -17,6 +18,7 @@ import (
"github.com/distribution/reference"
"github.com/docker/docker/api/types/events"
registrytypes "github.com/docker/docker/api/types/registry"
dimages "github.com/docker/docker/daemon/images"
"github.com/docker/docker/distribution"
"github.com/docker/docker/errdefs"
"github.com/docker/docker/internal/compatcontext"
Expand All @@ -29,7 +31,13 @@ import (

// PullImage initiates a pull operation. baseRef is the image to pull.
// If reference is not tagged, all tags are pulled.
func (i *ImageService) PullImage(ctx context.Context, baseRef reference.Named, platform *ocispec.Platform, metaHeaders map[string][]string, authConfig *registrytypes.AuthConfig, outStream io.Writer) error {
func (i *ImageService) PullImage(ctx context.Context, baseRef reference.Named, platform *ocispec.Platform, metaHeaders map[string][]string, authConfig *registrytypes.AuthConfig, outStream io.Writer) (retErr error) {
start := time.Now()
defer func() {
if retErr == nil {
dimages.ImageActions.WithValues("pull").UpdateSince(start)
}
}()
out := streamformatter.NewJSONProgressOutput(outStream, false)

if !reference.IsNameOnly(baseRef) {
Expand Down
8 changes: 8 additions & 0 deletions daemon/containerd/image_push.go
Expand Up @@ -6,6 +6,7 @@ import (
"io"
"strings"
"sync"
"time"

"github.com/containerd/containerd/content"
cerrdefs "github.com/containerd/containerd/errdefs"
Expand All @@ -19,6 +20,7 @@ import (
"github.com/distribution/reference"
"github.com/docker/docker/api/types/events"
"github.com/docker/docker/api/types/registry"
dimages "github.com/docker/docker/daemon/images"
Copy link
Member

Choose a reason for hiding this comment

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

A bit related to that, and I didn't really consider that's already the case, but the containerd and images packages are the two implementations we have for handling the image-store. I see there's multiple imports of the images package into the containerd implementation. Not "critical", but it feels a bit odd. Perhaps we should look at what parts are used "in common" and if that code is in the right place (or should be in a "common" package that's shared between the two if duplicating those parts is undesirable)

"github.com/docker/docker/errdefs"
"github.com/docker/docker/internal/compatcontext"
"github.com/docker/docker/pkg/progress"
Expand All @@ -40,6 +42,12 @@ import (
// to perform cross-repo mounts of the shared content when pushing to a different
// repository on the same registry.
func (i *ImageService) PushImage(ctx context.Context, sourceRef reference.Named, metaHeaders map[string][]string, authConfig *registry.AuthConfig, outStream io.Writer) (retErr error) {
start := time.Now()
defer func() {
if retErr == nil {
dimages.ImageActions.WithValues("push").UpdateSince(start)
}
}()
out := streamformatter.NewJSONProgressOutput(outStream, false)
progress.Messagef(out, "", "The push refers to repository [%s]", sourceRef.Name())

Expand Down
2 changes: 1 addition & 1 deletion daemon/images/image_delete.go
Expand Up @@ -173,7 +173,7 @@ func (i *ImageService) ImageDelete(ctx context.Context, imageRef string, force,
return nil, err
}

imageActions.WithValues("delete").UpdateSince(start)
ImageActions.WithValues("delete").UpdateSince(start)

return records, nil
}
Expand Down
2 changes: 1 addition & 1 deletion daemon/images/image_history.go
Expand Up @@ -76,6 +76,6 @@ func (i *ImageService) ImageHistory(ctx context.Context, name string) ([]*image.
break
}
}
imageActions.WithValues("history").UpdateSince(start)
ImageActions.WithValues("history").UpdateSince(start)
return history, nil
}
2 changes: 1 addition & 1 deletion daemon/images/image_pull.go
Expand Up @@ -26,7 +26,7 @@ func (i *ImageService) PullImage(ctx context.Context, ref reference.Named, platf
start := time.Now()

err := i.pullImageWithReference(ctx, ref, platform, metaHeaders, authConfig, outStream)
imageActions.WithValues("pull").UpdateSince(start)
ImageActions.WithValues("pull").UpdateSince(start)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion daemon/images/image_push.go
Expand Up @@ -48,6 +48,6 @@ func (i *ImageService) PushImage(ctx context.Context, ref reference.Named, metaH
err := distribution.Push(ctx, ref, imagePushConfig)
close(progressChan)
<-writesDone
imageActions.WithValues("push").UpdateSince(start)
ImageActions.WithValues("push").UpdateSince(start)
return err
}
7 changes: 5 additions & 2 deletions daemon/images/locals.go → daemon/images/metrics.go
Expand Up @@ -4,11 +4,14 @@ import (
metrics "github.com/docker/go-metrics"
)

var imageActions metrics.LabeledTimer
// ImagesActions measures the time it takes to process some image actions.
// Exported for use in the containerd-backed image store and it is not intended
// for external consumption. Do not use!
var ImageActions metrics.LabeledTimer

func init() {
ns := metrics.NewNamespace("engine", "daemon", nil)
imageActions = ns.NewLabeledTimer("image_actions", "The number of seconds it takes to process each image action", "action")
ImageActions = ns.NewLabeledTimer("image_actions", "The number of seconds it takes to process each image action", "action")
// TODO: is it OK to register a namespace with the same name? Or does this
// need to be exported from somewhere?
metrics.Register(ns)
Expand Down