Skip to content

Commit

Permalink
Merge pull request #7453 from swagatbora90/trace-cri-image
Browse files Browse the repository at this point in the history 
Add tracing spans in CRI image service and pull.go
  • Loading branch information
@mxpv
mxpv authored Nov 4, 2022
2 parents 525fe21 + ee64926 commit b2a01ee
Show file tree
Hide file tree
Showing 20 changed files with 308 additions and 74 deletions.
2 changes: 1 addition & 1 deletion docs/tracing.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func clientWithTrace() error {
defer cancel()

ctx, span := tracing.StartSpan(ctx, "OPERATION NAME")
defer span.End()
defer tracing.StopSpan(span)
...
}
```
4 changes: 4 additions & 0 deletions integration/client/go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/docker/go-events v0.0.0-20190806004212-e31b211e4f1c // indirect
github.com/docker/go-units v0.4.0 // indirect
github.com/go-logr/logr v1.2.3 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/godbus/dbus/v5 v5.0.6 // indirect
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
github.com/golang/protobuf v1.5.2 // indirect
Expand All @@ -50,6 +52,8 @@ require (
github.com/pkg/errors v0.9.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
go.opencensus.io v0.23.0 // indirect
go.opentelemetry.io/otel v1.7.0 // indirect
go.opentelemetry.io/otel/trace v1.7.0 // indirect
golang.org/x/mod v0.6.0 // indirect
golang.org/x/net v0.1.0 // indirect
golang.org/x/sync v0.1.0 // indirect
Expand Down
4 changes: 4 additions & 0 deletions integration/client/go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,9 @@ github.com/go-logr/logr v0.2.0/go.mod h1:z6/tIYblkpsD+a4lm/fGIIU9mZ+XfAiaFtq7xTg
github.com/go-logr/logr v0.4.0/go.mod h1:z6/tIYblkpsD+a4lm/fGIIU9mZ+XfAiaFtq7xTgseGU=
github.com/go-logr/logr v1.2.0/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
github.com/go-logr/logr v1.2.3 h1:2DntVwHkVopvECVRSlL5PSo9eG+cAkDCuckLubN+rq0=
github.com/go-logr/logr v1.2.3/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag=
github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE=
github.com/go-logr/zapr v1.2.0/go.mod h1:Qa4Bsj2Vb+FAVeAKsLD8RLQ+YRJB8YDmOAKxaBQf7Ro=
github.com/go-openapi/jsonpointer v0.0.0-20160704185906-46af16f9f7b1/go.mod h1:+35s3my2LFTysnkMfxsJBAMHj/DoqoB9knIWoYG/Vk0=
Expand Down Expand Up @@ -704,6 +706,7 @@ go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.2
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.32.0/go.mod h1:J0dBVrt7dPS/lKJyQoW0xzQiUr4r2Ik1VwPjAUWnofI=
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.20.0/go.mod h1:2AboqHi0CiIZU0qwhtUfCYD1GeUzvvIXWNkhDt7ZMG4=
go.opentelemetry.io/otel v0.20.0/go.mod h1:Y3ugLH2oa81t5QO+Lty+zXf8zC9L26ax4Nzoxm/dooo=
go.opentelemetry.io/otel v1.7.0 h1:Z2lA3Tdch0iDcrhJXDIlC94XE+bxok1F9B+4Lz/lGsM=
go.opentelemetry.io/otel v1.7.0/go.mod h1:5BdUoMIz5WEs0vt0CUEMtSSaTSHBBVwrhnz7+nrD5xk=
go.opentelemetry.io/otel/exporters/otlp v0.20.0/go.mod h1:YIieizyaN77rtLJra0buKiNBOm9XQfkPEKBeuhoMwAM=
go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.7.0/go.mod h1:M1hVZHNxcbkAlcvrOMlpQ4YOO3Awf+4N2dxkZL3xm04=
Expand All @@ -717,6 +720,7 @@ go.opentelemetry.io/otel/sdk v1.7.0/go.mod h1:uTEOTwaqIVuTGiJN7ii13Ibp75wJmYUDe3
go.opentelemetry.io/otel/sdk/export/metric v0.20.0/go.mod h1:h7RBNMsDJ5pmI1zExLi+bJK+Dr8NQCh0qGhm1KDnNlE=
go.opentelemetry.io/otel/sdk/metric v0.20.0/go.mod h1:knxiS8Xd4E/N+ZqKmUPf3gTTZ4/0TjTXukfxjzSTpHE=
go.opentelemetry.io/otel/trace v0.20.0/go.mod h1:6GjCW8zgDjwGHGa6GkyeB8+/5vjT16gUEi0Nf1iBdgw=
go.opentelemetry.io/otel/trace v1.7.0 h1:O37Iogk1lEkMRXewVtZ1BBTVn5JEp8GrJvP92bJqC6o=
go.opentelemetry.io/otel/trace v1.7.0/go.mod h1:fzLSB9nqR2eXzxPXb2JW9IKE+ScyXA48yyE4TNvoHqU=
go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqeYNgFYFoEGnI=
go.opentelemetry.io/proto/otlp v0.16.0/go.mod h1:H7XAot3MsfNsj7EXtrA2q5xSNQ10UqI405h3+duxN4U=
Expand Down
12 changes: 12 additions & 0 deletions pkg/cri/sbserver/helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ const (

// runtimeRunhcsV1 is the runtime type for runhcs.
runtimeRunhcsV1 = "io.containerd.runhcs.v1"

// name prefix for CRI sbserver specific spans
criSbServerSpanPrefix = "pkg.cri.sbserver"

spanDelimiter = "."
)

// makeSandboxName generates sandbox name from sandbox metadata. The name
Expand Down Expand Up @@ -501,3 +506,10 @@ func copyResourcesToStatus(spec *runtimespec.Spec, status containerstore.Status)
}
return status
}

func makeSpanName(funcName string) string {
return strings.Join([]string{
criSbServerSpanPrefix,
funcName,
}, spanDelimiter)
}
7 changes: 7 additions & 0 deletions pkg/cri/sbserver/image_pull.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import (
distribution "github.com/containerd/containerd/reference/docker"
"github.com/containerd/containerd/remotes/docker"
"github.com/containerd/containerd/remotes/docker/config"
"github.com/containerd/containerd/tracing"
)

// For image management:
Expand Down Expand Up @@ -93,6 +94,7 @@ import (

// PullImage pulls an image with authentication config.
func (c *criService) PullImage(ctx context.Context, r *runtime.PullImageRequest) (*runtime.PullImageResponse, error) {
span := tracing.CurrentSpan(ctx)
imageRef := r.GetImage().GetImage()
namedRef, err := distribution.ParseDockerRef(imageRef)
if err != nil {
Expand Down Expand Up @@ -133,6 +135,10 @@ func (c *criService) PullImage(ctx context.Context, r *runtime.PullImageRequest)
return nil, err
}
log.G(ctx).Debugf("PullImage %q with snapshotter %s", ref, snapshotter)
span.SetAttributes(
tracing.SpanAttribute("image.ref", ref),
tracing.SpanAttribute("snapshotter.name", snapshotter),
)

pullOpts := []containerd.RemoteOpt{
containerd.WithSchema1Conversion, //nolint:staticcheck // Ignore SA1019. Need to keep deprecated package for compatibility.
Expand Down Expand Up @@ -165,6 +171,7 @@ func (c *criService) PullImage(ctx context.Context, r *runtime.PullImageRequest)
if err != nil {
return nil, fmt.Errorf("failed to pull and unpack image %q: %w", ref, err)
}
span.AddEvent("Pull and unpack image complete")

configDesc, err := image.Config(ctx)
if err != nil {
Expand Down
5 changes: 4 additions & 1 deletion pkg/cri/sbserver/image_remove.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

"github.com/containerd/containerd/errdefs"
"github.com/containerd/containerd/images"
"github.com/containerd/containerd/tracing"

runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
)
Expand All @@ -33,15 +34,17 @@ import (
// Remove the whole image no matter the it's image id or reference. This is the
// semantic defined in CRI now.
func (c *criService) RemoveImage(ctx context.Context, r *runtime.RemoveImageRequest) (*runtime.RemoveImageResponse, error) {
span := tracing.CurrentSpan(ctx)
image, err := c.localResolve(r.GetImage().GetImage())
if err != nil {
if errdefs.IsNotFound(err) {
span.AddEvent(err.Error())
// return empty without error when image not found.
return &runtime.RemoveImageResponse{}, nil
}
return nil, fmt.Errorf("can not resolve %q locally: %w", r.GetImage().GetImage(), err)
}

span.SetAttributes(tracing.SpanAttribute("image.id", image.ID))
// Remove all image references.
for i, ref := range image.References {
var opts []images.DeleteOpt
Expand Down
4 changes: 4 additions & 0 deletions pkg/cri/sbserver/image_status.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/containerd/containerd/errdefs"
"github.com/containerd/containerd/log"
imagestore "github.com/containerd/containerd/pkg/cri/store/image"
"github.com/containerd/containerd/tracing"

imagespec "github.com/opencontainers/image-spec/specs-go/v1"
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
Expand All @@ -33,14 +34,17 @@ import (
// TODO(random-liu): We should change CRI to distinguish image id and image spec. (See
// kubernetes/kubernetes#46255)
func (c *criService) ImageStatus(ctx context.Context, r *runtime.ImageStatusRequest) (*runtime.ImageStatusResponse, error) {
span := tracing.CurrentSpan(ctx)
image, err := c.localResolve(r.GetImage().GetImage())
if err != nil {
if errdefs.IsNotFound(err) {
span.AddEvent(err.Error())
// return empty without error when image not found.
return &runtime.ImageStatusResponse{}, nil
}
return nil, fmt.Errorf("can not resolve %q locally: %w", r.GetImage().GetImage(), err)
}
span.SetAttributes(tracing.SpanAttribute("image.id", image.ID))
// TODO(random-liu): [P0] Make sure corresponding snapshot exists. What if snapshot
// doesn't exist?

Expand Down
31 changes: 31 additions & 0 deletions pkg/cri/sbserver/instrumented_service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

"github.com/containerd/containerd/errdefs"
"github.com/containerd/containerd/log"
"github.com/containerd/containerd/tracing"
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
runtime_alpha "k8s.io/cri-api/pkg/apis/runtime/v1alpha2"

Expand Down Expand Up @@ -931,6 +932,8 @@ func (in *instrumentedService) PullImage(ctx context.Context, r *runtime.PullIma
if err := in.checkInitialized(); err != nil {
return nil, err
}
ctx, span := tracing.StartSpan(ctx, makeSpanName("PullImage"))
defer tracing.StopSpan(span)
log.G(ctx).Infof("PullImage %q", r.GetImage().GetImage())
defer func() {
if err != nil {
Expand All @@ -939,6 +942,7 @@ func (in *instrumentedService) PullImage(ctx context.Context, r *runtime.PullIma
log.G(ctx).Infof("PullImage %q returns image reference %q",
r.GetImage().GetImage(), res.GetImageRef())
}
tracing.SetSpanStatus(span, err)
}()
res, err = in.c.PullImage(ctrdutil.WithNamespace(ctx), r)
return res, errdefs.ToGRPC(err)
Expand All @@ -948,6 +952,8 @@ func (in *instrumentedAlphaService) PullImage(ctx context.Context, r *runtime_al
if err := in.checkInitialized(); err != nil {
return nil, err
}
ctx, span := tracing.StartSpan(ctx, makeSpanName("PullImage"))
defer tracing.StopSpan(span)
log.G(ctx).Infof("PullImage %q", r.GetImage().GetImage())
defer func() {
if err != nil {
Expand All @@ -956,6 +962,7 @@ func (in *instrumentedAlphaService) PullImage(ctx context.Context, r *runtime_al
log.G(ctx).Infof("PullImage %q returns image reference %q",
r.GetImage().GetImage(), res.GetImageRef())
}
tracing.SetSpanStatus(span, err)
}()
// converts request and response for earlier CRI version to call and get response from the current version
var v1r runtime.PullImageRequest
Expand Down Expand Up @@ -986,6 +993,8 @@ func (in *instrumentedService) ListImages(ctx context.Context, r *runtime.ListIm
if err := in.checkInitialized(); err != nil {
return nil, err
}
ctx, span := tracing.StartSpan(ctx, makeSpanName("ListImages"))
defer tracing.StopSpan(span)
log.G(ctx).Tracef("ListImages with filter %+v", r.GetFilter())
defer func() {
if err != nil {
Expand All @@ -994,6 +1003,7 @@ func (in *instrumentedService) ListImages(ctx context.Context, r *runtime.ListIm
log.G(ctx).Tracef("ListImages with filter %+v returns image list %+v",
r.GetFilter(), res.GetImages())
}
tracing.SetSpanStatus(span, err)
}()
res, err = in.c.ListImages(ctrdutil.WithNamespace(ctx), r)
return res, errdefs.ToGRPC(err)
Expand All @@ -1003,6 +1013,8 @@ func (in *instrumentedAlphaService) ListImages(ctx context.Context, r *runtime_a
if err := in.checkInitialized(); err != nil {
return nil, err
}
ctx, span := tracing.StartSpan(ctx, makeSpanName("ListImages"))
defer tracing.StopSpan(span)
log.G(ctx).Tracef("ListImages with filter %+v", r.GetFilter())
defer func() {
if err != nil {
Expand All @@ -1011,6 +1023,7 @@ func (in *instrumentedAlphaService) ListImages(ctx context.Context, r *runtime_a
log.G(ctx).Tracef("ListImages with filter %+v returns image list %+v",
r.GetFilter(), res.GetImages())
}
tracing.SetSpanStatus(span, err)
}()
// converts request and response for earlier CRI version to call and get response from the current version
var v1r runtime.ListImagesRequest
Expand Down Expand Up @@ -1041,6 +1054,8 @@ func (in *instrumentedService) ImageStatus(ctx context.Context, r *runtime.Image
if err := in.checkInitialized(); err != nil {
return nil, err
}
ctx, span := tracing.StartSpan(ctx, makeSpanName("ImageStatus"))
defer tracing.StopSpan(span)
log.G(ctx).Tracef("ImageStatus for %q", r.GetImage().GetImage())
defer func() {
if err != nil {
Expand All @@ -1049,6 +1064,7 @@ func (in *instrumentedService) ImageStatus(ctx context.Context, r *runtime.Image
log.G(ctx).Tracef("ImageStatus for %q returns image status %+v",
r.GetImage().GetImage(), res.GetImage())
}
tracing.SetSpanStatus(span, err)
}()
res, err = in.c.ImageStatus(ctrdutil.WithNamespace(ctx), r)
return res, errdefs.ToGRPC(err)
Expand All @@ -1058,6 +1074,8 @@ func (in *instrumentedAlphaService) ImageStatus(ctx context.Context, r *runtime_
if err := in.checkInitialized(); err != nil {
return nil, err
}
ctx, span := tracing.StartSpan(ctx, makeSpanName("ImageStatus"))
defer tracing.StopSpan(span)
log.G(ctx).Tracef("ImageStatus for %q", r.GetImage().GetImage())
defer func() {
if err != nil {
Expand All @@ -1066,6 +1084,7 @@ func (in *instrumentedAlphaService) ImageStatus(ctx context.Context, r *runtime_
log.G(ctx).Tracef("ImageStatus for %q returns image status %+v",
r.GetImage().GetImage(), res.GetImage())
}
tracing.SetSpanStatus(span, err)
}()
// converts request and response for earlier CRI version to call and get response from the current version
var v1r runtime.ImageStatusRequest
Expand Down Expand Up @@ -1096,13 +1115,16 @@ func (in *instrumentedService) RemoveImage(ctx context.Context, r *runtime.Remov
if err := in.checkInitialized(); err != nil {
return nil, err
}
ctx, span := tracing.StartSpan(ctx, makeSpanName("RemoveImage"))
defer tracing.StopSpan(span)
log.G(ctx).Infof("RemoveImage %q", r.GetImage().GetImage())
defer func() {
if err != nil {
log.G(ctx).WithError(err).Errorf("RemoveImage %q failed", r.GetImage().GetImage())
} else {
log.G(ctx).Infof("RemoveImage %q returns successfully", r.GetImage().GetImage())
}
tracing.SetSpanStatus(span, err)
}()
res, err := in.c.RemoveImage(ctrdutil.WithNamespace(ctx), r)
return res, errdefs.ToGRPC(err)
Expand All @@ -1112,13 +1134,16 @@ func (in *instrumentedAlphaService) RemoveImage(ctx context.Context, r *runtime_
if err := in.checkInitialized(); err != nil {
return nil, err
}
ctx, span := tracing.StartSpan(ctx, makeSpanName("RemoveImage"))
defer tracing.StopSpan(span)
log.G(ctx).Infof("RemoveImage %q", r.GetImage().GetImage())
defer func() {
if err != nil {
log.G(ctx).WithError(err).Errorf("RemoveImage %q failed", r.GetImage().GetImage())
} else {
log.G(ctx).Infof("RemoveImage %q returns successfully", r.GetImage().GetImage())
}
tracing.SetSpanStatus(span, err)
}()
// converts request and response for earlier CRI version to call and get response from the current version
var v1r runtime.RemoveImageRequest
Expand Down Expand Up @@ -1149,13 +1174,16 @@ func (in *instrumentedService) ImageFsInfo(ctx context.Context, r *runtime.Image
if err := in.checkInitialized(); err != nil {
return nil, err
}
ctx, span := tracing.StartSpan(ctx, makeSpanName("ImageFsInfo"))
defer tracing.StopSpan(span)
log.G(ctx).Debugf("ImageFsInfo")
defer func() {
if err != nil {
log.G(ctx).WithError(err).Error("ImageFsInfo failed")
} else {
log.G(ctx).Debugf("ImageFsInfo returns filesystem info %+v", res.ImageFilesystems)
}
tracing.SetSpanStatus(span, err)
}()
res, err = in.c.ImageFsInfo(ctrdutil.WithNamespace(ctx), r)
return res, errdefs.ToGRPC(err)
Expand All @@ -1165,13 +1193,16 @@ func (in *instrumentedAlphaService) ImageFsInfo(ctx context.Context, r *runtime_
if err := in.checkInitialized(); err != nil {
return nil, err
}
ctx, span := tracing.StartSpan(ctx, makeSpanName("ImageFsInfo"))
defer tracing.StopSpan(span)
log.G(ctx).Debugf("ImageFsInfo")
defer func() {
if err != nil {
log.G(ctx).WithError(err).Error("ImageFsInfo failed")
} else {
log.G(ctx).Debugf("ImageFsInfo returns filesystem info %+v", res.ImageFilesystems)
}
tracing.SetSpanStatus(span, err)
}()
// converts request and response for earlier CRI version to call and get response from the current version
var v1r runtime.ImageFsInfoRequest
Expand Down
12 changes: 12 additions & 0 deletions pkg/cri/server/helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ const (

// runtimeRunhcsV1 is the runtime type for runhcs.
runtimeRunhcsV1 = "io.containerd.runhcs.v1"

// name prefix for CRI server specific spans
criServerSpanPrefix = "pkg.cri.server"

spanDelimiter = "."
)

// makeSandboxName generates sandbox name from sandbox metadata. The name
Expand Down Expand Up @@ -510,3 +515,10 @@ func copyResourcesToStatus(spec *runtimespec.Spec, status containerstore.Status)
}
return status
}

func makeSpanName(funcName string) string {
return strings.Join([]string{
criServerSpanPrefix,
funcName,
}, spanDelimiter)
}
Loading

0 comments on commit b2a01ee

Please sign in to comment.