Skip to content

Commit

Permalink
only print errors with --verbose and always exit 0 (#76)
Browse files Browse the repository at this point in the history
adds the --verbose flag and config value

adds 2 helpers, softLog and softFail that only print if --verbose is
set, and softFail always exits 0 (so otel-cli doesn't surprise people
running under `set -e`)

switches the hot paths in otel-cli to use these new helpers instead of
the log.Printf or log.Fatalf

/cc #56 which this takes care of

Signed-off-by: Amy Tobey <atobey@equinix.com>
  • Loading branch information
Amy Tobey authored Sep 10, 2021
1 parent 141f78d commit f4bbca3
Show file tree
Hide file tree
Showing 9 changed files with 50 additions and 34 deletions.
31 changes: 25 additions & 6 deletions cmd/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"io"
"log"
"os"
"regexp"
"strconv"
"strings"
Expand Down Expand Up @@ -93,19 +94,19 @@ func parseTime(ts, which string) time.Time {

// none of the formats worked, print whatever errors are remaining
if uterr != nil {
log.Fatalf("Could not parse span %s time %q as Unix Epoch: %s", which, ts, uterr)
softFail("Could not parse span %s time %q as Unix Epoch: %s", which, ts, uterr)
}
if utnerr != nil || utnnerr != nil {
log.Fatalf("Could not parse span %s time %q as Unix Epoch.Nano: %s | %s", which, ts, utnerr, utnnerr)
softFail("Could not parse span %s time %q as Unix Epoch.Nano: %s | %s", which, ts, utnerr, utnnerr)
}
if rerr != nil {
log.Fatalf("Could not parse span %s time %q as RFC3339: %s", which, ts, rerr)
softFail("Could not parse span %s time %q as RFC3339: %s", which, ts, rerr)
}
if rnerr != nil {
log.Fatalf("Could not parse span %s time %q as RFC3339Nano: %s", which, ts, rnerr)
softFail("Could not parse span %s time %q as RFC3339Nano: %s", which, ts, rnerr)
}

log.Fatalf("Could not parse span %s time %q as any supported format", which, ts)
softFail("Could not parse span %s time %q as any supported format", which, ts)
return time.Now() // never happens, just here to make compiler happy
}

Expand Down Expand Up @@ -180,7 +181,25 @@ func parseCliTimeout() time.Duration {
} else if secs, serr := strconv.ParseInt(config.Timeout, 10, 0); serr == nil {
return time.Second * time.Duration(secs)
} else {
log.Printf("unable to parse --timeout %q: %s", config.Timeout, err)
softLog("unable to parse --timeout %q: %s", config.Timeout, err)
return time.Duration(0)
}
}

// softLog only calls through to log if otel-cli was run with the --verbose flag.
func softLog(format string, a ...interface{}) {
if !config.Verbose {
return
}
log.Printf(format, a...)
}

// softFail only calls through to log if otel-cli was run with the --verbose
// flag, then immediately exits with status 0.
func softFail(format string, a ...interface{}) {
if !config.Verbose {
return
}
log.Printf(format, a...)
os.Exit(0)
}
9 changes: 4 additions & 5 deletions cmd/otelclicarrier.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package cmd
import (
"bufio"
"context"
"log"
"os"
"regexp"
"strings"
Expand Down Expand Up @@ -72,7 +71,7 @@ func loadTraceparent(ctx context.Context, filename string) context.Context {
return ctx
}
}
log.Fatalf("failed to find a valid traceparent carrier in either environment for file '%s' while it's required by --tp-required", filename)
softFail("failed to find a valid traceparent carrier in either environment for file '%s' while it's required by --tp-required", filename)
}
return ctx
}
Expand All @@ -87,7 +86,7 @@ func loadTraceparentFromFile(ctx context.Context, filename string) context.Conte
// only fatal when the tp carrier file is required explicitly, otherwise
// just silently return the unmodified context
if config.TraceparentRequired {
log.Fatalf("could not open file '%s' for read: %s", filename, err)
softFail("could not open file '%s' for read: %s", filename, err)
} else {
return ctx
}
Expand Down Expand Up @@ -118,7 +117,7 @@ func loadTraceparentFromFile(ctx context.Context, filename string) context.Conte
tp = strings.TrimPrefix(tp, "TRACEPARENT=")

if !checkTracecarrierRe.MatchString(tp) {
log.Printf("file '%s' was read but does not contain a valid traceparent", filename)
softLog("file '%s' was read but does not contain a valid traceparent", filename)
return ctx
}

