From 6502d6816255ba07fe7d85543ef5da999ac93dcd Mon Sep 17 00:00:00 2001 From: Joe Kralicky Date: Tue, 14 Jan 2025 13:50:22 -0500 Subject: [PATCH] config: set default tracing sample rate to 1.0 (#5422) The previous default sample rate of 0.0001 is very low, so traces are unlikely to be visible after enabling them until many thousands of requests have been sent. This could be confusing to users. --- config/envoyconfig/listeners_main.go | 6 +++++- .../main_http_connection_manager_filter.json | 6 ++++-- config/options.go | 9 ++++----- config/trace.go | 6 +++++- config/trace_test.go | 16 +++++++--------- 5 files changed, 25 insertions(+), 18 deletions(-) diff --git a/config/envoyconfig/listeners_main.go b/config/envoyconfig/listeners_main.go index b72f760f3..e21915011 100644 --- a/config/envoyconfig/listeners_main.go +++ b/config/envoyconfig/listeners_main.go @@ -187,6 +187,10 @@ func (b *Builder) buildMainHTTPConnectionManagerFilter( return nil, err } + sampleRate := 1.0 + if cfg.Options.TracingSampleRate != nil { + sampleRate = *cfg.Options.TracingSampleRate + } mgr := &envoy_extensions_filters_network_http_connection_manager.HttpConnectionManager{ AlwaysSetRequestIdInResponse: true, StatPrefix: "ingress", @@ -199,7 +203,7 @@ func (b *Builder) buildMainHTTPConnectionManagerFilter( HttpProtocolOptions: http1ProtocolOptions, RequestTimeout: durationpb.New(cfg.Options.ReadTimeout), Tracing: &envoy_extensions_filters_network_http_connection_manager.HttpConnectionManager_Tracing{ - RandomSampling: &envoy_type_v3.Percent{Value: cfg.Options.TracingSampleRate * 100}, + RandomSampling: &envoy_type_v3.Percent{Value: max(0.0, min(1.0, sampleRate)) * 100}, Provider: tracingProvider, }, // See https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_conn_man/headers#x-forwarded-for diff --git a/config/envoyconfig/testdata/main_http_connection_manager_filter.json b/config/envoyconfig/testdata/main_http_connection_manager_filter.json index b67fa0d06..ca5d62958 100644 --- a/config/envoyconfig/testdata/main_http_connection_manager_filter.json +++ b/config/envoyconfig/testdata/main_http_connection_manager_filter.json @@ -52,7 +52,9 @@ }, "timeout": "10s" }, - "metadataContextNamespaces": ["com.pomerium.client-certificate-info"], + "metadataContextNamespaces": [ + "com.pomerium.client-certificate-info" + ], "statusOnError": { "code": "InternalServerError" }, @@ -180,7 +182,7 @@ "statPrefix": "ingress", "tracing": { "randomSampling": { - "value": 0.01 + "value": 100 } }, "useRemoteAddress": true, diff --git a/config/options.go b/config/options.go index 6dfa088e5..612dcdbf6 100644 --- a/config/options.go +++ b/config/options.go @@ -211,8 +211,8 @@ type Options struct { MetricsClientCAFile string `mapstructure:"metrics_client_ca_file" yaml:"metrics_client_ca_file,omitempty"` // Tracing shared settings - TracingProvider string `mapstructure:"tracing_provider" yaml:"tracing_provider,omitempty"` - TracingSampleRate float64 `mapstructure:"tracing_sample_rate" yaml:"tracing_sample_rate,omitempty"` + TracingProvider string `mapstructure:"tracing_provider" yaml:"tracing_provider,omitempty"` + TracingSampleRate *float64 `mapstructure:"tracing_sample_rate" yaml:"tracing_sample_rate,omitempty"` // Datadog tracing address TracingDatadogAddress string `mapstructure:"tracing_datadog_address" yaml:"tracing_datadog_address,omitempty"` @@ -317,7 +317,6 @@ var defaultOptions = Options{ GRPCAddr: ":443", GRPCClientTimeout: 10 * time.Second, // Try to withstand transient service failures for a single request AuthenticateCallbackPath: "/oauth2/callback", - TracingSampleRate: 0.0001, AutocertOptions: AutocertOptions{ Folder: dataDir(), @@ -1520,7 +1519,7 @@ func (o *Options) ApplySettings(ctx context.Context, certsIndex *cryptutil.Certi setCertificate(&o.MetricsCertificate, &o.MetricsCertificateKey, settings.MetricsCertificate) set(&o.MetricsClientCA, settings.MetricsClientCa) set(&o.TracingProvider, settings.TracingProvider) - set(&o.TracingSampleRate, settings.TracingSampleRate) + setOptional(&o.TracingSampleRate, settings.TracingSampleRate) set(&o.TracingDatadogAddress, settings.TracingDatadogAddress) set(&o.TracingJaegerCollectorEndpoint, settings.TracingJaegerCollectorEndpoint) set(&o.TracingJaegerAgentEndpoint, settings.TracingJaegerAgentEndpoint) @@ -1610,7 +1609,7 @@ func (o *Options) ToProto() *config.Config { settings.MetricsCertificate = toCertificateOrFromFile(o.MetricsCertificate, o.MetricsCertificateKey, o.MetricsCertificateFile, o.MetricsCertificateKeyFile) copySrcToOptionalDest(&settings.MetricsClientCa, valueOrFromFileBase64(o.MetricsClientCA, o.MetricsClientCAFile)) copySrcToOptionalDest(&settings.TracingProvider, &o.TracingProvider) - copySrcToOptionalDest(&settings.TracingSampleRate, &o.TracingSampleRate) + settings.TracingSampleRate = o.TracingSampleRate copySrcToOptionalDest(&settings.TracingDatadogAddress, &o.TracingDatadogAddress) copySrcToOptionalDest(&settings.TracingJaegerCollectorEndpoint, &o.TracingJaegerCollectorEndpoint) copySrcToOptionalDest(&settings.TracingJaegerAgentEndpoint, &o.TracingJaegerAgentEndpoint) diff --git a/config/trace.go b/config/trace.go index 1c8830494..a0cd82b71 100644 --- a/config/trace.go +++ b/config/trace.go @@ -19,11 +19,15 @@ type TracingOptions = trace.TracingOptions // NewTracingOptions builds a new TracingOptions from core Options func NewTracingOptions(o *Options) (*TracingOptions, error) { + sampleRate := 1.0 + if o.TracingSampleRate != nil { + sampleRate = *o.TracingSampleRate + } tracingOpts := TracingOptions{ Provider: o.TracingProvider, Service: telemetry.ServiceName(o.Services), JaegerAgentEndpoint: o.TracingJaegerAgentEndpoint, - SampleRate: o.TracingSampleRate, + SampleRate: sampleRate, } switch o.TracingProvider { diff --git a/config/trace_test.go b/config/trace_test.go index af87ac955..2ae2a7896 100644 --- a/config/trace_test.go +++ b/config/trace_test.go @@ -25,13 +25,13 @@ func Test_NewTracingOptions(t *testing.T) { { "datadog_good", &Options{TracingProvider: "datadog"}, - &TracingOptions{Provider: "datadog", Service: "pomerium"}, + &TracingOptions{Provider: "datadog", Service: "pomerium", SampleRate: 1}, false, }, { "jaeger_good", &Options{TracingProvider: "jaeger", TracingJaegerAgentEndpoint: "foo", TracingJaegerCollectorEndpoint: "http://foo", Services: ServiceAll}, - &TracingOptions{Provider: "jaeger", JaegerAgentEndpoint: "foo", JaegerCollectorEndpoint: &url.URL{Scheme: "http", Host: "foo"}, Service: "pomerium"}, + &TracingOptions{Provider: "jaeger", JaegerAgentEndpoint: "foo", JaegerCollectorEndpoint: &url.URL{Scheme: "http", Host: "foo"}, Service: "pomerium", SampleRate: 1}, false, }, { @@ -43,7 +43,7 @@ func Test_NewTracingOptions(t *testing.T) { { "zipkin_good", &Options{TracingProvider: "zipkin", ZipkinEndpoint: "https://foo/api/v1/spans", Services: ServiceAuthorize}, - &TracingOptions{Provider: "zipkin", ZipkinEndpoint: &url.URL{Scheme: "https", Host: "foo", Path: "/api/v1/spans"}, Service: "pomerium-authorize"}, + &TracingOptions{Provider: "zipkin", ZipkinEndpoint: &url.URL{Scheme: "https", Host: "foo", Path: "/api/v1/spans"}, Service: "pomerium-authorize", SampleRate: 1}, false, }, { @@ -118,9 +118,8 @@ func TestTraceManager(t *testing.T) { defer srv2.Close() src := NewStaticSource(&Config{Options: &Options{ - TracingProvider: "zipkin", - ZipkinEndpoint: srv1.URL, - TracingSampleRate: 1, + TracingProvider: "zipkin", + ZipkinEndpoint: srv1.URL, }}) _ = NewTraceManager(ctx, src) @@ -129,9 +128,8 @@ func TestTraceManager(t *testing.T) { span.End() src.SetConfig(ctx, &Config{Options: &Options{ - TracingProvider: "zipkin", - ZipkinEndpoint: srv2.URL, - TracingSampleRate: 1, + TracingProvider: "zipkin", + ZipkinEndpoint: srv2.URL, }}) _, span = trace.StartSpan(ctx, "Example")