-
Notifications
You must be signed in to change notification settings - Fork 59
Description
G'day! I'd expected otel-cli exec to invoke its first argument with its remaining arguments, so the first result below surprised me until I dug into the source code and saw it matched the second:
$ otel-cli exec -- bash -c 'echo a; echo b'
b
$ sh -c 'bash -c echo a; echo b'
b
I'd hoped for stricter argument passing so I could put otel-cli anywhere without having to troubleshoot whitespace and escaping in arguments, eg. so I could safely put otel-cli in a shebang to wrap a script:
#!/usr/bin/env otel-cli exec -- python3
import os, sys
print("Arguments:", repr(sys.argv))
print("Traceparent:", os.environ["TRACEPARENT"])
Propagation works, but with the current argument handling the argument gets split on its whitespace:
$ ./script.py "hello there"
Arguments: ['./script.py', 'hello', 'there']
Traceparent: 00-e16154a608fa3a412d3f8f4739a950a2-ea907477c0ba37b7-01
Straight argument passing would be a breaking change demanding a minor version bump given your major is zero… I reckon at about the same scale as #181? A lot of people won't notice, but it's worth documenting.
Proof-of-concept patch:
The following fixes the problem for POSIX-flavoured systems only:
diff --git otelcli/exec.go otelcli/exec.go
index 46df894..31560c1 100644
--- otelcli/exec.go
+++ otelcli/exec.go
@@ -52,13 +52,10 @@ func doExec(cmd *cobra.Command, args []string) {
- // use cmd.exe to launch PowerShell on Windows
- var shell string
- if runtime.GOOS == "windows" {
- shell = "cmd.exe"
+ var child *exec.Cmd
+ if runtime.GOOS == "windows" { // use cmd.exe to launch PowerShell on Windows
commandString = "/C powershell " + commandString
+ child = exec.Command("cmd.exe", "-c", commandString)
} else {
- shell = "/bin/sh"
+ child = exec.Command(args[0], args[1:]...)
}
- child := exec.Command(shell, "-c", commandString)
-
// attach all stdio to the parent's handles
A full PR might remove the special case for Windows, leaving that to the Go standard library (syscall.StartProcess?).