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

Don't fail on err in exec #259

Merged
merged 5 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
41 changes: 30 additions & 11 deletions data_for_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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 " +
Expand Down Expand Up @@ -369,6 +370,7 @@ var suites = []FixtureSuite{
},
},
},

// regression tests
{
{
Expand Down Expand Up @@ -465,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
{
Expand Down Expand Up @@ -830,17 +849,17 @@ 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,
DetectedLocalhost: true,
ParsedTimeoutMs: 1000,
Endpoint: "*",
EndpointSource: "*",
ExecExitCode: 1,
},
SpanCount: 0,
},
Expand Down Expand Up @@ -919,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,
Expand Down
9 changes: 5 additions & 4 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion otelcli/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"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
Expand Down Expand Up @@ -94,7 +95,10 @@ func doExec(cmd *cobra.Command, args []string) {
}

if err := child.Run(); err != nil {
config.SoftFail("command failed: %s", err)
span.Status = &tracev1.Status{
Message: fmt.Sprintf("exec command failed: %s", err),
Code: tracev1.Status_STATUS_CODE_ERROR,
}
}
span.EndTimeUnixNano = uint64(time.Now().UnixNano())

Expand Down