From 9d2afd794d8bcc06685501455170a22afa1e9b7d Mon Sep 17 00:00:00 2001 From: parsonsmatt Date: Thu, 31 Aug 2023 15:55:05 -0600 Subject: [PATCH 1/5] Don't fail on err in exec --- otelcli/exec.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/otelcli/exec.go b/otelcli/exec.go index 9c69201..8724df5 100644 --- a/otelcli/exec.go +++ b/otelcli/exec.go @@ -94,7 +94,8 @@ func doExec(cmd *cobra.Command, args []string) { } if err := child.Run(); err != nil { - config.SoftFail("command failed: %s", err) + span.SetStatus(codes.Error) + span.RecordError(err) } span.EndTimeUnixNano = uint64(time.Now().UnixNano()) From 7094069f7cb3d882f0e4a075fa11d38f04018222 Mon Sep 17 00:00:00 2001 From: parsonsmatt Date: Thu, 31 Aug 2023 17:02:13 -0600 Subject: [PATCH 2/5] Try to make a test --- data_for_test.go | 14 ++++++++++++++ otelcli/exec.go | 7 +++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/data_for_test.go b/data_for_test.go index 2401486..0cff5f9 100644 --- a/data_for_test.go +++ b/data_for_test.go @@ -369,6 +369,20 @@ var suites = []FixtureSuite{ }, }, }, + // exec false reports a span + { + { + Name: "#258 Commands that exit with a non-zero exit code should report a span", + Config: FixtureConfig{ + CliArgs: []string{"exec", "--endpoint", "{{endpoint}}", "--", "false"}, + }, + Expect: Results{ + SpanCount: 1, + CommandFailed: true, + Config: otelcli.DefaultConfig().WithEndpoint("grpc://{{endpoint}}"), + }, + }, + }, // regression tests { { diff --git a/otelcli/exec.go b/otelcli/exec.go index 8724df5..b4c23c1 100644 --- a/otelcli/exec.go +++ b/otelcli/exec.go @@ -9,6 +9,7 @@ import ( "strings" "time" + tracev1 "go.opentelemetry.io/proto/otlp/trace/v1" "github.com/equinix-labs/otel-cli/otlpclient" "github.com/spf13/cobra" ) @@ -94,8 +95,10 @@ func doExec(cmd *cobra.Command, args []string) { } if err := child.Run(); err != nil { - span.SetStatus(codes.Error) - span.RecordError(err) + span.Status = & tracev1.Status { + Message: fmt.Sprintln("exec command failed: ", err), + Code: tracev1.Status_STATUS_CODE_ERROR, + } } span.EndTimeUnixNano = uint64(time.Now().UnixNano()) From ce67e55aa97750c4d4e44bcef13aa69e6aeb1999 Mon Sep 17 00:00:00 2001 From: parsonsmatt Date: Thu, 7 Sep 2023 16:50:09 -0600 Subject: [PATCH 3/5] comment out test --- data_for_test.go | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/data_for_test.go b/data_for_test.go index 0cff5f9..159d590 100644 --- a/data_for_test.go +++ b/data_for_test.go @@ -370,19 +370,22 @@ var suites = []FixtureSuite{ }, }, // exec false reports a span - { - { - Name: "#258 Commands that exit with a non-zero exit code should report a span", - Config: FixtureConfig{ - CliArgs: []string{"exec", "--endpoint", "{{endpoint}}", "--", "false"}, - }, - Expect: Results{ - SpanCount: 1, - CommandFailed: true, - Config: otelcli.DefaultConfig().WithEndpoint("grpc://{{endpoint}}"), - }, - }, - }, + // { + // { + // Name: "#258 Commands that exit with a non-zero exit code should report a span", + // Config: FixtureConfig{ + // CliArgs: []string{"exec", "--endpoint", "{{endpoint}}", "--", "false"}, + // }, + // Expect: Results{ + // SpanCount: 1, + // CliOutput: "", + // Diagnostics: otelcli.Diagnostics{ + // ExecExitCode: 1, + // }, + // Config: otelcli.DefaultConfig().WithEndpoint("grpc://{{endpoint}}"), + // }, + // }, + // }, // regression tests { { From 0c7df7ea80547a66676765258a298df7eafdb94c Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Thu, 21 Sep 2023 11:30:37 -0400 Subject: [PATCH 4/5] use fmt.Sprintf, reformat --- otelcli/exec.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/otelcli/exec.go b/otelcli/exec.go index b4c23c1..b6225b5 100644 --- a/otelcli/exec.go +++ b/otelcli/exec.go @@ -9,9 +9,9 @@ import ( "strings" "time" - tracev1 "go.opentelemetry.io/proto/otlp/trace/v1" "github.com/equinix-labs/otel-cli/otlpclient" "github.com/spf13/cobra" + tracev1 "go.opentelemetry.io/proto/otlp/trace/v1" ) // execCmd sets up the `otel-cli exec` command @@ -95,10 +95,10 @@ func doExec(cmd *cobra.Command, args []string) { } if err := child.Run(); err != nil { - span.Status = & tracev1.Status { - Message: fmt.Sprintln("exec command failed: ", err), - Code: tracev1.Status_STATUS_CODE_ERROR, - } + span.Status = &tracev1.Status{ + Message: fmt.Sprintf("exec command failed: %s", err), + Code: tracev1.Status_STATUS_CODE_ERROR, + } } span.EndTimeUnixNano = uint64(time.Now().UnixNano()) From aa5a2147b05821f9b177e4b5bc24113d729b217f Mon Sep 17 00:00:00 2001 From: Amy Tobey Date: Thu, 21 Sep 2023 12:02:21 -0400 Subject: [PATCH 5/5] rework failure testing Added ExitCode to the results struct so it's easier to test against. I'm trying to remove Diagnostics eventually so it should move anyways. Reworked some failure catching to be more robust for exec. Tests pass :) --- data_for_test.go | 58 +++++++++++++++++++++++++----------------------- main_test.go | 9 ++++---- 2 files changed, 35 insertions(+), 32 deletions(-) diff --git a/data_for_test.go b/data_for_test.go index 159d590..44a9933 100644 --- a/data_for_test.go +++ b/data_for_test.go @@ -70,7 +70,8 @@ type Results struct { SpanCount int // number of spans received EventCount int // number of events received TimedOut bool // true when test timed out - CommandFailed bool // otel-cli failed / was killed + CommandFailed bool // otel-cli was killed, did not exit() on its own + ExitCode int // the process exit code returned by otel-cli Span *tracepb.Span SpanEvents []*tracepb.Span_Event } @@ -326,8 +327,8 @@ var suites = []FixtureSuite{ }, }, Expect: Results{ - Config: otelcli.DefaultConfig(), - CommandFailed: true, + Config: otelcli.DefaultConfig(), + ExitCode: 1, // strips the date off the log line before comparing to expectation CliOutputRe: regexp.MustCompile(`^\d{4}/\d{2}/\d{2} \d{2}:\d{2}:\d{2} `), CliOutput: "Error while loading environment variables: could not parse OTEL_CLI_VERBOSE value " + @@ -369,23 +370,7 @@ var suites = []FixtureSuite{ }, }, }, - // exec false reports a span - // { - // { - // Name: "#258 Commands that exit with a non-zero exit code should report a span", - // Config: FixtureConfig{ - // CliArgs: []string{"exec", "--endpoint", "{{endpoint}}", "--", "false"}, - // }, - // Expect: Results{ - // SpanCount: 1, - // CliOutput: "", - // Diagnostics: otelcli.Diagnostics{ - // ExecExitCode: 1, - // }, - // Config: otelcli.DefaultConfig().WithEndpoint("grpc://{{endpoint}}"), - // }, - // }, - // }, + // regression tests { { @@ -482,6 +467,23 @@ var suites = []FixtureSuite{ }, }, }, + { + Name: "#258 Commands that exit with a non-zero exit code should report a span", + Config: FixtureConfig{ + CliArgs: []string{"exec", + "--endpoint", "{{endpoint}}", + "--verbose", "--fail", + "--", "false", + }, + }, + Expect: Results{ + ExitCode: 1, + SpanCount: 1, + CliOutput: "", + CommandFailed: false, // otel-cli should exit voluntarily in this case + Config: otelcli.DefaultConfig().WithEndpoint("grpc://{{endpoint}}"), + }, + }, }, // otel-cli span with no OTLP config should do and print nothing { @@ -847,10 +849,9 @@ var suites = []FixtureSuite{ TestTimeoutMs: 1000, }, Expect: Results{ - CommandFailed: true, - CliOutputRe: regexp.MustCompile(`^\d{4}/\d{2}/\d{2} \d{2}:\d{2}:\d{2} `), - CliOutput: "invalid protocol setting \"xxx\"\n", - Config: otelcli.DefaultConfig().WithEndpoint("{{endpoint}}"), + CliOutputRe: regexp.MustCompile(`^\d{4}/\d{2}/\d{2} \d{2}:\d{2}:\d{2} `), + CliOutput: "invalid protocol setting \"xxx\"\n", + Config: otelcli.DefaultConfig().WithEndpoint("{{endpoint}}"), Diagnostics: otelcli.Diagnostics{ IsRecording: false, NumArgs: 7, @@ -858,6 +859,7 @@ var suites = []FixtureSuite{ ParsedTimeoutMs: 1000, Endpoint: "*", EndpointSource: "*", + ExecExitCode: 1, }, SpanCount: 0, }, @@ -936,10 +938,10 @@ var suites = []FixtureSuite{ }, }, Expect: Results{ - CommandFailed: true, - CliOutputRe: regexp.MustCompile(`^\d{4}/\d{2}/\d{2} \d{2}:\d{2}:\d{2} `), - CliOutput: "invalid protocol setting \"roflcopter\"\n", - Config: otelcli.DefaultConfig().WithEndpoint("http://{{endpoint}}"), + ExitCode: 1, + CliOutputRe: regexp.MustCompile(`^\d{4}/\d{2}/\d{2} \d{2}:\d{2}:\d{2} `), + CliOutput: "invalid protocol setting \"roflcopter\"\n", + Config: otelcli.DefaultConfig().WithEndpoint("http://{{endpoint}}"), Diagnostics: otelcli.Diagnostics{ IsRecording: false, NumArgs: 3, diff --git a/main_test.go b/main_test.go index 95f4716..df02656 100644 --- a/main_test.go +++ b/main_test.go @@ -167,7 +167,7 @@ func checkAll(t *testing.T, fixture Fixture, results Results) { // many of the basic plumbing tests use status so it has its own set of checks // but these shouldn't run for testing the other subcommands - if len(fixture.Config.CliArgs) > 0 && fixture.Config.CliArgs[0] == "status" && !fixture.Expect.CommandFailed { + if len(fixture.Config.CliArgs) > 0 && fixture.Config.CliArgs[0] == "status" && results.ExitCode == 0 { checkStatusData(t, fixture, results) } else { // checking the text output only makes sense for non-status paths @@ -485,9 +485,10 @@ func runOtelCli(t *testing.T, fixture Fixture) (string, Results) { t.Logf("[%s] going to exec 'env -i %s %s'", fixture.Name, strings.Join(statusCmd.Env, " "), strings.Join(statusCmd.Args, " ")) cliOut, err := statusCmd.CombinedOutput() results.CliOutput = string(cliOut) + results.ExitCode = statusCmd.ProcessState.ExitCode() + results.CommandFailed = !statusCmd.ProcessState.Exited() if err != nil { - results.CommandFailed = true - t.Logf("[%s] executing command failed: %s", fixture.Name, err) + t.Logf("[%s] command exited with status %d: %s", fixture.Name, results.ExitCode, err) } // send stop signals to the timeouts and OTLP server @@ -497,7 +498,7 @@ func runOtelCli(t *testing.T, fixture Fixture) (string, Results) { // only try to parse status json if it was a status command // TODO: support variations on otel-cli where status isn't the first arg - if len(args) > 0 && args[0] == "status" && !fixture.Expect.CommandFailed { + if len(args) > 0 && args[0] == "status" && results.ExitCode == 0 { err = json.Unmarshal(cliOut, &results) if err != nil { t.Errorf("[%s] parsing otel-cli status output failed: %s", fixture.Name, err)