github/containerd
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

Remove extra span created in the instrumented service layer

Browse Source 
Signed-off-by: Swagat Bora <sbora@amazon.com>
This commit is contained in:
Swagat Bora 2023-02-23 22:42:56 +00:00
parent c0cdcb34f1
commit 83c26962c2
3 changed files with 29 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
internal/cri/instrument/instrumented_service.go
View File

@ -27,11 +27,6 @@ import (
ctrdutil "github.com/containerd/containerd/v2/internal/cri/util"
)
const (
// criSpanPrefix is a prefix for CRI server specific spans
criSpanPrefix = "pkg.cri.server"
)
// criService is an CRI server dependency to be wrapped with instrumentation.
type criService interface {
GRPCServices
@ -69,6 +64,7 @@ func (in *instrumentedService) RunPodSandbox(ctx context.Context, r *runtime.Run
if err := in.checkInitialized(); err != nil {
return nil, err
}
span := tracing.SpanFromContext(ctx)
log.G(ctx).Infof("RunPodSandbox for %+v", r.GetConfig().GetMetadata())
defer func() {
if err != nil {
@ -76,6 +72,7 @@ func (in *instrumentedService) RunPodSandbox(ctx context.Context, r *runtime.Run
} else {
log.G(ctx).Infof("RunPodSandbox for %+v returns sandbox id %q", r.GetConfig().GetMetadata(), res.GetPodSandboxId())
}
span.RecordError(err)
}()
res, err = in.c.RunPodSandbox(ctrdutil.WithNamespace(ctx), r)
return res, errdefs.ToGRPC(err)
@ -117,6 +114,7 @@ func (in *instrumentedService) StopPodSandbox(ctx context.Context, r *runtime.St
if err := in.checkInitialized(); err != nil {
return nil, err
}
span := tracing.SpanFromContext(ctx)
log.G(ctx).Infof("StopPodSandbox for %q", r.GetPodSandboxId())
defer func() {
if err != nil {
@ -124,6 +122,7 @@ func (in *instrumentedService) StopPodSandbox(ctx context.Context, r *runtime.St
} else {
log.G(ctx).Infof("StopPodSandbox for %q returns successfully", r.GetPodSandboxId())
}
span.RecordError(err)
}()
res, err := in.c.StopPodSandbox(ctrdutil.WithNamespace(ctx), r)
return res, errdefs.ToGRPC(err)
@ -133,6 +132,7 @@ func (in *instrumentedService) RemovePodSandbox(ctx context.Context, r *runtime.
if err := in.checkInitialized(); err != nil {
return nil, err
}
span := tracing.SpanFromContext(ctx)
log.G(ctx).Infof("RemovePodSandbox for %q", r.GetPodSandboxId())
defer func() {
if err != nil {
@ -140,6 +140,7 @@ func (in *instrumentedService) RemovePodSandbox(ctx context.Context, r *runtime.
} else {
log.G(ctx).Infof("RemovePodSandbox %q returns successfully", r.GetPodSandboxId())
}
span.RecordError(err)
}()
res, err := in.c.RemovePodSandbox(ctrdutil.WithNamespace(ctx), r)
return res, errdefs.ToGRPC(err)
@ -165,6 +166,7 @@ func (in *instrumentedService) CreateContainer(ctx context.Context, r *runtime.C
if err := in.checkInitialized(); err != nil {
return nil, err
}
span := tracing.SpanFromContext(ctx)
log.G(ctx).Infof("CreateContainer within sandbox %q for container %+v",
r.GetPodSandboxId(), r.GetConfig().GetMetadata())
defer func() {
@ -175,6 +177,7 @@ func (in *instrumentedService) CreateContainer(ctx context.Context, r *runtime.C
log.G(ctx).Infof("CreateContainer within sandbox %q for %+v returns container id %q",
r.GetPodSandboxId(), r.GetConfig().GetMetadata(), res.GetContainerId())
}
span.RecordError(err)
}()
res, err = in.c.CreateContainer(ctrdutil.WithNamespace(ctx), r)
return res, errdefs.ToGRPC(err)
@ -184,6 +187,7 @@ func (in *instrumentedService) StartContainer(ctx context.Context, r *runtime.St
if err := in.checkInitialized(); err != nil {
return nil, err
}
span := tracing.SpanFromContext(ctx)
log.G(ctx).Infof("StartContainer for %q", r.GetContainerId())
defer func() {
if err != nil {
@ -191,6 +195,7 @@ func (in *instrumentedService) StartContainer(ctx context.Context, r *runtime.St
} else {
log.G(ctx).Infof("StartContainer for %q returns successfully", r.GetContainerId())
}
span.RecordError(err)
}()
res, err := in.c.StartContainer(ctrdutil.WithNamespace(ctx), r)
return res, errdefs.ToGRPC(err)
@ -233,6 +238,7 @@ func (in *instrumentedService) StopContainer(ctx context.Context, r *runtime.Sto
if err := in.checkInitialized(); err != nil {
return nil, err
}
span := tracing.SpanFromContext(ctx)
log.G(ctx).Infof("StopContainer for %q with timeout %d (s)", r.GetContainerId(), r.GetTimeout())
defer func() {
if err != nil {
@ -240,6 +246,7 @@ func (in *instrumentedService) StopContainer(ctx context.Context, r *runtime.Sto
} else {
log.G(ctx).Infof("StopContainer for %q returns successfully", r.GetContainerId())
}
span.RecordError(err)
}()
res, err = in.c.StopContainer(ctrdutil.WithNamespace(ctx), r)
return res, errdefs.ToGRPC(err)
@ -249,6 +256,7 @@ func (in *instrumentedService) RemoveContainer(ctx context.Context, r *runtime.R
if err := in.checkInitialized(); err != nil {
return nil, err
}
span := tracing.SpanFromContext(ctx)
log.G(ctx).Infof("RemoveContainer for %q", r.GetContainerId())
defer func() {
if err != nil {
@ -256,6 +264,7 @@ func (in *instrumentedService) RemoveContainer(ctx context.Context, r *runtime.R
} else {
log.G(ctx).Infof("RemoveContainer for %q returns successfully", r.GetContainerId())
}
span.RecordError(err)
}()
res, err = in.c.RemoveContainer(ctrdutil.WithNamespace(ctx), r)
return res, errdefs.ToGRPC(err)
@ -265,6 +274,7 @@ func (in *instrumentedService) ExecSync(ctx context.Context, r *runtime.ExecSync
if err := in.checkInitialized(); err != nil {
return nil, err
}
span := tracing.SpanFromContext(ctx)
log.G(ctx).Debugf("ExecSync for %q with command %+v and timeout %d (s)", r.GetContainerId(), r.GetCmd(), r.GetTimeout())
defer func() {
if err != nil {
@ -272,6 +282,7 @@ func (in *instrumentedService) ExecSync(ctx context.Context, r *runtime.ExecSync
} else {
log.G(ctx).Tracef("ExecSync for %q returns with exit code %d", r.GetContainerId(), res.GetExitCode())
}
span.RecordError(err)
}()
res, err = in.c.ExecSync(ctrdutil.WithNamespace(ctx), r)
return res, errdefs.ToGRPC(err)
@ -281,6 +292,7 @@ func (in *instrumentedService) Exec(ctx context.Context, r *runtime.ExecRequest)
if err := in.checkInitialized(); err != nil {
return nil, err
}
span := tracing.SpanFromContext(ctx)
log.G(ctx).Debugf("Exec for %q with command %+v, tty %v and stdin %v",
r.GetContainerId(), r.GetCmd(), r.GetTty(), r.GetStdin())
defer func() {
@ -289,6 +301,7 @@ func (in *instrumentedService) Exec(ctx context.Context, r *runtime.ExecRequest)
} else {
log.G(ctx).Debugf("Exec for %q returns URL %q", r.GetContainerId(), res.GetUrl())
}
span.RecordError(err)
}()
res, err = in.c.Exec(ctrdutil.WithNamespace(ctx), r)
return res, errdefs.ToGRPC(err)
@ -298,6 +311,7 @@ func (in *instrumentedService) Attach(ctx context.Context, r *runtime.AttachRequ
if err := in.checkInitialized(); err != nil {
return nil, err
}
span := tracing.SpanFromContext(ctx)
log.G(ctx).Debugf("Attach for %q with tty %v and stdin %v", r.GetContainerId(), r.GetTty(), r.GetStdin())
defer func() {
if err != nil {
@ -305,6 +319,7 @@ func (in *instrumentedService) Attach(ctx context.Context, r *runtime.AttachRequ
} else {
log.G(ctx).Debugf("Attach for %q returns URL %q", r.GetContainerId(), res.Url)
}
span.RecordError(err)
}()
res, err = in.c.Attach(ctrdutil.WithNamespace(ctx), r)
return res, errdefs.ToGRPC(err)
@ -330,8 +345,7 @@ func (in *instrumentedService) PullImage(ctx context.Context, r *runtime.PullIma
if err := in.checkInitialized(); err != nil {
return nil, err
}
ctx, span := tracing.StartSpan(ctx, tracing.Name(criSpanPrefix, "PullImage"))
defer span.End()
span := tracing.SpanFromContext(ctx)
log.G(ctx).Infof("PullImage %q", r.GetImage().GetImage())
defer func() {
if err != nil {
@ -340,7 +354,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())
}
span.SetStatus(err)
span.RecordError(err)
}()
res, err = in.c.PullImage(ctrdutil.WithNamespace(ctx), r)
return res, errdefs.ToGRPC(err)
@ -350,8 +364,6 @@ func (in *instrumentedService) ListImages(ctx context.Context, r *runtime.ListIm
if err := in.checkInitialized(); err != nil {
return nil, err
}
ctx, span := tracing.StartSpan(ctx, tracing.Name(criSpanPrefix, "ListImages"))
defer span.End()
log.G(ctx).Tracef("ListImages with filter %+v", r.GetFilter())
defer func() {
if err != nil {
@ -360,7 +372,6 @@ 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())
}
span.SetStatus(err)
}()
res, err = in.c.ListImages(ctrdutil.WithNamespace(ctx), r)
return res, errdefs.ToGRPC(err)
@ -370,8 +381,6 @@ func (in *instrumentedService) ImageStatus(ctx context.Context, r *runtime.Image
if err := in.checkInitialized(); err != nil {
return nil, err
}
ctx, span := tracing.StartSpan(ctx, tracing.Name(criSpanPrefix, "ImageStatus"))
defer span.End()
log.G(ctx).Tracef("ImageStatus for %q", r.GetImage().GetImage())
defer func() {
if err != nil {
@ -380,7 +389,6 @@ 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())
}
span.SetStatus(err)
}()
res, err = in.c.ImageStatus(ctrdutil.WithNamespace(ctx), r)
return res, errdefs.ToGRPC(err)
@ -390,8 +398,7 @@ func (in *instrumentedService) RemoveImage(ctx context.Context, r *runtime.Remov
if err := in.checkInitialized(); err != nil {
return nil, err
}
ctx, span := tracing.StartSpan(ctx, tracing.Name(criSpanPrefix, "RemoveImage"))
defer span.End()
span := tracing.SpanFromContext(ctx)
log.G(ctx).Infof("RemoveImage %q", r.GetImage().GetImage())
defer func() {
if err != nil {
@ -399,7 +406,7 @@ func (in *instrumentedService) RemoveImage(ctx context.Context, r *runtime.Remov
} else {
log.G(ctx).Infof("RemoveImage %q returns successfully", r.GetImage().GetImage())
}
span.SetStatus(err)
span.RecordError(err)
}()
res, err := in.c.RemoveImage(ctrdutil.WithNamespace(ctx), r)
return res, errdefs.ToGRPC(err)
@ -409,8 +416,6 @@ func (in *instrumentedService) ImageFsInfo(ctx context.Context, r *runtime.Image
if err := in.checkInitialized(); err != nil {
return nil, err
}
ctx, span := tracing.StartSpan(ctx, tracing.Name(criSpanPrefix, "ImageFsInfo"))
defer span.End()
log.G(ctx).Tracef("ImageFsInfo")
defer func() {
if err != nil {
@ -418,7 +423,6 @@ func (in *instrumentedService) ImageFsInfo(ctx context.Context, r *runtime.Image
} else {
log.G(ctx).Tracef("ImageFsInfo returns filesystem info %+v", res.ImageFilesystems)
}
span.SetStatus(err)
}()
res, err = in.c.ImageFsInfo(ctrdutil.WithNamespace(ctx), r)
return res, errdefs.ToGRPC(err)

4
internal/cri/server/images/image_status.go
View File

@ -25,7 +25,6 @@ import (
imagestore "github.com/containerd/containerd/v2/internal/cri/store/image"
"github.com/containerd/containerd/v2/internal/cri/util"
"github.com/containerd/containerd/v2/pkg/tracing"
"github.com/containerd/errdefs"
"github.com/containerd/log"
@ -37,17 +36,14 @@ import (
// TODO(random-liu): We should change CRI to distinguish image id and image spec. (See
// kubernetes/kubernetes#46255)
func (c *CRIImageService) ImageStatus(ctx context.Context, r *runtime.ImageStatusRequest) (*runtime.ImageStatusResponse, error) {
span := tracing.SpanFromContext(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.Attribute("image.id", image.ID))
// TODO(random-liu): [P0] Make sure corresponding snapshot exists. What if snapshot
// doesn't exist?

5
pkg/tracing/tracing.go
View File

@ -92,6 +92,11 @@ func (s *Span) AddEvent(name string, attributes ...attribute.KeyValue) {
s.otelSpan.AddEvent(name, trace.WithAttributes(attributes...))
}
// RecordError will record err as an exception span event for this span
func (s *Span) RecordError(err error, options ...trace.EventOption) {
s.otelSpan.RecordError(err, options...)
}
// SetStatus sets the status of the current span.
// If an error is encountered, it records the error and sets span status to Error.
func (s *Span) SetStatus(err error) {