Skip to content

Commit

Permalink
move config globals into a config struct (#69)
Browse files Browse the repository at this point in the history 
* move most of the config globals into a config struct
* remove unused timeout in jsonSvr struct
* move more globals into config struct

Signed-off-by: Amy Tobey <atobey@equinix.com>
  • Loading branch information
Amy Tobey authored Sep 9, 2021
1 parent 7d2709f commit 44d2088
Show file tree
Hide file tree
Showing 13 changed files with 125 additions and 116 deletions.
10 changes: 5 additions & 5 deletions cmd/exec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,16 @@ func init() {
func doExec(cmd *cobra.Command, args []string) {
ctx, shutdown := initTracer()
defer shutdown()
ctx = loadTraceparent(ctx, traceparentCarrierFile)
ctx = loadTraceparent(ctx, config.TraceparentCarrierFile)
tracer := otel.Tracer("otel-cli/exec")

// joining the string here is kinda gross... but should be fine
// there might be a better way in Cobra, maybe require passing it after a '--'?
commandString := strings.Join(args, " ")

kindOption := trace.WithSpanKind(otelSpanKind(spanKind))
ctx, span := tracer.Start(ctx, spanName, kindOption)
span.SetAttributes(cliAttrsToOtel(spanAttrs)...) // applies CLI attributes to the span
kindOption := trace.WithSpanKind(otelSpanKind(config.Kind))
ctx, span := tracer.Start(ctx, config.SpanName, kindOption)
span.SetAttributes(cliAttrsToOtel(config.Attributes)...) // applies CLI attributes to the span

// put the command in the attributes
span.SetAttributes(attribute.KeyValue{
Expand All @@ -71,7 +71,7 @@ func doExec(cmd *cobra.Command, args []string) {
// pass the existing env but add the latest TRACEPARENT carrier so e.g.
// otel-cli exec 'otel-cli exec sleep 1' will relate the spans automatically
child.Env = os.Environ()
if !traceparentIgnoreEnv {
if !config.TraceparentIgnoreEnv {
child.Env = append(child.Env, fmt.Sprintf("TRACEPARENT=%s", getTraceparent(ctx)))
}

Expand Down
12 changes: 6 additions & 6 deletions cmd/helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func GetExitCode() int {
// propagateOtelCliSpan saves the traceparent to file if necessary, then prints
// span info to the console according to command-line args.
func propagateOtelCliSpan(ctx context.Context, span trace.Span, target io.Writer) {
saveTraceparentToFile(ctx, traceparentCarrierFile)
saveTraceparentToFile(ctx, config.TraceparentCarrierFile)

tpout := getTraceparent(ctx)
tid := span.SpanContext().TraceID().String()
Expand All @@ -151,14 +151,14 @@ func propagateOtelCliSpan(ctx context.Context, span trace.Span, target io.Writer
// depending on which command line arguments were set.
func printSpanData(target io.Writer, traceId, spanId, tp string) {
// --tp-print / --tp-export
if !traceparentPrint && !traceparentPrintExport {
if !config.TraceparentPrint && !config.TraceparentPrintExport {
return
}

// --tp-export will print "export TRACEPARENT" so it's
// one less step to print to a file & source, or eval
var exported string
if traceparentPrintExport {
if config.TraceparentPrintExport {
exported = "export "
}

Expand All @@ -168,14 +168,14 @@ func printSpanData(target io.Writer, traceId, spanId, tp string) {
// parseCliTimeout parses the cliTimeout global string value to a time.Duration.
// It logs an error and returns time.Duration(0) if the string is empty or unparseable.
func parseCliTimeout() time.Duration {
if cliTimeout == "" {
if config.Timeout == "" {
return time.Duration(0)
}

if d, err := time.ParseDuration(cliTimeout); err == nil {
if d, err := time.ParseDuration(config.Timeout); err == nil {
return d
} else {
log.Printf("unable to parse --timeout %q: %s", cliTimeout, err)
log.Printf("unable to parse --timeout %q: %s", config.Timeout, err)
return time.Duration(0)
}
}
16 changes: 9 additions & 7 deletions cmd/helpers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,11 @@ func TestPropagateOtelCliSpan(t *testing.T) {
// TODO: should this noop the tracing backend?

// set package globals to a known state
traceparentCarrierFile = ""
traceparentPrint = false
traceparentPrintExport = false
config = Config{
TraceparentCarrierFile: "",
TraceparentPrint: false,
TraceparentPrintExport: false,
}

tp := "00-3433d5ae39bdfee397f44be5146867b3-8a5518f1e5c54d0a-01"
tid := "3433d5ae39bdfee397f44be5146867b3"
Expand All @@ -187,8 +189,8 @@ func TestPropagateOtelCliSpan(t *testing.T) {
t.Errorf("nothing was supposed to be written but %d bytes were", buf.Len())
}

traceparentPrint = true
traceparentPrintExport = true
config.TraceparentPrint = true
config.TraceparentPrintExport = true
buf = new(bytes.Buffer)
printSpanData(buf, tid, sid, tp)
if buf.Len() == 0 {
Expand Down Expand Up @@ -230,12 +232,12 @@ func TestParseCliTime(t *testing.T) {
},
} {
t.Run(testcase.name, func(t *testing.T) {
cliTimeout = testcase.input
config = Config{Timeout: testcase.input}
got := parseCliTimeout()
if got != testcase.expected {
ed := testcase.expected.String()
gd := got.String()
t.Errorf("duration string %q was expected to return %s but returned %s", cliTimeout, ed, gd)
t.Errorf("duration string %q was expected to return %s but returned %s", config.Timeout, ed, gd)
}
})
}
Expand Down
6 changes: 3 additions & 3 deletions cmd/otelclicarrier.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func loadTraceparent(ctx context.Context, filename string) context.Context {
if filename != "" {
ctx = loadTraceparentFromFile(ctx, filename)
}
if traceparentRequired {
if config.TraceparentRequired {
tp := getTraceparent(ctx) // get the text representation in the context
if len(tp) > 0 && checkTracecarrierRe.MatchString(tp) {
parts := strings.Split(tp, "-") // e.g. 00-9765b2f71c68b04dc0ad2a4d73027d6f-1881444346b6296e-01
Expand All @@ -86,7 +86,7 @@ func loadTraceparentFromFile(ctx context.Context, filename string) context.Conte
if err != nil {
// only fatal when the tp carrier file is required explicitly, otherwise
// just silently return the unmodified context
if traceparentRequired {
if config.TraceparentRequired {
log.Fatalf("could not open file '%s' for read: %s", filename, err)
} else {
return ctx
Expand Down Expand Up @@ -135,7 +135,7 @@ func saveTraceparentToFile(ctx context.Context, filename string) {
// TRACEPARENT and sets it in the returned Go context.
func loadTraceparentFromEnv(ctx context.Context) context.Context {
// don't load the envvar when --tp-ignore-env is set
if traceparentIgnoreEnv {
if config.TraceparentIgnoreEnv {
return ctx
}

Expand Down
32 changes: 11 additions & 21 deletions cmd/plumbing.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (

"go.opentelemetry.io/otel"
otlpgrpc "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc"
stdout "go.opentelemetry.io/otel/exporters/stdout/stdouttrace"
"go.opentelemetry.io/otel/propagation"
"go.opentelemetry.io/otel/sdk/resource"
sdktrace "go.opentelemetry.io/otel/sdk/trace"
Expand All @@ -29,11 +28,11 @@ func initTracer() (context.Context, func()) {
// when no endpoint is set, do not set up plumbing. everything will still
// run but in non-recording mode, and otel-cli is effectively disabled
// and will not time out trying to connect out
if otlpEndpoint == "" {
if config.Endpoint == "" {
return ctx, func() {}
}

grpcOpts := []otlpgrpc.Option{otlpgrpc.WithEndpoint(otlpEndpoint)}
grpcOpts := []otlpgrpc.Option{otlpgrpc.WithEndpoint(config.Endpoint)}

// set timeout if the duration is non-zero, otherwise just leave things to the defaults
if timeout := parseCliTimeout(); timeout > 0 {
Expand All @@ -45,29 +44,29 @@ func initTracer() (context.Context, func()) {
// have any encryption available, or setting it up raises the bar of entry too high.
// The compromise is to automatically flip this flag to true when endpoint contains an
// an obvious "localhost", "127.0.0.x", or "::1" address.
if otlpInsecure || isLoopbackAddr(otlpEndpoint) {
if config.Insecure || isLoopbackAddr(config.Endpoint) {
grpcOpts = append(grpcOpts, otlpgrpc.WithInsecure())
} else {
var config *tls.Config
if noTlsVerify {
config = &tls.Config{
var tlsConfig *tls.Config
if config.NoTlsVerify {
tlsConfig = &tls.Config{
InsecureSkipVerify: true,
}
}
grpcOpts = append(grpcOpts, otlpgrpc.WithDialOption(grpc.WithTransportCredentials(credentials.NewTLS(config))))
grpcOpts = append(grpcOpts, otlpgrpc.WithDialOption(grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig))))
}

// support for OTLP headers, e.g. for authenticating to SaaS OTLP endpoints
if len(otlpHeaders) > 0 {
if len(config.Headers) > 0 {
// fortunately WithHeaders can accept the string map as-is
grpcOpts = append(grpcOpts, otlpgrpc.WithHeaders(otlpHeaders))
grpcOpts = append(grpcOpts, otlpgrpc.WithHeaders(config.Headers))
}

// OTLP examples usually show this with the grpc.WithBlock() dial option to
// make the connection synchronous, but it's not the right default for cli
// instead, rely on the shutdown methods to make sure everything is flushed
// by the time the program exits.
if otlpBlocking {
if config.Blocking {
grpcOpts = append(grpcOpts, otlpgrpc.WithDialOption(grpc.WithBlock()))
}

Expand All @@ -77,17 +76,8 @@ func initTracer() (context.Context, func()) {
log.Fatalf("failed to configure OTLP exporter: %s", err)
}

// when in test mode, let the otlp exporter setup happen, then overwrite it
// with the stdout exporter so spans only go to stdout
if testMode {
exporter, err = stdout.New()
if err != nil {
log.Fatalf("failed to configure stdout exporter in --test mode: %s", err)
}
}

// set the service name that will show up in tracing UIs
resAttrs := resource.WithAttributes(semconv.ServiceNameKey.String(serviceName))
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)
Expand Down
90 changes: 59 additions & 31 deletions cmd/root.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,44 @@ import (
"github.com/spf13/viper"
)

// TODO: that's a lot of globals, maybe move this into a struct
var cfgFile, serviceName, spanName, spanKind, traceparentCarrierFile string
var spanAttrs, otlpHeaders map[string]string
var otlpEndpoint, cliTimeout string
var otlpInsecure, otlpBlocking, noTlsVerify bool
var traceparentIgnoreEnv, traceparentPrint, traceparentPrintExport bool
var traceparentRequired, testMode bool
// Config stores the runtime configuration for otel-cli.
// This is used as a singleton as "config" and accessed from many other files.
// Data structure is public so that it can serialize to json easily.
type Config struct {
Endpoint string `json:"endpoint"`
Timeout string `json:"timeout"`
Headers map[string]string `json:"headers"` // TODO: needs json marshaler hook to mask tokens
Insecure bool `json:"insecure"`
Blocking bool `json:"blocking"`
NoTlsVerify bool `json:"no_tls_verify"`

ServiceName string `json:"service_name"`
SpanName string `json:"span_name"`
Kind string `json:"span_kind"`
Attributes map[string]string `json:"span_attributes"`

TraceparentCarrierFile string `json:"traceparent_carrier_file"`
TraceparentIgnoreEnv bool `json:"traceparent_ignore_env"`
TraceparentPrint bool `json:"traceparent_print"`
TraceparentPrintExport bool `json:"traceparent_print_export"`
TraceparentRequired bool `json:"traceparent_required"`

BackgroundParentPollMs int `json:"background_parent_poll_ms"`
BackgroundSockdir string `json:"background_socket_directory"`

SpanStartTime string `json:"span_start_time"`
SpanEndTime string `json:"span_end_time"`
EventName string `json:"event_name"`
EventTime string `json:"event_time"`

CfgFile string `json:"config_file"`
}

const defaultOtlpEndpoint = "localhost:4317"
const spanBgSockfilename = "otel-cli-background.sock"

var exitCode int
var defaultOtlpEndpoint = "localhost:4317"
var config Config

// rootCmd represents the base command when called without any subcommands
var rootCmd = &cobra.Command{
Expand All @@ -30,6 +59,7 @@ func Execute() {
}

func init() {

cobra.OnInitialize(initViperConfig)
cobra.EnableCommandSorting = false
rootCmd.Flags().SortFlags = false
Expand All @@ -38,13 +68,13 @@ func init() {
// addCommonParams adds the --config and --endpoint params to the command.
func addCommonParams(cmd *cobra.Command) {
// --config / -c a viper configuration file
cmd.Flags().StringVarP(&cfgFile, "config", "c", "", "config file (default is $HOME/.otel-cli.yaml)")
cmd.Flags().StringVarP(&config.CfgFile, "config", "c", "", "config file (default is $HOME/.otel-cli.yaml)")

cmd.Flags().StringVar(&otlpEndpoint, "endpoint", "", "dial address for the desired OTLP/gRPC endpoint")
cmd.Flags().StringVar(&config.Endpoint, "endpoint", "", "dial address for the desired OTLP/gRPC endpoint")
viper.BindPFlag("endpoint", rootCmd.Flags().Lookup("endpoint"))
viper.BindEnv("OTEL_EXPORTER_OTLP_ENDPOINT", "endpoint")

cmd.Flags().StringVar(&cliTimeout, "timeout", "1s", "timeout for otel-cli operations, all timeouts in otel-cli use this value")
cmd.Flags().StringVar(&config.Timeout, "timeout", "1s", "timeout for otel-cli operations, all timeouts in otel-cli use this value")
viper.BindPFlag("timeout", rootCmd.Flags().Lookup("timeout"))
viper.BindEnv("OTEL_EXPORTER_OTLP_TIMEOUT", "timeout")
}
Expand All @@ -54,72 +84,70 @@ func addCommonParams(cmd *cobra.Command) {
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/sdk-environment-variables.md
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md
func addClientParams(cmd *cobra.Command) {
otlpHeaders = make(map[string]string)
config.Headers = make(map[string]string)

cmd.Flags().BoolVar(&otlpInsecure, "insecure", false, "refuse to connect if TLS is unavailable (true by default when endpoint is localhost)")
cmd.Flags().BoolVar(&config.Insecure, "insecure", false, "refuse to connect if TLS is unavailable (true by default when endpoint is localhost)")
viper.BindPFlag("insecure", cmd.Flags().Lookup("insecure"))
viper.BindEnv("OTEL_EXPORTER_OTLP_INSECURE", "insecure")

cmd.Flags().StringToStringVar(&otlpHeaders, "otlp-headers", map[string]string{}, "a comma-sparated list of key=value headers to send on OTLP connection")
cmd.Flags().StringToStringVar(&config.Headers, "otlp-headers", map[string]string{}, "a comma-sparated list of key=value headers to send on OTLP connection")
viper.BindPFlag("otlp-headers", cmd.Flags().Lookup("otlp-headers"))
viper.BindEnv("OTEL_EXPORTER_OTLP_HEADERS", "otlp-headers")

cmd.Flags().BoolVar(&otlpBlocking, "otlp-blocking", false, "block on connecting to the OTLP server before proceeding")
cmd.Flags().BoolVar(&config.Blocking, "otlp-blocking", false, "block on connecting to the OTLP server before proceeding")
viper.BindPFlag("otlp-blocking", cmd.Flags().Lookup("otlp-blocking"))
viper.BindEnv("OTEL_EXPORTER_OTLP_BLOCKING", "otlp-blocking")

// trace propagation options
cmd.Flags().BoolVar(&traceparentRequired, "tp-required", false, "when set to true, fail and log if a traceparent can't be picked up from TRACEPARENT ennvar or a carrier file")
cmd.Flags().BoolVar(&config.TraceparentRequired, "tp-required", false, "when set to true, fail and log if a traceparent can't be picked up from TRACEPARENT ennvar or a carrier file")
viper.BindPFlag("tp-required", cmd.Flags().Lookup("tp-required"))
viper.BindEnv("OTEL_CLI_TRACEPARENT_REQUIRED", "tp-required")

cmd.Flags().StringVar(&traceparentCarrierFile, "tp-carrier", "", "a file for reading and WRITING traceparent across invocations")
cmd.Flags().StringVar(&config.TraceparentCarrierFile, "tp-carrier", "", "a file for reading and WRITING traceparent across invocations")
viper.BindPFlag("tp-carrier", cmd.Flags().Lookup("tp-carrier"))
viper.BindEnv("OTEL_CLI_CARRIER_FILE", "tp-carrier")

cmd.Flags().BoolVar(&traceparentIgnoreEnv, "tp-ignore-env", false, "ignore the TRACEPARENT envvar even if it's set")
cmd.Flags().BoolVar(&config.TraceparentIgnoreEnv, "tp-ignore-env", false, "ignore the TRACEPARENT envvar even if it's set")
viper.BindPFlag("tp-ignore-env", cmd.Flags().Lookup("tp-ignore-env"))
viper.BindEnv("OTEL_CLI_IGNORE_ENV", "tp-ignore-env")

cmd.Flags().BoolVar(&traceparentPrint, "tp-print", false, "print the trace id, span id, and the w3c-formatted traceparent representation of the new span")
cmd.Flags().BoolVar(&config.TraceparentPrint, "tp-print", false, "print the trace id, span id, and the w3c-formatted traceparent representation of the new span")
viper.BindPFlag("tp-print", cmd.Flags().Lookup("tp-print"))
viper.BindEnv("OTEL_CLI_PRINT_TRACEPARENT", "tp-print")

cmd.Flags().BoolVarP(&traceparentPrintExport, "tp-export", "p", false, "same as --tp-print but it puts an 'export ' in front so it's more convinenient to source in scripts")
cmd.Flags().BoolVarP(&config.TraceparentPrintExport, "tp-export", "p", false, "same as --tp-print but it puts an 'export ' in front so it's more convinenient to source in scripts")
viper.BindPFlag("tp-export", cmd.Flags().Lookup("tp-export"))
viper.BindEnv("OTEL_CLI_EXPORT_TRACEPARENT", "tp-export")

cmd.Flags().BoolVar(&noTlsVerify, "no-tls-verify", false, "enable it when TLS is enabled and you want to ignore the certificate validation. This is common when you are testing and usign self-signed certificates.")
cmd.Flags().BoolVar(&config.NoTlsVerify, "no-tls-verify", false, "enable it when TLS is enabled and you want to ignore the certificate validation. This is common when you are testing and usign self-signed certificates.")
viper.BindPFlag("no-tls-verify", cmd.Flags().Lookup("no-tls-verify"))
viper.BindEnv("OTEL_CLI_NO_TLS_VERIFY", "no-tls-verify")

cmd.Flags().BoolVar(&testMode, "test", false, "configure noop exporter and dump data to json on stdout instead of sending")
}

func addSpanParams(cmd *cobra.Command) {
// --name / -s
cmd.Flags().StringVarP(&spanName, "name", "s", "todo-generate-default-span-names", "set the name of the span")
cmd.Flags().StringVarP(&config.SpanName, "name", "s", "todo-generate-default-span-names", "set the name of the span")

cmd.Flags().StringVarP(&serviceName, "service", "n", "otel-cli", "set the name of the application sent on the traces")
cmd.Flags().StringVarP(&config.ServiceName, "service", "n", "otel-cli", "set the name of the application sent on the traces")
viper.BindPFlag("service", cmd.Flags().Lookup("service"))
viper.BindEnv("OTEL_CLI_SERVICE_NAME", "service")

cmd.Flags().StringVarP(&spanKind, "kind", "k", "client", "set the trace kind, e.g. internal, server, client, producer, consumer")
cmd.Flags().StringVarP(&config.Kind, "kind", "k", "client", "set the trace kind, e.g. internal, server, client, producer, consumer")
viper.BindPFlag("kind", cmd.Flags().Lookup("kind"))
viper.BindEnv("OTEL_CLI_TRACE_KIND", "kind")
}

func addAttrParams(cmd *cobra.Command) {
// --attrs key=value,foo=bar
spanAttrs = make(map[string]string)
cmd.Flags().StringToStringVarP(&spanAttrs, "attrs", "a", map[string]string{}, "a comma-separated list of key=value attributes")
config.Attributes = make(map[string]string)
cmd.Flags().StringToStringVarP(&config.Attributes, "attrs", "a", map[string]string{}, "a comma-separated list of key=value attributes")
viper.BindPFlag("attrs", cmd.Flags().Lookup("attrs"))
viper.BindEnv("OTEL_CLI_ATTRIBUTES", "attrs")
}

func initViperConfig() {
if cfgFile != "" {
viper.SetConfigFile(cfgFile)
if config.CfgFile != "" {
viper.SetConfigFile(config.CfgFile)
} else {
home, err := homedir.Dir()
cobra.CheckErr(err)
Expand All @@ -132,7 +160,7 @@ func initViperConfig() {
// We want to suppress errors here if the config is not found, but only if the user has not expressly given us a location to search.
// Otherwise, we'll raise any config-reading error up to the user.
_, cfgNotFound := err.(viper.ConfigFileNotFoundError)
if cfgFile != "" || !cfgNotFound {
if config.CfgFile != "" || !cfgNotFound {
cobra.CheckErr(err)
}
}
Expand Down
Loading

0 comments on commit 44d2088

Please sign in to comment.