From ee1b2836af90b5674a932ad9fd64fb708dc6be2a Mon Sep 17 00:00:00 2001 From: Laura Brehm Date: Tue, 2 Apr 2024 11:05:20 +0100 Subject: [PATCH] otel: capture whether process was invoked from a terminal This commit adds a "terminal" attribute to `BaseMetricAttributes` that allows us to discern whether an invocation was from an interactive terminal or not. Signed-off-by: Laura Brehm --- cli/command/telemetry.go | 4 +- cli/command/telemetry_utils.go | 32 ++++++---- cli/command/telemetry_utils_test.go | 95 +++++++++++++++++++++++++++++ cmd/docker/docker.go | 2 +- 4 files changed, 119 insertions(+), 14 deletions(-) diff --git a/cli/command/telemetry.go b/cli/command/telemetry.go index e10571e331..92e80420ac 100644 --- a/cli/command/telemetry.go +++ b/cli/command/telemetry.go @@ -110,7 +110,7 @@ func (r *telemetryResource) init() { return } - opts := append(r.defaultOptions(), r.opts...) + opts := append(defaultResourceOptions(), r.opts...) res, err := resource.New(context.Background(), opts...) if err != nil { otel.Handle(err) @@ -122,7 +122,7 @@ func (r *telemetryResource) init() { r.opts = nil } -func (r *telemetryResource) defaultOptions() []resource.Option { +func defaultResourceOptions() []resource.Option { return []resource.Option{ resource.WithDetectors(serviceNameDetector{}), resource.WithAttributes( diff --git a/cli/command/telemetry_utils.go b/cli/command/telemetry_utils.go index 034fa1f00a..7ee0bb1ff1 100644 --- a/cli/command/telemetry_utils.go +++ b/cli/command/telemetry_utils.go @@ -8,18 +8,18 @@ import ( "time" "github.com/docker/cli/cli/version" + "github.com/moby/term" "github.com/pkg/errors" "github.com/spf13/cobra" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" ) -// BaseMetricAttributes returns an attribute.Set containing attributes to attach to metrics/traces -func BaseMetricAttributes(cmd *cobra.Command) attribute.Set { - attrList := []attribute.KeyValue{ +// BaseCommandAttributes returns an attribute.Set containing attributes to attach to metrics/traces +func BaseCommandAttributes(cmd *cobra.Command, streams Streams) []attribute.KeyValue { + return append([]attribute.KeyValue{ attribute.String("command.name", getCommandName(cmd)), - } - return attribute.NewSet(attrList...) + }, stdioAttributes(streams)...) } // InstrumentCobraCommands wraps all cobra commands' RunE funcs to set a command duration metric using otel. @@ -27,7 +27,7 @@ func BaseMetricAttributes(cmd *cobra.Command) attribute.Set { // Note: this should be the last func to wrap/modify the PersistentRunE/RunE funcs before command execution. // // can also be used for spans! -func InstrumentCobraCommands(cmd *cobra.Command, mp metric.MeterProvider) { +func (cli *DockerCli) InstrumentCobraCommands(cmd *cobra.Command, mp metric.MeterProvider) { meter := getDefaultMeter(mp) // If PersistentPreRunE is nil, make it execute PersistentPreRun and return nil by default ogPersistentPreRunE := cmd.PersistentPreRunE @@ -56,7 +56,8 @@ func InstrumentCobraCommands(cmd *cobra.Command, mp metric.MeterProvider) { } cmd.RunE = func(cmd *cobra.Command, args []string) error { // start the timer as the first step of every cobra command - stopCobraCmdTimer := startCobraCommandTimer(cmd, meter) + baseAttrs := BaseCommandAttributes(cmd, cli) + stopCobraCmdTimer := startCobraCommandTimer(cmd, meter, baseAttrs) cmdErr := ogRunE(cmd, args) stopCobraCmdTimer(cmdErr) return cmdErr @@ -66,9 +67,8 @@ func InstrumentCobraCommands(cmd *cobra.Command, mp metric.MeterProvider) { } } -func startCobraCommandTimer(cmd *cobra.Command, meter metric.Meter) func(err error) { +func startCobraCommandTimer(cmd *cobra.Command, meter metric.Meter, attrs []attribute.KeyValue) func(err error) { ctx := cmd.Context() - baseAttrs := BaseMetricAttributes(cmd) durationCounter, _ := meter.Float64Counter( "command.time", metric.WithDescription("Measures the duration of the cobra command"), @@ -80,12 +80,22 @@ func startCobraCommandTimer(cmd *cobra.Command, meter metric.Meter) func(err err duration := float64(time.Since(start)) / float64(time.Millisecond) cmdStatusAttrs := attributesFromError(err) durationCounter.Add(ctx, duration, - metric.WithAttributeSet(baseAttrs), - metric.WithAttributeSet(attribute.NewSet(cmdStatusAttrs...)), + metric.WithAttributes(attrs...), + metric.WithAttributes(cmdStatusAttrs...), ) } } +func stdioAttributes(streams Streams) []attribute.KeyValue { + // we don't wrap stderr, but we do wrap in/out + _, stderrTty := term.GetFdInfo(streams.Err()) + return []attribute.KeyValue{ + attribute.Bool("command.stdin.isatty", streams.In().IsTerminal()), + attribute.Bool("command.stdout.isatty", streams.Out().IsTerminal()), + attribute.Bool("command.stderr.isatty", stderrTty), + } +} + func attributesFromError(err error) []attribute.KeyValue { attrs := []attribute.KeyValue{} exitCode := 0 diff --git a/cli/command/telemetry_utils_test.go b/cli/command/telemetry_utils_test.go index b70ed0f69c..45daaa9c70 100644 --- a/cli/command/telemetry_utils_test.go +++ b/cli/command/telemetry_utils_test.go @@ -1,9 +1,16 @@ package command import ( + "bytes" + "context" + "io" + "reflect" + "strings" "testing" + "github.com/docker/cli/cli/streams" "github.com/spf13/cobra" + "go.opentelemetry.io/otel/attribute" "gotest.tools/v3/assert" ) @@ -92,3 +99,91 @@ func TestGetCommandName(t *testing.T) { }) } } + +func TestStdioAttributes(t *testing.T) { + outBuffer := new(bytes.Buffer) + errBuffer := new(bytes.Buffer) + t.Parallel() + for _, tc := range []struct { + test string + stdinTty bool + stdoutTty bool + // TODO(laurazard): test stderr + expected []attribute.KeyValue + }{ + { + test: "", + expected: []attribute.KeyValue{ + attribute.Bool("command.stdin.isatty", false), + attribute.Bool("command.stdout.isatty", false), + attribute.Bool("command.stderr.isatty", false), + }, + }, + { + test: "", + stdinTty: true, + stdoutTty: true, + expected: []attribute.KeyValue{ + attribute.Bool("command.stdin.isatty", true), + attribute.Bool("command.stdout.isatty", true), + attribute.Bool("command.stderr.isatty", false), + }, + }, + } { + tc := tc + t.Run(tc.test, func(t *testing.T) { + t.Parallel() + cli := &DockerCli{ + in: streams.NewIn(io.NopCloser(strings.NewReader(""))), + out: streams.NewOut(outBuffer), + err: errBuffer, + } + cli.In().SetIsTerminal(tc.stdinTty) + cli.Out().SetIsTerminal(tc.stdoutTty) + actual := stdioAttributes(cli) + + assert.Check(t, reflect.DeepEqual(actual, tc.expected)) + }) + } +} + +func TestAttributesFromError(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + testName string + err error + expected []attribute.KeyValue + }{ + { + testName: "no error", + err: nil, + expected: []attribute.KeyValue{ + attribute.String("command.status.code", "0"), + }, + }, + { + testName: "non-0 exit code", + err: statusError{StatusCode: 127}, + expected: []attribute.KeyValue{ + attribute.String("command.error.type", "generic"), + attribute.String("command.status.code", "127"), + }, + }, + { + testName: "canceled", + err: context.Canceled, + expected: []attribute.KeyValue{ + attribute.String("command.error.type", "canceled"), + attribute.String("command.status.code", "1"), + }, + }, + } { + tc := tc + t.Run(tc.testName, func(t *testing.T) { + t.Parallel() + actual := attributesFromError(tc.err) + assert.Check(t, reflect.DeepEqual(actual, tc.expected)) + }) + } +} diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index 1b871289ee..16849f3190 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -304,7 +304,7 @@ func runDocker(ctx context.Context, dockerCli *command.DockerCli) error { mp := dockerCli.MeterProvider(ctx) defer mp.Shutdown(ctx) otel.SetMeterProvider(mp) - command.InstrumentCobraCommands(cmd, mp) + dockerCli.InstrumentCobraCommands(cmd, mp) var envs []string args, os.Args, envs, err = processAliases(dockerCli, cmd, args, os.Args)