From 05a46fede0f4338bb825b8c75e276c6cec33229b Mon Sep 17 00:00:00 2001 From: Chelsea Mafrica Date: Fri, 14 May 2021 14:39:21 -0700 Subject: [PATCH] tracing: Make runtime span attributes more consistent Span attributes (tags) are not consistent in runtime tracing, so designate and use core attributes such source, package, subsystem, and type as span metadata for more understandable output. Use WithAttributes() during span creation to reduce calls to SetAttributes(). Modify Trace() in katautils to accept slice of attributes so multiple functions using different attributes can use it. Fixes #1852 Signed-off-by: Chelsea Mafrica --- src/runtime/containerd-shim-v2/service.go | 3 +-- src/runtime/pkg/katautils/create.go | 8 ++++---- src/runtime/pkg/katautils/hook.go | 8 ++------ src/runtime/pkg/katautils/tracing.go | 7 +++---- src/runtime/virtcontainers/acrn.go | 3 +-- src/runtime/virtcontainers/api.go | 6 +----- src/runtime/virtcontainers/clh.go | 3 +-- src/runtime/virtcontainers/container.go | 3 +-- src/runtime/virtcontainers/factory/factory.go | 3 +-- src/runtime/virtcontainers/fc.go | 3 +-- src/runtime/virtcontainers/kata_agent.go | 3 +-- src/runtime/virtcontainers/network.go | 3 +-- src/runtime/virtcontainers/qemu.go | 3 +-- src/runtime/virtcontainers/sandbox.go | 3 +-- src/runtime/virtcontainers/virtiofsd.go | 3 +-- 15 files changed, 21 insertions(+), 41 deletions(-) diff --git a/src/runtime/containerd-shim-v2/service.go b/src/runtime/containerd-shim-v2/service.go index 1003f8eb9c..53fefa8e7e 100644 --- a/src/runtime/containerd-shim-v2/service.go +++ b/src/runtime/containerd-shim-v2/service.go @@ -294,8 +294,7 @@ func trace(ctx context.Context, name string) (otelTrace.Span, context.Context) { ctx = context.Background() } tracer := otel.Tracer("kata") - ctx, span := tracer.Start(ctx, name) - span.SetAttributes([]label.KeyValue{label.Key("source").String("runtime"), label.Key("package").String("containerdshim")}...) + ctx, span := tracer.Start(ctx, name, otelTrace.WithAttributes(label.String("source", "runtime"), label.String("package", "containerdshim"))) return span, ctx } diff --git a/src/runtime/pkg/katautils/create.go b/src/runtime/pkg/katautils/create.go index 3a39c0dc3d..ce9401084d 100644 --- a/src/runtime/pkg/katautils/create.go +++ b/src/runtime/pkg/katautils/create.go @@ -104,7 +104,7 @@ func SetEphemeralStorageType(ociSpec specs.Spec) specs.Spec { // CreateSandbox create a sandbox container func CreateSandbox(ctx context.Context, vci vc.VC, ociSpec specs.Spec, runtimeConfig oci.RuntimeConfig, rootFs vc.RootFs, containerID, bundlePath, console string, disableOutput, systemdCgroup bool) (_ vc.VCSandbox, _ vc.Process, err error) { - span, ctx := Trace(ctx, "createSandbox") + span, ctx := Trace(ctx, "CreateSandbox", []label.KeyValue{label.Key("source").String("runtime"), label.Key("package").String("katautils"), label.Key("subsystem").String("sandbox")}...) defer span.End() sandboxConfig, err := oci.SandboxConfig(ociSpec, runtimeConfig, bundlePath, containerID, console, disableOutput, systemdCgroup) @@ -159,7 +159,7 @@ func CreateSandbox(ctx context.Context, vci vc.VC, ociSpec specs.Spec, runtimeCo sid := sandbox.ID() kataUtilsLogger = kataUtilsLogger.WithField("sandbox", sid) - span.SetAttributes(label.Key("sandbox").String(sid)) + span.SetAttributes(label.Key("sandbox_id").String(sid)) containers := sandbox.GetAllContainers() if len(containers) != 1 { @@ -202,7 +202,7 @@ func checkForFIPS(sandboxConfig *vc.SandboxConfig) error { func CreateContainer(ctx context.Context, sandbox vc.VCSandbox, ociSpec specs.Spec, rootFs vc.RootFs, containerID, bundlePath, console string, disableOutput bool) (vc.Process, error) { var c vc.VCContainer - span, ctx := Trace(ctx, "createContainer") + span, ctx := Trace(ctx, "CreateContainer", []label.KeyValue{label.Key("source").String("runtime"), label.Key("package").String("katautils"), label.Key("subsystem").String("sandbox")}...) defer span.End() ociSpec = SetEphemeralStorageType(ociSpec) @@ -228,7 +228,7 @@ func CreateContainer(ctx context.Context, sandbox vc.VCSandbox, ociSpec specs.Sp return vc.Process{}, err } - span.SetAttributes(label.Key("sandbox").String(sandboxID)) + span.SetAttributes(label.Key("sandbox_id").String(sandboxID)) c, err = sandbox.CreateContainer(ctx, contConfig) if err != nil { diff --git a/src/runtime/pkg/katautils/hook.go b/src/runtime/pkg/katautils/hook.go index 50f6cf4fa1..fda1a75478 100644 --- a/src/runtime/pkg/katautils/hook.go +++ b/src/runtime/pkg/katautils/hook.go @@ -26,11 +26,9 @@ func hookLogger() *logrus.Entry { } func runHook(ctx context.Context, hook specs.Hook, cid, bundlePath string) error { - span, _ := Trace(ctx, "hook") + span, _ := Trace(ctx, "runHook", []label.KeyValue{label.Key("source").String("runtime"), label.Key("package").String("katautils"), label.Key("subsystem").String("hook")}...) defer span.End() - span.SetAttributes(label.Key("subsystem").String("runHook")) - // FIXME // span.LogFields( // log.String("hook-name", hook.Path), @@ -90,11 +88,9 @@ func runHook(ctx context.Context, hook specs.Hook, cid, bundlePath string) error } func runHooks(ctx context.Context, hooks []specs.Hook, cid, bundlePath, hookType string) error { - span, _ := Trace(ctx, "hooks") + span, _ := Trace(ctx, "runHooks", []label.KeyValue{label.Key("source").String("runtime"), label.Key("package").String("katautils"), label.Key("subsystem").String("hook"), label.Key("type").String(hookType)}...) defer span.End() - span.SetAttributes(label.Key("subsystem").String(hookType)) - for _, hook := range hooks { if err := runHook(ctx, hook, cid, bundlePath); err != nil { hookLogger().WithFields(logrus.Fields{ diff --git a/src/runtime/pkg/katautils/tracing.go b/src/runtime/pkg/katautils/tracing.go index 0166ef1bc5..e759d764e9 100644 --- a/src/runtime/pkg/katautils/tracing.go +++ b/src/runtime/pkg/katautils/tracing.go @@ -110,12 +110,11 @@ func StopTracing(ctx context.Context) { } // Trace creates a new tracing span based on the specified name and parent -// context. -func Trace(parent context.Context, name string) (otelTrace.Span, context.Context) { +// context and an opentelemetry label.KeyValue slice for span attributes. +func Trace(parent context.Context, name string, tags ...label.KeyValue) (otelTrace.Span, context.Context) { tracer := otel.Tracer("kata") - ctx, span := tracer.Start(parent, name) - span.SetAttributes(label.Key("source").String("runtime")) + ctx, span := tracer.Start(parent, name, otelTrace.WithAttributes(tags...)) // This is slightly confusing: when tracing is disabled, trace spans // are still created - but the tracer used is a NOP. Therefore, only diff --git a/src/runtime/virtcontainers/acrn.go b/src/runtime/virtcontainers/acrn.go index 93359e16ad..7ca22c6430 100644 --- a/src/runtime/virtcontainers/acrn.go +++ b/src/runtime/virtcontainers/acrn.go @@ -214,8 +214,7 @@ func (a *Acrn) trace(parent context.Context, name string) (otelTrace.Span, conte } tracer := otel.Tracer("kata") - ctx, span := tracer.Start(parent, name) - span.SetAttributes([]label.KeyValue{label.Key("subsystem").String("hypervisor"), label.Key("type").String("acrn")}...) + ctx, span := tracer.Start(parent, name, otelTrace.WithAttributes(label.String("source", "runtime"), label.String("package", "virtcontainers"), label.String("subsystem", "hypervisor"), label.String("type", "acrn"))) return span, ctx } diff --git a/src/runtime/virtcontainers/api.go b/src/runtime/virtcontainers/api.go index 3cb6fe66c3..cd3ed93b0e 100644 --- a/src/runtime/virtcontainers/api.go +++ b/src/runtime/virtcontainers/api.go @@ -30,11 +30,7 @@ var virtLog = logrus.WithField("source", "virtcontainers") // context. func trace(parent context.Context, name string) (otelTrace.Span, context.Context) { tracer := otel.Tracer("kata") - ctx, span := tracer.Start(parent, name) - span.SetAttributes([]label.KeyValue{ - label.Key("source").String("virtcontainers"), - label.Key("component").String("virtcontainers"), - label.Key("subsystem").String("api")}...) + ctx, span := tracer.Start(parent, name, otelTrace.WithAttributes(label.String("source", "runtime"), label.String("packages", "virtcontainers"), label.String("subsystem", "api"))) return span, ctx } diff --git a/src/runtime/virtcontainers/clh.go b/src/runtime/virtcontainers/clh.go index 6295031cdf..577da8fc5e 100644 --- a/src/runtime/virtcontainers/clh.go +++ b/src/runtime/virtcontainers/clh.go @@ -764,8 +764,7 @@ func (clh *cloudHypervisor) trace(parent context.Context, name string) (otelTrac } tracer := otel.Tracer("kata") - ctx, span := tracer.Start(parent, name) - span.SetAttributes([]otelLabel.KeyValue{otelLabel.Key("subsystem").String("hypervisor"), otelLabel.Key("type").String("clh")}...) + ctx, span := tracer.Start(parent, name, otelTrace.WithAttributes(otelLabel.String("source", "runtime"), otelLabel.String("package", "virtcontainers"), otelLabel.String("subsystem", "hypervisor"), otelLabel.String("type", "clh"))) return span, ctx } diff --git a/src/runtime/virtcontainers/container.go b/src/runtime/virtcontainers/container.go index 6e8e1ba90d..93a68c9913 100644 --- a/src/runtime/virtcontainers/container.go +++ b/src/runtime/virtcontainers/container.go @@ -360,8 +360,7 @@ func (c *Container) trace(parent context.Context, name string) (otelTrace.Span, } tracer := otel.Tracer("kata") - ctx, span := tracer.Start(parent, name) - span.SetAttributes(otelLabel.Key("subsystem").String("container")) + ctx, span := tracer.Start(parent, name, otelTrace.WithAttributes(otelLabel.String("source", "runtime"), otelLabel.String("package", "virtcontainers"), otelLabel.String("subsystem", "container"))) return span, ctx } diff --git a/src/runtime/virtcontainers/factory/factory.go b/src/runtime/virtcontainers/factory/factory.go index 6b6e99d2d2..bbdf949983 100644 --- a/src/runtime/virtcontainers/factory/factory.go +++ b/src/runtime/virtcontainers/factory/factory.go @@ -42,8 +42,7 @@ type factory struct { func trace(parent context.Context, name string) (otelTrace.Span, context.Context) { tracer := otel.Tracer("kata") - ctx, span := tracer.Start(parent, name) - span.SetAttributes(label.Key("subsystem").String("factory")) + ctx, span := tracer.Start(parent, name, otelTrace.WithAttributes(label.String("source", "runtime"), label.String("package", "factory"), label.String("subsystem", "factory"))) return span, ctx } diff --git a/src/runtime/virtcontainers/fc.go b/src/runtime/virtcontainers/fc.go index 7dac0e78a0..1daa1d077d 100644 --- a/src/runtime/virtcontainers/fc.go +++ b/src/runtime/virtcontainers/fc.go @@ -175,8 +175,7 @@ func (fc *firecracker) trace(parent context.Context, name string) (otelTrace.Spa } tracer := otel.Tracer("kata") - ctx, span := tracer.Start(parent, name) - span.SetAttributes([]otelLabel.KeyValue{otelLabel.Key("subsystem").String("hypervisor"), otelLabel.Key("type").String("firecracker")}...) + ctx, span := tracer.Start(parent, name, otelTrace.WithAttributes(otelLabel.String("source", "runtime"), otelLabel.String("package", "virtcontainers"), otelLabel.String("subsystem", "hypervisor"), otelLabel.String("type", "firecracker"))) return span, ctx } diff --git a/src/runtime/virtcontainers/kata_agent.go b/src/runtime/virtcontainers/kata_agent.go index 12756f0b11..110e1f5b3d 100644 --- a/src/runtime/virtcontainers/kata_agent.go +++ b/src/runtime/virtcontainers/kata_agent.go @@ -251,8 +251,7 @@ func (k *kataAgent) trace(parent context.Context, name string) (otelTrace.Span, } tracer := otel.Tracer("kata") - ctx, span := tracer.Start(parent, name) - span.SetAttributes([]label.KeyValue{label.Key("subsystem").String("agent"), label.Key("type").String("kata")}...) + ctx, span := tracer.Start(parent, name, otelTrace.WithAttributes(label.String("source", "runtime"), label.String("package", "virtcontainers"), label.String("subsystem", "agent"))) return span, ctx } diff --git a/src/runtime/virtcontainers/network.go b/src/runtime/virtcontainers/network.go index 42739118d8..660a397bd6 100644 --- a/src/runtime/virtcontainers/network.go +++ b/src/runtime/virtcontainers/network.go @@ -1255,8 +1255,7 @@ type Network struct { func (n *Network) trace(ctx context.Context, name string) (otelTrace.Span, context.Context) { tracer := otel.Tracer("kata") - ctx, span := tracer.Start(ctx, name) - span.SetAttributes([]otelLabel.KeyValue{otelLabel.Key("subsystem").String("network"), otelLabel.Key("type").String("default")}...) + ctx, span := tracer.Start(ctx, name, otelTrace.WithAttributes(otelLabel.String("source", "runtime"), otelLabel.String("package", "virtcontainers"), otelLabel.String("subsystem", "network"), otelLabel.String("type", "default"))) return span, ctx } diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index cf1c4377ee..7540ac8a93 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/src/runtime/virtcontainers/qemu.go @@ -219,8 +219,7 @@ func (q *qemu) trace(parent context.Context, name string) (otelTrace.Span, conte } tracer := otel.Tracer("kata") - ctx, span := tracer.Start(parent, name) - span.SetAttributes([]otelLabel.KeyValue{otelLabel.Key("subsystem").String("hypervisor"), otelLabel.Key("type").String("qemu")}...) + ctx, span := tracer.Start(parent, name, otelTrace.WithAttributes(otelLabel.String("source", "runtime"), otelLabel.String("package", "virtcontainers"), otelLabel.String("subsystem", "hypervisor"), otelLabel.String("type", "qemu"))) return span, ctx } diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index 5f2bd56d57..37f6e633b0 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/src/runtime/virtcontainers/sandbox.go @@ -134,8 +134,7 @@ func (s *Sandbox) trace(parent context.Context, name string) (otelTrace.Span, co } tracer := otel.Tracer("kata") - ctx, span := tracer.Start(parent, name) - span.SetAttributes(otelLabel.Key("subsystem").String("sandbox")) + ctx, span := tracer.Start(parent, name, otelTrace.WithAttributes(otelLabel.String("source", "runtime"), otelLabel.String("package", "virtcontainers"), otelLabel.String("subsystem", "sandbox"))) return span, ctx } diff --git a/src/runtime/virtcontainers/virtiofsd.go b/src/runtime/virtcontainers/virtiofsd.go index 9b4bc27aac..f594957bc7 100644 --- a/src/runtime/virtcontainers/virtiofsd.go +++ b/src/runtime/virtcontainers/virtiofsd.go @@ -210,8 +210,7 @@ func (v *virtiofsd) trace(parent context.Context, name string) (otelTrace.Span, } tracer := otel.Tracer("kata") - ctx, span := tracer.Start(parent, name) - span.SetAttributes(label.Key("subsystem").String("virtiofds")) + ctx, span := tracer.Start(parent, name, otelTrace.WithAttributes(label.String("source", "runtime"), label.String("package", "virtcontainers"), label.String("subsystem", "virtiofsd"))) return span, ctx }