Expand All @@ -137,7 +136,7 @@ func saveTraceparentToFile(ctx context.Context, filename string) {

file, err := os.OpenFile(filename, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0600)
if err != nil {
log.Printf("failure opening file '%s' for write: %s", filename, err)
softLog("failure opening file '%s' for write: %s", filename, err)
}
defer file.Close()

Expand Down
16 changes: 7 additions & 9 deletions cmd/plumbing.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package cmd
import (
"context"
"crypto/tls"
"log"
"net"
"net/url"
"regexp"
Expand All @@ -21,7 +20,6 @@ import (

// initTracer sets up the OpenTelemetry plumbing so it's ready to use.
// Returns a context and a func() that encapuslates clean shutdown.
//
func initTracer() (context.Context, func()) {
ctx := context.Background()

Expand Down Expand Up @@ -73,14 +71,14 @@ func initTracer() (context.Context, func()) {
var exporter sdktrace.SpanExporter // allows overwrite in --test mode
exporter, err := otlpgrpc.New(ctx, grpcOpts...)
if err != nil {
log.Fatalf("failed to configure OTLP exporter: %s", err)
softFail("failed to configure OTLP exporter: %s", err)
}

// set the service name that will show up in tracing UIs
resAttrs := resource.WithAttributes(semconv.ServiceNameKey.String(config.ServiceName))
res, err := resource.New(ctx, resAttrs)
if err != nil {
log.Fatalf("failed to create OpenTelemetry service name resource: %s", err)
softFail("failed to create OpenTelemetry service name resource: %s", err)
}

// SSP sends all completed spans to the exporter immediately and that is
Expand All @@ -104,12 +102,12 @@ func initTracer() (context.Context, func()) {
return ctx, func() {
err = tracerProvider.Shutdown(ctx)
if err != nil {
log.Fatalf("shutdown of OpenTelemetry tracerProvider failed: %s", err)
softFail("shutdown of OpenTelemetry tracerProvider failed: %s", err)
}

err = exporter.Shutdown(ctx)
if err != nil {
log.Fatalf("shutdown of OpenTelemetry OTLP exporter failed: %s", err)
softFail("shutdown of OpenTelemetry OTLP exporter failed: %s", err)
}
}
}
Expand All @@ -132,16 +130,16 @@ func isLoopbackAddr(endpoint string) bool {
} else if uriRe.MatchString(endpoint) {
u, err := url.Parse(endpoint)
if err != nil {
log.Fatalf("error parsing provided URI '%s': %s", endpoint, err)
softFail("error parsing provided URI '%s': %s", endpoint, err)
}
hostname = u.Hostname()
} else {
log.Fatalf("'%s' is not a valid endpoint, must be host:port or a URI", endpoint)
softFail("'%s' is not a valid endpoint, must be host:port or a URI", endpoint)
}

ips, err := net.LookupIP(hostname)
if err != nil {
log.Fatalf("unable to look up hostname '%s': %s", hostname, err)
softFail("unable to look up hostname '%s': %s", hostname, err)
}

// all ips returned must be loopback to return true
Expand Down
4 changes: 4 additions & 0 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ type Config struct {
EventTime string `json:"event_time"`

CfgFile string `json:"config_file"`
Verbose bool `json:"verbose"`
}

const defaultOtlpEndpoint = "localhost:4317"
Expand Down Expand Up @@ -73,10 +74,13 @@ func addCommonParams(cmd *cobra.Command) {
cmd.Flags().StringVar(&config.Endpoint, "endpoint", "", "dial address for the desired OTLP/gRPC endpoint")
// --timeout a default timeout to use in all otel-cli operations (default 1s)
cmd.Flags().StringVar(&config.Timeout, "timeout", "1s", "timeout for otel-cli operations, all timeouts in otel-cli use this value")
// --verbose tells otel-cli to actually log errors to stderr instead of failing silently
cmd.Flags().BoolVar(&config.Verbose, "verbose", false, "print errors on failure instead of always being silent")

var common_env_flags = map[string]string{
"endpoint": "OTEL_EXPORTER_OTLP_ENDPOINT",
"timeout": "OTEL_EXPORTER_OTLP_TIMEOUT",
"verbose": "OTEL_CLI_VERBOSE",
}

for config_key, env_value := range common_env_flags {
Expand Down
3 changes: 1 addition & 2 deletions cmd/span_background.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package cmd

import (
"log"
"os"
"os/signal"
"path"
Expand Down Expand Up @@ -72,7 +71,7 @@ func doSpanBackground(cmd *cobra.Command, args []string) {
defer shutdown()
err := client.Call("BgSpan.Wait", &struct{}{}, &struct{}{})
if err != nil {
log.Printf("error while waiting on span background: %s", err)
softFail("error while waiting on span background: %s", err)
}
return
}
Expand Down
13 changes: 6 additions & 7 deletions cmd/span_background_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package cmd
import (
"context"
"fmt"
"log"
"net"
"net/rpc"
"net/rpc/jsonrpc"
Expand Down Expand Up @@ -94,7 +93,7 @@ func createBgServer(sockfile string, span trace.Span) *bgServer {

// TODO: be safer?
if err = os.RemoveAll(sockfile); err != nil {
log.Fatalf("failed while cleaning up for socket file '%s': %s", sockfile, err)
softFail("failed while cleaning up for socket file '%s': %s", sockfile, err)
}

bgspan := BgSpan{
Expand All @@ -108,7 +107,7 @@ func createBgServer(sockfile string, span trace.Span) *bgServer {

bgs.listener, err = net.Listen("unix", sockfile)
if err != nil {
log.Fatalf("unable to listen on unix socket '%s': %s", sockfile, err)
softFail("unable to listen on unix socket '%s': %s", sockfile, err)
}

bgs.wg.Add(1) // cleanup will block until this is done
Expand All @@ -126,7 +125,7 @@ func (bgs *bgServer) Run() {
case <-bgs.quit: // quitting gracefully
return
default:
log.Fatalf("error while accepting connection: %s", err)
softFail("error while accepting connection: %s", err)
}
}

Expand Down Expand Up @@ -161,20 +160,20 @@ func createBgClient() (*rpc.Client, func()) {
if os.IsNotExist(err) {
time.Sleep(time.Millisecond * 25) // sleep 25ms between checks
} else if err != nil {
log.Fatalf("failed to stat file '%s': %s", sockfile, err)
softFail("failed to stat file '%s': %s", sockfile, err)
} else {
break
}

if timeout > 0 && time.Since(started) > timeout {
log.Fatalf("timeout after %s while waiting for span background socket '%s' to appear", config.Timeout, sockfile)
softFail("timeout after %s while waiting for span background socket '%s' to appear", config.Timeout, sockfile)
}
}

sock := net.UnixAddr{Name: sockfile, Net: "unix"}
conn, err := net.DialUnix(sock.Net, nil, &sock)
if err != nil {
log.Fatalf("unable to connect to span background server at '%s': %s", config.BackgroundSockdir, err)
softFail("unable to connect to span background server at '%s': %s", config.BackgroundSockdir, err)
}

return jsonrpc.NewClient(conn), func() { conn.Close() }
Expand Down
3 changes: 1 addition & 2 deletions cmd/span_end.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package cmd

import (
"log"
"os"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -33,7 +32,7 @@ func doSpanEnd(cmd *cobra.Command, args []string) {
res := BgSpan{}
err := client.Call("BgSpan.End", rpcArgs, &res)
if err != nil {
log.Fatalf("error while calling background server rpc BgSpan.End: %s", err)
softFail("error while calling background server rpc BgSpan.End: %s", err)
}
shutdown()

Expand Down
3 changes: 1 addition & 2 deletions cmd/span_event.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package cmd

import (
"log"
"os"
"time"

Expand Down Expand Up @@ -52,7 +51,7 @@ func doSpanEvent(cmd *cobra.Command, args []string) {
defer shutdown()
err := client.Call("BgSpan.AddEvent", rpcArgs, &res)
if err != nil {
log.Fatalf("error while calling background server rpc BgSpan.AddEvent: %s", err)
softFail("error while calling background server rpc BgSpan.AddEvent: %s", err)
}

printSpanData(os.Stdout, res.TraceID, res.SpanID, res.Traceparent)
Expand Down
2 changes: 1 addition & 1 deletion cmd/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func doStatus(cmd *cobra.Command, args []string) {
env[parts[0]] = parts[1]
}
} else {
log.Fatalf("BUG in otel-cli: this shouldn't happen")
softFail("BUG in otel-cli: this shouldn't happen")
}
}

Expand Down

0 comments on commit f4bbca3

Please sign in to comment.