Followup on tracing (#10338)

* Simplify code by letting common deal with empty TLS config
* Improve error message if we notice a user is putting an authorization
header into its configuration.

Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
This commit is contained in:
Julien Pivotto 2022-02-22 21:44:36 +01:00 committed by GitHub
parent 0acbe5e3f5
commit fb2da1f26a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 30 additions and 17 deletions

View file

@ -558,7 +558,7 @@ func (t *TracingConfig) UnmarshalYAML(unmarshal func(interface{}) error) error {
return err return err
} }
if err := validateHeaders(t.Headers); err != nil { if err := validateHeadersForTracing(t.Headers); err != nil {
return err return err
} }
@ -805,6 +805,18 @@ func (c *RemoteWriteConfig) UnmarshalYAML(unmarshal func(interface{}) error) err
return nil return nil
} }
func validateHeadersForTracing(headers map[string]string) error {
for header := range headers {
if strings.ToLower(header) == "authorization" {
return errors.New("custom authorization header configuration is not yet supported")
}
if _, ok := reservedHeaders[strings.ToLower(header)]; ok {
return errors.Errorf("%s is a reserved header. It must not be changed", header)
}
}
return nil
}
func validateHeaders(headers map[string]string) error { func validateHeaders(headers map[string]string) error {
for header := range headers { for header := range headers {
if strings.ToLower(header) == "authorization" { if strings.ToLower(header) == "authorization" {

View file

@ -1464,6 +1464,10 @@ var expectedErrors = []struct {
filename: "tracing_invalid_header.bad.yml", filename: "tracing_invalid_header.bad.yml",
errMsg: "x-prometheus-remote-write-version is a reserved header. It must not be changed", errMsg: "x-prometheus-remote-write-version is a reserved header. It must not be changed",
}, },
{
filename: "tracing_invalid_authorization_header.bad.yml",
errMsg: "authorization header configuration is not yet supported",
},
{ {
filename: "tracing_invalid_compression.bad.yml", filename: "tracing_invalid_compression.bad.yml",
errMsg: "invalid compression type foo provided, valid options: gzip", errMsg: "invalid compression type foo provided, valid options: gzip",

View file

@ -0,0 +1,5 @@
tracing:
sampling_fraction: 1
endpoint: "localhost:4317"
headers:
"authorization": foo

View file

@ -194,15 +194,11 @@ func getClient(tracingCfg config.TracingConfig) (otlptrace.Client, error) {
opts = append(opts, otlptracegrpc.WithTimeout(time.Duration(tracingCfg.Timeout))) opts = append(opts, otlptracegrpc.WithTimeout(time.Duration(tracingCfg.Timeout)))
} }
// Configure TLS only if config is not empty. tlsConf, err := config_util.NewTLSConfig(&tracingCfg.TLSConfig)
var blankTLSConfig config_util.TLSConfig if err != nil {
if tracingCfg.TLSConfig != blankTLSConfig { return nil, err
tlsConf, err := config_util.NewTLSConfig(&tracingCfg.TLSConfig)
if err != nil {
return nil, err
}
opts = append(opts, otlptracegrpc.WithTLSCredentials(credentials.NewTLS(tlsConf)))
} }
opts = append(opts, otlptracegrpc.WithTLSCredentials(credentials.NewTLS(tlsConf)))
client = otlptracegrpc.NewClient(opts...) client = otlptracegrpc.NewClient(opts...)
case config.TracingClientHTTP: case config.TracingClientHTTP:
@ -221,15 +217,11 @@ func getClient(tracingCfg config.TracingConfig) (otlptrace.Client, error) {
opts = append(opts, otlptracehttp.WithTimeout(time.Duration(tracingCfg.Timeout))) opts = append(opts, otlptracehttp.WithTimeout(time.Duration(tracingCfg.Timeout)))
} }
// Configure TLS only if config is not empty. tlsConf, err := config_util.NewTLSConfig(&tracingCfg.TLSConfig)
var blankTLSConfig config_util.TLSConfig if err != nil {
if tracingCfg.TLSConfig != blankTLSConfig { return nil, err
tlsConf, err := config_util.NewTLSConfig(&tracingCfg.TLSConfig)
if err != nil {
return nil, err
}
opts = append(opts, otlptracehttp.WithTLSClientConfig(tlsConf))
} }
opts = append(opts, otlptracehttp.WithTLSClientConfig(tlsConf))
client = otlptracehttp.NewClient(opts...) client = otlptracehttp.NewClient(opts...)
} }