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

make OTel endpoint optional and use defaults if note set #4

Merged
merged 5 commits into from Feb 1, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
76 changes: 58 additions & 18 deletions cobrautil.go
Expand Up @@ -18,6 +18,8 @@ import (
"github.com/spf13/cobra"
"github.com/spf13/pflag"
"github.com/spf13/viper"
"go.opentelemetry.io/contrib/propagators/b3"
"go.opentelemetry.io/contrib/propagators/ot"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/exporters/jaeger"
"go.opentelemetry.io/otel/exporters/otlp/otlptrace"
Expand Down Expand Up @@ -155,8 +157,9 @@ func RegisterOpenTelemetryFlags(flags *pflag.FlagSet, flagPrefix, serviceName st
prefixed := prefixJoiner(stringz.DefaultEmpty(flagPrefix, "otel"))

flags.String(prefixed("provider"), "none", `OpenTelemetry provider for tracing ("none", "jaeger, otlphttp", "otlpgrpc")`)
flags.String(prefixed("endpoint"), "http://collector:14268/api/traces", "collector endpoint")
flags.String(prefixed("endpoint"), "", "OpenTelemetry collector endpoint - the endpoint can also be set by using enviroment variables")
flags.String(prefixed("service-name"), serviceName, "service name for trace data")
flags.String(prefixed("trace-propagator"), "w3c", `OpenTelemetry trace propagation format ("b3", "w3c", "ottrace"). Add multiple propagators separated by comma.`)
}

// OpenTelemetryRunE returns a Cobra run func that configures the
Expand All @@ -173,28 +176,52 @@ func OpenTelemetryRunE(flagPrefix string, prerunLevel zerolog.Level) CobraRunFun

provider := strings.ToLower(MustGetString(cmd, prefixed("provider")))
serviceName := MustGetString(cmd, prefixed("service-name"))
endpoint, _ := cmd.Flags().GetString(prefixed("endpoint"))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be MustGetString(cmd, prefixed("endpoint"))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, updated it so the function uses the same Method to get the Flag value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I missed this comment. This should not be MustGetString, since my intention was to make specifying the endpoint via the command line optional. The reasoning is WithEndpoint() does not allow configuration of the URL path, so if that is needed, configuration via environment variable is required.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my Test, theMustGetString will just render an empty String as that is set as default.

propagators := strings.Split(MustGetString(cmd, prefixed("trace-propagator")), ",")

var exporter trace.SpanExporter
var err error

// If endpoint is not set, the clients are configured via the OpenTelemetry environment variables or
// default values.
jzelinskie marked this conversation as resolved.
Show resolved Hide resolved
// See: https://github.com/open-telemetry/opentelemetry-go/tree/main/exporters/otlp/otlptrace#environment-variables
// or https://github.com/open-telemetry/opentelemetry-go/tree/main/exporters/jaeger#environment-variables
switch provider {
case "none":
// Nothing.
case "jaeger":
exporter, err := jaeger.New(jaeger.WithCollectorEndpoint(jaeger.WithEndpoint(MustGetString(cmd, prefixed("endpoint")))))
var opts []jaeger.CollectorEndpointOption
if endpoint != "" {
opts = append(opts, jaeger.WithEndpoint(endpoint))
}

exporter, err = jaeger.New(jaeger.WithCollectorEndpoint(opts...))
if err != nil {
return err
}
return initOtelTracer(exporter, serviceName)
return initOtelTracer(exporter, serviceName, propagators)
case "otlphttp":
exporter, err := otlptrace.New(context.Background(), otlptracehttp.NewClient())
var opts []otlptracehttp.Option
if endpoint != "" {
opts = append(opts, otlptracehttp.WithEndpoint(endpoint))
}

exporter, err = otlptrace.New(context.Background(), otlptracehttp.NewClient(opts...))
if err != nil {
return err
}
return initOtelTracer(exporter, serviceName)
return initOtelTracer(exporter, serviceName, propagators)
case "otlpgrpc":
exporter, err := otlptrace.New(context.Background(), otlptracegrpc.NewClient())
var opts []otlptracegrpc.Option
if endpoint != "" {
opts = append(opts, otlptracegrpc.WithEndpoint(endpoint))
}

exporter, err = otlptrace.New(context.Background(), otlptracegrpc.NewClient(opts...))
if err != nil {
return err
}
return initOtelTracer(exporter, serviceName)
return initOtelTracer(exporter, serviceName, propagators)
default:
return fmt.Errorf("unknown tracing provider: %s", provider)
}
Expand All @@ -204,7 +231,7 @@ func OpenTelemetryRunE(flagPrefix string, prerunLevel zerolog.Level) CobraRunFun
}
}

