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 1003f8eb9..53fefa8e7 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 3a39c0dc3..ce9401084 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 50f6cf4fa..fda1a7547 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 0166ef1bc..e759d764e 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 93359e16a..7ca22c643 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 3cb6fe66c..cd3ed93b0 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 6295031cd..577da8fc5 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 6e8e1ba90..93a68c991 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 6b6e99d2d..bbdf94998 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 7dac0e78a..1daa1d077 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 12756f0b1..110e1f5b3 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 42739118d..660a397bd 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 cf1c4377e..7540ac8a9 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 5f2bd56d5..37f6e633b 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 9b4bc27aa..f594957bc 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 }