diff --git a/go/cmd/vtadmin/main.go b/go/cmd/vtadmin/main.go index abc7d5b08c7..785ddbe1730 100644 --- a/go/cmd/vtadmin/main.go +++ b/go/cmd/vtadmin/main.go @@ -81,6 +81,9 @@ func main() { rootCmd.Flags().Var(&clusterFileConfig, "cluster-config", "path to a yaml cluster configuration. see clusters.example.yaml") // (TODO:@amason) provide example config. rootCmd.Flags().Var(&defaultClusterConfig, "cluster-defaults", "default options for all clusters") + rootCmd.Flags().AddGoFlag(flag.Lookup("tracer")) // defined in go/vt/trace + rootCmd.Flags().AddGoFlag(flag.Lookup("tracing-sampling-type")) // defined in go/vt/trace + rootCmd.Flags().AddGoFlag(flag.Lookup("tracing-sampling-rate")) // defined in go/vt/trace rootCmd.Flags().BoolVar(&opts.EnableTracing, "grpc-tracing", false, "whether to enable tracing on the gRPC server") rootCmd.Flags().BoolVar(&httpOpts.EnableTracing, "http-tracing", false, "whether to enable tracing on the HTTP server") rootCmd.Flags().BoolVar(&httpOpts.DisableCompression, "http-no-compress", false, "whether to disable compression of HTTP API responses") diff --git a/go/flagutil/optional.go b/go/flagutil/optional.go new file mode 100644 index 00000000000..3bfcd3dd473 --- /dev/null +++ b/go/flagutil/optional.go @@ -0,0 +1,146 @@ +/* +Copyright 2021 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package flagutil + +import ( + "errors" + "flag" + "strconv" +) + +// OptionalFlag augements the flag.Value interface with a method to determine +// if a flag was set explicitly on the comand-line. +// +// Though not part of the interface, because the return type would be different +// for each implementation, by convention, each implementation should define a +// Get() method to access the underlying value. +type OptionalFlag interface { + flag.Value + IsSet() bool +} + +var ( + _ OptionalFlag = (*OptionalFloat64)(nil) + _ OptionalFlag = (*OptionalString)(nil) +) + +// OptionalFloat64 implements OptionalFlag for float64 values. +type OptionalFloat64 struct { + val float64 + set bool +} + +// NewOptionalFloat64 returns an OptionalFloat64 with the specified value as its +// starting value. +func NewOptionalFloat64(val float64) *OptionalFloat64 { + return &OptionalFloat64{ + val: val, + set: false, + } +} + +// Set is part of the flag.Value interface. +func (f *OptionalFloat64) Set(arg string) error { + v, err := strconv.ParseFloat(arg, 64) + if err != nil { + return numError(err) + } + + f.val = v + f.set = true + + return nil +} + +// String is part of the flag.Value interface. +func (f *OptionalFloat64) String() string { + return strconv.FormatFloat(f.val, 'g', -1, 64) +} + +// Get returns the underlying float64 value of this flag. If the flag was not +// explicitly set, this will be the initial value passed to the constructor. +func (f *OptionalFloat64) Get() float64 { + return f.val +} + +// IsSet is part of the OptionalFlag interface. +func (f *OptionalFloat64) IsSet() bool { + return f.set +} + +// OptionalString implements OptionalFlag for string values. +type OptionalString struct { + val string + set bool +} + +// NewOptionalString returns an OptionalString with the specified value as its +// starting value. +func NewOptionalString(val string) *OptionalString { + return &OptionalString{ + val: val, + set: false, + } +} + +// Set is part of the flag.Value interface. +func (f *OptionalString) Set(arg string) error { + f.val = arg + f.set = true + return nil +} + +// String is part of the flag.Value interface. +func (f *OptionalString) String() string { + return f.val +} + +// Get returns the underlying string value of this flag. If the flag was not +// explicitly set, this will be the initial value passed to the constructor. +func (f *OptionalString) Get() string { + return f.val +} + +// IsSet is part of the OptionalFlag interface. +func (f *OptionalString) IsSet() bool { + return f.set +} + +// lifted directly from package flag to make the behavior of numeric parsing +// consistent with the standard library for our custom optional types. +var ( + errParse = errors.New("parse error") + errRange = errors.New("value out of range") +) + +// lifted directly from package flag to make the behavior of numeric parsing +// consistent with the standard library for our custom optional types. +func numError(err error) error { + ne, ok := err.(*strconv.NumError) + if !ok { + return err + } + + switch ne.Err { + case strconv.ErrSyntax: + return errParse + case strconv.ErrRange: + return errRange + default: + return err + } +} diff --git a/go/trace/plugin_datadog.go b/go/trace/plugin_datadog.go index 87809d9bc50..45b743c408e 100644 --- a/go/trace/plugin_datadog.go +++ b/go/trace/plugin_datadog.go @@ -24,7 +24,7 @@ func newDatadogTracer(serviceName string) (tracingService, io.Closer, error) { ddtracer.WithAgentAddr(*dataDogHost+":"+*dataDogPort), ddtracer.WithServiceName(serviceName), ddtracer.WithDebugMode(true), - ddtracer.WithSampler(ddtracer.NewRateSampler(*samplingRate)), + ddtracer.WithSampler(ddtracer.NewRateSampler(samplingRate.Get())), ) opentracing.SetGlobalTracer(t) diff --git a/go/trace/plugin_jaeger.go b/go/trace/plugin_jaeger.go index 587f3ac5233..ba64e277bdf 100644 --- a/go/trace/plugin_jaeger.go +++ b/go/trace/plugin_jaeger.go @@ -19,11 +19,13 @@ package trace import ( "flag" "io" + "os" "github.com/opentracing/opentracing-go" "github.com/uber/jaeger-client-go" "github.com/uber/jaeger-client-go/config" + "vitess.io/vitess/go/flagutil" "vitess.io/vitess/go/vt/log" ) @@ -35,9 +37,15 @@ included but nothing Jaeger specific. var ( agentHost = flag.String("jaeger-agent-host", "", "host and port to send spans to. if empty, no tracing will be done") - samplingRate = flag.Float64("tracing-sampling-rate", 0.1, "sampling rate for the probabilistic jaeger sampler") + samplingType = flagutil.NewOptionalString("const") + samplingRate = flagutil.NewOptionalFloat64(0.1) ) +func init() { + flag.Var(samplingType, "tracing-sampling-type", "sampling strategy to use for jaeger. possible values are 'const', 'probabilistic', 'rateLimiting', or 'remote'") + flag.Var(samplingRate, "tracing-sampling-rate", "sampling rate for the probabilistic jaeger sampler") +} + // newJagerTracerFromEnv will instantiate a tracingService implemented by Jaeger, // taking configuration from environment variables. Available properties are: // JAEGER_SERVICE_NAME -- If this is set, the service name used in code will be ignored and this value used instead @@ -70,11 +78,25 @@ func newJagerTracerFromEnv(serviceName string) (tracingService, io.Closer, error cfg.Reporter.LocalAgentHostPort = *agentHost } log.Infof("Tracing to: %v as %v", cfg.Reporter.LocalAgentHostPort, cfg.ServiceName) - cfg.Sampler = &config.SamplerConfig{ - Type: jaeger.SamplerTypeConst, - Param: *samplingRate, + + if os.Getenv("JAEGER_SAMPLER_PARAM") == "" { + // If the environment variable was not set, we take the flag regardless + // of whether it was explicitly set on the command line. + cfg.Sampler.Param = samplingRate.Get() + } else if samplingRate.IsSet() { + // If the environment variable was set, but the user also explicitly + // passed the command line flag, the flag takes precedence. + cfg.Sampler.Param = samplingRate.Get() } - log.Infof("Tracing sampling rate: %v", *samplingRate) + + if samplingType.IsSet() { + cfg.Sampler.Type = samplingType.Get() + } else if cfg.Sampler.Type == "" { + log.Infof("-tracing-sampler-type was not set, and JAEGER_SAMPLER_TYPE was not set, defaulting to const sampler") + cfg.Sampler.Type = jaeger.SamplerTypeConst + } + + log.Infof("Tracing sampler type %v (param: %v)", cfg.Sampler.Type, cfg.Sampler.Param) tracer, closer, err := cfg.NewTracer()