diff --git a/pkg/tracing/jaeger/config_yaml.go b/pkg/tracing/jaeger/config_yaml.go index 71009070b3d..fae3f8c21cc 100644 --- a/pkg/tracing/jaeger/config_yaml.go +++ b/pkg/tracing/jaeger/config_yaml.go @@ -19,6 +19,13 @@ import ( tracesdk "go.opentelemetry.io/otel/sdk/trace" ) +const ( + SamplerTypeRemote = "remote" + SamplerTypeProbabilistic = "probabilistic" + SamplerTypeConstant = "const" + SamplerTypeRateLimiting = "ratelimiting" +) + type ParentBasedSamplerConfig struct { LocalParentSampled bool `yaml:"local_parent_sampled"` RemoteParentSampled bool `yaml:"remote_parent_sampled"` @@ -114,22 +121,27 @@ func getSamplingFraction(samplerType string, samplingFactor float64) float64 { func getSampler(config Config) tracesdk.Sampler { samplerType := config.SamplerType + if samplerType == "" { + samplerType = SamplerTypeRateLimiting + } samplingFraction := getSamplingFraction(samplerType, config.SamplerParam) var sampler tracesdk.Sampler switch samplerType { - case "probabilistic": - sampler = tracesdk.ParentBased(tracesdk.TraceIDRatioBased(samplingFraction)) - case "const": + case SamplerTypeProbabilistic: + sampler = tracesdk.TraceIDRatioBased(samplingFraction) + case SamplerTypeConstant: if samplingFraction == 1.0 { sampler = tracesdk.AlwaysSample() } else { sampler = tracesdk.NeverSample() } - case "remote": + case SamplerTypeRemote: remoteOptions := getRemoteOptions(config) sampler = jaegerremote.New(config.ServiceName, remoteOptions...) - case "ratelimiting": + // Fallback always to default (rate limiting). + case SamplerTypeRateLimiting: + default: // The same config options are applicable to both remote and rate-limiting samplers. remoteOptions := getRemoteOptions(config) sampler = jaegerremote.New(config.ServiceName, remoteOptions...) @@ -137,17 +149,20 @@ func getSampler(config Config) tracesdk.Sampler { if ok { sampler.Update(config.SamplerParam) } - default: - var root tracesdk.Sampler - var parentOptions []tracesdk.ParentBasedSamplerOption - if config.SamplerParentConfig.LocalParentSampled { - parentOptions = append(parentOptions, tracesdk.WithLocalParentSampled(root)) - } - if config.SamplerParentConfig.RemoteParentSampled { - parentOptions = append(parentOptions, tracesdk.WithRemoteParentSampled(root)) - } - sampler = tracesdk.ParentBased(root, parentOptions...) } + + // Use parent-based to make sure we respect the span parent, if + // it is sampled. Optionally, allow user to specify the + // parent-based options. + var parentOptions []tracesdk.ParentBasedSamplerOption + if config.SamplerParentConfig.LocalParentSampled { + parentOptions = append(parentOptions, tracesdk.WithLocalParentSampled(sampler)) + } + if config.SamplerParentConfig.RemoteParentSampled { + parentOptions = append(parentOptions, tracesdk.WithRemoteParentSampled(sampler)) + } + sampler = tracesdk.ParentBased(sampler, parentOptions...) + return sampler } diff --git a/pkg/tracing/jaeger/jaeger_test.go b/pkg/tracing/jaeger/jaeger_test.go index bc220bed0ee..5e48b3e88a4 100644 --- a/pkg/tracing/jaeger/jaeger_test.go +++ b/pkg/tracing/jaeger/jaeger_test.go @@ -24,7 +24,7 @@ var parentConfig = ParentBasedSamplerConfig{LocalParentSampled: true} // This test shows that if sample factor will enable tracing on client process, even when it would be disabled on server // it will be still enabled for all spans within this span. -func TestContextTracing_ClientEnablesTracing(t *testing.T) { +func TestContextTracing_ClientEnablesProbabilisticTracing(t *testing.T) { exp := tracetest.NewInMemoryExporter() config := Config{ SamplerType: "probabilistic", @@ -65,7 +65,7 @@ func TestContextTracing_ClientEnablesTracing(t *testing.T) { // This test shows that if sample factor will disable tracing on client process, when it would be enabled on server // it will be still disabled for all spans within this span. -func TestContextTracing_ClientDisablesTracing(t *testing.T) { +func TestContextTracing_ClientDisablesProbabilisticTracing(t *testing.T) { exp := tracetest.NewInMemoryExporter() config := Config{ @@ -105,6 +105,45 @@ func TestContextTracing_ClientDisablesTracing(t *testing.T) { tracing.ContextTracing_ClientDisablesTracing(t, exp, clientRoot, srvRoot, srvChild) } +func TestContextTracing_ClientDisablesAlwaysOnSampling(t *testing.T) { + exp := tracetest.NewInMemoryExporter() + + config := Config{ + SamplerType: SamplerTypeConstant, + SamplerParam: 0, + } + sampler := getSampler(config) + tracerOtel := newTraceProvider( + context.Background(), + "tracerOtel", + log.NewNopLogger(), + tracesdk.NewSimpleSpanProcessor(exp), + sampler, // never sample + []attribute.KeyValue{}, + ) + tracer, _ := migration.Bridge(tracerOtel, log.NewNopLogger()) + + clientRoot, clientCtx := tracing.StartSpan(tracing.ContextWithTracer(context.Background(), tracer), "a") + + config.SamplerParam = 1 + sampler2 := getSampler(config) + // Simulate Server process with different tracer, but with client span in context. + srvTracerOtel := newTraceProvider( + context.Background(), + "srvTracerOtel", + log.NewNopLogger(), + tracesdk.NewSimpleSpanProcessor(exp), + sampler2, // never sample + []attribute.KeyValue{}, + ) + srvTracer, _ := migration.Bridge(srvTracerOtel, log.NewNopLogger()) + + srvRoot, srvCtx := tracing.StartSpan(tracing.ContextWithTracer(clientCtx, srvTracer), "b") + srvChild, _ := tracing.StartSpan(srvCtx, "bb") + + tracing.ContextTracing_ClientDisablesTracing(t, exp, clientRoot, srvRoot, srvChild) +} + // This test shows that if span will contain special baggage (for example from special HTTP header), even when sample // factor will disable client & server tracing, it will be still enabled for all spans within this span. func TestContextTracing_ForceTracing(t *testing.T) {