From 5c3dc7ca89abd3542a7910f53446cecc42501219 Mon Sep 17 00:00:00 2001 From: seph Date: Thu, 8 Feb 2024 14:37:00 -0500 Subject: [PATCH 1/3] add to traces --- ee/tables/tablehelpers/exec.go | 12 ++++++++++++ pkg/traces/traces.go | 4 ++++ 2 files changed, 16 insertions(+) diff --git a/ee/tables/tablehelpers/exec.go b/ee/tables/tablehelpers/exec.go index 411cafbf6..a34583967 100644 --- a/ee/tables/tablehelpers/exec.go +++ b/ee/tables/tablehelpers/exec.go @@ -5,12 +5,14 @@ import ( "context" "fmt" "os" + "path/filepath" "time" "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" "github.com/kolide/launcher/ee/allowedcmd" "github.com/kolide/launcher/pkg/traces" + "go.opentelemetry.io/otel/attribute" ) // Exec is a wrapper over exec.CommandContext. It does a couple of @@ -40,6 +42,10 @@ func Exec(ctx context.Context, logger log.Logger, timeoutSeconds int, execCmd al return nil, fmt.Errorf("creating command: %w", err) } + span.SetAttributes(attribute.String("exec.path", cmd.Path)) + span.SetAttributes(attribute.String("exec.binary", filepath.Base(cmd.Path))) + span.SetAttributes(attribute.StringSlice("exec.args", args)) + cmd.Stdout = &stdout if includeStderr { cmd.Stderr = &stdout @@ -52,11 +58,17 @@ func Exec(ctx context.Context, logger log.Logger, timeoutSeconds int, execCmd al "cmd", cmd.String(), ) + // FIXME: log if the error is a timeout + switch err := cmd.Run(); { case err == nil: return stdout.Bytes(), nil case os.IsNotExist(err): return nil, fmt.Errorf("could not find %s to run: %w", cmd.Path, err) + case ctx.Err() != nil: + // ctx.Err() should only be set if the context is canceled or done + traces.SetError(span, ctx.Err()) + return nil, fmt.Errorf("context canceled during exec '%s'. Got: '%s': %w", cmd.String(), stderr.String(), ctx.Err()) default: traces.SetError(span, err) return nil, fmt.Errorf("exec '%s'. Got: '%s': %w", cmd.String(), stderr.String(), err) diff --git a/pkg/traces/traces.go b/pkg/traces/traces.go index fc67d7f13..150981e91 100644 --- a/pkg/traces/traces.go +++ b/pkg/traces/traces.go @@ -77,8 +77,12 @@ func startSpanWithExtractedAttributes(ctx context.Context, keyVals ...interface{ // SetError records the error on the span and sets the span's status to error. func SetError(span trace.Span, err error) { + // These are some otel ways to record errors. But we're not sure where they come through in GCP traces span.RecordError(err) span.SetStatus(codes.Error, err.Error()) + + // Dump the error into a span attribute, because :shrug: + span.SetAttributes(attribute.String("error.message", err.Error())) } // buildAttributes takes the given keyVals, expected to be pairs representing the key From b3e709602eb2d14195fd34c7447bf456023e9006 Mon Sep 17 00:00:00 2001 From: seph Date: Thu, 8 Feb 2024 14:45:34 -0500 Subject: [PATCH 2/3] Update pkg/traces/traces.go Co-authored-by: Rebecca Mahany-Horton --- pkg/traces/traces.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/traces/traces.go b/pkg/traces/traces.go index 150981e91..d01346681 100644 --- a/pkg/traces/traces.go +++ b/pkg/traces/traces.go @@ -82,7 +82,7 @@ func SetError(span trace.Span, err error) { span.SetStatus(codes.Error, err.Error()) // Dump the error into a span attribute, because :shrug: - span.SetAttributes(attribute.String("error.message", err.Error())) + span.SetAttributes(semconv.ExceptionMessage(err.Error())) } // buildAttributes takes the given keyVals, expected to be pairs representing the key From d182e8c2c399b74b45daeadbf92e38311ef76a9c Mon Sep 17 00:00:00 2001 From: seph Date: Thu, 8 Feb 2024 14:52:38 -0500 Subject: [PATCH 3/3] not that --- ee/tables/tablehelpers/exec.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/ee/tables/tablehelpers/exec.go b/ee/tables/tablehelpers/exec.go index a34583967..32944cf6d 100644 --- a/ee/tables/tablehelpers/exec.go +++ b/ee/tables/tablehelpers/exec.go @@ -58,8 +58,6 @@ func Exec(ctx context.Context, logger log.Logger, timeoutSeconds int, execCmd al "cmd", cmd.String(), ) - // FIXME: log if the error is a timeout - switch err := cmd.Run(); { case err == nil: return stdout.Bytes(), nil