func initOtelTracer(exporter trace.SpanExporter, serviceName string) error {
func initOtelTracer(exporter trace.SpanExporter, serviceName string, propagators []string) error {
res, _ := resource.Merge(
resource.Default(),
resource.NewSchemaless(semconv.ServiceNameKey.String(serviceName)),
Expand All @@ -217,20 +244,33 @@ func initOtelTracer(exporter trace.SpanExporter, serviceName string) error {
)

otel.SetTracerProvider(tp)

// Configure the global tracer to use the W3C method for propagating contexts
// across services.
//
// For low-level details see:
// https://www.w3.org/TR/trace-context/
otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator(
propagation.Baggage{}, // W3C baggage support
propagation.TraceContext{}, // W3C for compatibility with other tracing system
))
setTracePropagators(propagators)

return nil
}

// setTextMapPropagator sets the OpenTelemetry trace propagation format.
// Currently it supports b3, ot-trace and w3c.
func setTracePropagators(propagators []string) {
var tmPropagators []propagation.TextMapPropagator

for _, p := range propagators {
switch p {
case "b3":
tmPropagators = append(tmPropagators, b3.New())
case "ottrace":
tmPropagators = append(tmPropagators, ot.OT{})
case "w3c":
fallthrough
default:
tmPropagators = append(tmPropagators, propagation.Baggage{}) // W3C baggage support
tmPropagators = append(tmPropagators, propagation.TraceContext{}) // W3C for compatibility with other tracing system
}
}

otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator(tmPropagators...))
}

// RegisterGrpcServerFlags adds the following flags for use with
// GrpcServerFromFlags:
// - "$PREFIX-addr"
Expand Down
2 changes: 2 additions & 0 deletions go.mod
Expand Up @@ -9,6 +9,8 @@ require (
github.com/spf13/cobra v1.1.3
github.com/spf13/pflag v1.0.5
github.com/spf13/viper v1.7.1
go.opentelemetry.io/contrib/propagators/b3 v1.3.0
go.opentelemetry.io/contrib/propagators/ot v1.3.0
go.opentelemetry.io/otel v1.3.0
go.opentelemetry.io/otel/exporters/jaeger v1.3.0
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.3.0
Expand Down
4 changes: 4 additions & 0 deletions go.sum
Expand Up @@ -244,6 +244,10 @@ github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1
go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU=
go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU=
go.opencensus.io v0.22.0/go.mod h1:+kGneAE2xo2IficOXnaByMWTGM9T73dGwxeWcUqIpI8=
go.opentelemetry.io/contrib/propagators/b3 v1.3.0 h1:f+JfMSDNm2u+fekYYjyoixk+DWDTDAGD3SC50y61koE=
go.opentelemetry.io/contrib/propagators/b3 v1.3.0/go.mod h1:qzi0km8qO3l2jxB5aDg4Q9xyqV4HKnCWZYpVYDTUIT0=
go.opentelemetry.io/contrib/propagators/ot v1.3.0 h1:hqFpnicJXKy8l8PfwFWhRSt/TgOHCpugKiXsPP1zJUc=
go.opentelemetry.io/contrib/propagators/ot v1.3.0/go.mod h1:Gpwe4R8j9Zbw7aaADYSQRE1U0o41j0TwnHxuhwRLklk=
go.opentelemetry.io/otel v1.3.0 h1:APxLf0eiBwLl+SOXiJJCVYzA1OOJNyAoV8C5RNRyy7Y=
go.opentelemetry.io/otel v1.3.0/go.mod h1:PWIKzi6JCp7sM0k9yZ43VX+T345uNbAkDKwHVjb2PTs=
go.opentelemetry.io/otel/exporters/jaeger v1.3.0 h1:HfydzioALdtcB26H5WHc4K47iTETJCdloL7VN579/L0=
Expand Down