Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add additional data to exec traces #1590

Merged
merged 3 commits into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions ee/tables/tablehelpers/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions pkg/traces/traces.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
directionless marked this conversation as resolved.
Show resolved Hide resolved
}

// buildAttributes takes the given keyVals, expected to be pairs representing the key
Expand Down
Loading