From cd3a001746f53958256134ee98ef854367f64ab8 Mon Sep 17 00:00:00 2001 From: "diana.shevchenko" Date: Tue, 11 Oct 2022 11:05:42 +0100 Subject: [PATCH 1/4] removed restriction of non-empty fields in sampling rules --- ddtrace/tracer/doc.go | 36 ++++++++++++++++++++------------- ddtrace/tracer/rules_sampler.go | 4 ---- ddtrace/tracer/sampler_test.go | 7 ++++--- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/ddtrace/tracer/doc.go b/ddtrace/tracer/doc.go index bcd33ccfb9..246bd4ddae 100644 --- a/ddtrace/tracer/doc.go +++ b/ddtrace/tracer/doc.go @@ -9,22 +9,25 @@ // tracer, simply call the start method along with an optional set of options. // By default, the trace agent is considered to be found at "localhost:8126". In a // setup where this would be different (let's say 127.0.0.1:1234), we could do: -// tracer.Start(tracer.WithAgentAddr("127.0.0.1:1234")) -// defer tracer.Stop() +// +// tracer.Start(tracer.WithAgentAddr("127.0.0.1:1234")) +// defer tracer.Stop() // // The tracing client can perform trace sampling. While the trace agent // already samples traces to reduce bandwidth usage, client sampling reduces // performance overhead. To make use of it, the package comes with a ready-to-use // rate sampler that can be passed to the tracer. To use it and keep only 30% of the // requests, one would do: -// s := tracer.NewRateSampler(0.3) -// tracer.Start(tracer.WithSampler(s)) +// +// s := tracer.NewRateSampler(0.3) +// tracer.Start(tracer.WithSampler(s)) // // More precise control of sampling rates can be configured using sampling rules. // This can be applied based on span name, service or both, and is used to determine // the sampling rate to apply. MaxPerSecond specifies max number of spans per second // that can be sampled per the rule and applies only to sampling rules of type // tracer.SamplingRuleSpan. If MaxPerSecond is not specified, the default is no limit. +// // rules := []tracer.SamplingRule{ // // sample 10% of traces with the span name "web.request" // tracer.NameRule("web.request", 0.1), @@ -47,9 +50,10 @@ // DD_SPAN_SAMPLING_RULES environment variables. When set, it overrides rules set by tracer.WithSamplingRules. // The value is a JSON array of objects. All rule objects must have a "sample_rate". // For trace sampling rules the "name" and "service" fields are optional. -// For span sampling rules, at least one of the fields must be specified and must be a valid glob pattern, -// i.e. a string where "*" matches any contiguous substring, even the empty string, +// For span sampling rules, the "name" and "service", if specified, must be a valid glob pattern, +// i.e. a string where "*" matches any contiguous substring, even an empty string, // and "?" character matches exactly one of any character. +// // export DD_TRACE_SAMPLING_RULES='[{"name": "web.request", "sample_rate": 1.0}]' // export DD_SPAN_SAMPLING_RULES='[{"service":"test.?","name": "web.*", "sample_rate": 1.0, "max_per_second":100}]' // @@ -80,15 +84,19 @@ // interfaces. An example alternate implementation is the MDCarrier in our gRPC integration. // // As an example, injecting a span's context into an HTTP request would look like this: -// req, err := http.NewRequest("GET", "http://example.com", nil) -// // ... -// err := tracer.Inject(span.Context(), tracer.HTTPHeadersCarrier(req.Header)) -// // ... -// http.DefaultClient.Do(req) +// +// req, err := http.NewRequest("GET", "http://example.com", nil) +// // ... +// err := tracer.Inject(span.Context(), tracer.HTTPHeadersCarrier(req.Header)) +// // ... +// http.DefaultClient.Do(req) +// // Then, on the server side, to continue the trace one would do: -// sctx, err := tracer.Extract(tracer.HTTPHeadersCarrier(req.Header)) -// // ... -// span := tracer.StartSpan("child.span", tracer.ChildOf(sctx)) +// +// sctx, err := tracer.Extract(tracer.HTTPHeadersCarrier(req.Header)) +// // ... +// span := tracer.StartSpan("child.span", tracer.ChildOf(sctx)) +// // In the same manner, any means can be used as a carrier to inject a context into a transport. Go's // context can also be used as a means to transport spans within the same process. The methods // StartSpanFromContext, ContextWithSpan and SpanFromContext exist for this reason. diff --git a/ddtrace/tracer/rules_sampler.go b/ddtrace/tracer/rules_sampler.go index b7ac1daeae..c0b1b60100 100644 --- a/ddtrace/tracer/rules_sampler.go +++ b/ddtrace/tracer/rules_sampler.go @@ -524,10 +524,6 @@ func unmarshalSamplingRules(b []byte, spanType SamplingRuleType) ([]SamplingRule } switch spanType { case SamplingRuleSpan: - if v.Service == "" && v.Name == "" { - errs = append(errs, fmt.Sprintf("at index %d: ignoring rule %+v: service name and operation name are not provided", i, v)) - continue - } rules = append(rules, SamplingRule{ Service: globMatch(v.Service), Name: globMatch(v.Name), diff --git a/ddtrace/tracer/sampler_test.go b/ddtrace/tracer/sampler_test.go index 1aacd0d2e5..f640755be4 100644 --- a/ddtrace/tracer/sampler_test.go +++ b/ddtrace/tracer/sampler_test.go @@ -287,6 +287,9 @@ func TestRuleEnvVars(t *testing.T) { }, { value: `[{"service": "abcd", "sample_rate": 1.0}]`, ruleN: 1, + }, { + value: `[{"sample_rate": 1.0}]`, + ruleN: 1, }, { value: `[{"service": "abcd", "sample_rate": 1.0},{"name": "wxyz", "sample_rate": 0.9},{"service": "efgh", "name": "lmnop", "sample_rate": 0.42}]`, ruleN: 3, @@ -298,9 +301,6 @@ func TestRuleEnvVars(t *testing.T) { }, { value: `not JSON at all`, errStr: "\n\terror unmarshalling JSON: invalid character 'o' in literal null (expecting 'u')", - }, { - value: `[{"sample_rate": 1.0}]`, - errStr: "\n\tat index 0: ignoring rule {Service: Name: Rate:1.0 MaxPerSecond:0}: service name and operation name are not provided", }, } { t.Run(fmt.Sprintf("%v", i), func(t *testing.T) { @@ -315,6 +315,7 @@ func TestRuleEnvVars(t *testing.T) { }) } }) + t.Run("span-sampling-rules-regex", func(t *testing.T) { assert := assert.New(t) defer os.Unsetenv("DD_SPAN_SAMPLING_RULES") From af250a0f8431703b74bf7a32e8949969b9632439 Mon Sep 17 00:00:00 2001 From: "diana.shevchenko" Date: Tue, 11 Oct 2022 12:57:40 +0100 Subject: [PATCH 2/4] unset sample rate for single span sampling defaults to 0 --- ddtrace/tracer/rules_sampler.go | 22 ++++++++++++++++++++-- ddtrace/tracer/sampler_test.go | 3 +++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/ddtrace/tracer/rules_sampler.go b/ddtrace/tracer/rules_sampler.go index c0b1b60100..348553b2c4 100644 --- a/ddtrace/tracer/rules_sampler.go +++ b/ddtrace/tracer/rules_sampler.go @@ -510,8 +510,12 @@ func unmarshalSamplingRules(b []byte, spanType SamplingRuleType) ([]SamplingRule var errs []string for i, v := range jsonRules { if v.Rate == "" { - errs = append(errs, fmt.Sprintf("at index %d: rate not provided", i)) - continue + if spanType == SamplingRuleSpan { + v.Rate = "0" + } else { + errs = append(errs, fmt.Sprintf("at index %d: rate not provided", i)) + continue + } } rate, err := v.Rate.Float64() if err != nil { @@ -533,6 +537,20 @@ func unmarshalSamplingRules(b []byte, spanType SamplingRuleType) ([]SamplingRule ruleType: SamplingRuleSpan, }) case SamplingRuleTrace: + if v.Rate == "" { + errs = append(errs, fmt.Sprintf("at index %d: rate not provided", i)) + continue + } + rate, err := v.Rate.Float64() + if err != nil { + errs = append(errs, fmt.Sprintf("at index %d: %v", i, err)) + continue + } + if rate < 0.0 || rate > 1.0 { + errs = append(errs, fmt.Sprintf("at index %d: ignoring rule %+v: rate is out of [0.0, 1.0] range", i, v)) + continue + } + switch { case v.Service != "" && v.Name != "": rules = append(rules, NameServiceRule(v.Name, v.Service, rate)) diff --git a/ddtrace/tracer/sampler_test.go b/ddtrace/tracer/sampler_test.go index f640755be4..20ac5da472 100644 --- a/ddtrace/tracer/sampler_test.go +++ b/ddtrace/tracer/sampler_test.go @@ -287,6 +287,9 @@ func TestRuleEnvVars(t *testing.T) { }, { value: `[{"service": "abcd", "sample_rate": 1.0}]`, ruleN: 1, + }, { + value: `[{"service": "abcd", "name": "wxyz"}]`, + ruleN: 1, }, { value: `[{"sample_rate": 1.0}]`, ruleN: 1, From 35e789cfd35b9c8f63da12a5d174104f4154ac8b Mon Sep 17 00:00:00 2001 From: "diana.shevchenko" Date: Wed, 12 Oct 2022 11:29:48 +0100 Subject: [PATCH 3/4] added more test examples to single span sampling rules unmarshalling --- ddtrace/tracer/rules_sampler.go | 2 +- ddtrace/tracer/sampler_test.go | 30 ++++++++++++++++++++++++++++-- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/ddtrace/tracer/rules_sampler.go b/ddtrace/tracer/rules_sampler.go index 348553b2c4..e466d3815e 100644 --- a/ddtrace/tracer/rules_sampler.go +++ b/ddtrace/tracer/rules_sampler.go @@ -511,7 +511,7 @@ func unmarshalSamplingRules(b []byte, spanType SamplingRuleType) ([]SamplingRule for i, v := range jsonRules { if v.Rate == "" { if spanType == SamplingRuleSpan { - v.Rate = "0" + v.Rate = "1" } else { errs = append(errs, fmt.Sprintf("at index %d: rate not provided", i)) continue diff --git a/ddtrace/tracer/sampler_test.go b/ddtrace/tracer/sampler_test.go index 20ac5da472..2ed61a595f 100644 --- a/ddtrace/tracer/sampler_test.go +++ b/ddtrace/tracer/sampler_test.go @@ -287,6 +287,9 @@ func TestRuleEnvVars(t *testing.T) { }, { value: `[{"service": "abcd", "sample_rate": 1.0}]`, ruleN: 1, + }, { + value: `[{"sample_rate": 1.0}, {"service": "abcd"}, {"name": "abcd"}, {}]`, + ruleN: 4, }, { value: `[{"service": "abcd", "name": "wxyz"}]`, ruleN: 1, @@ -327,21 +330,43 @@ func TestRuleEnvVars(t *testing.T) { rules string srvRegex string nameRegex string + rate float64 }{ { rules: `[{"name": "abcd?", "sample_rate": 1.0}]`, srvRegex: "^.*$", nameRegex: "^abcd.$", + rate: 1.0, + }, + { + rules: `[{"sample_rate": 0.5}]`, + srvRegex: "^.*$", + nameRegex: "^.*$", + rate: 0.5, + }, + { + rules: `[{"max_per_second":100}]`, + srvRegex: "^.*$", + nameRegex: "^.*$", + rate: 1, + }, + { + rules: `[{"name": "abcd?"}]`, + srvRegex: "^.*$", + nameRegex: "^abcd.$", + rate: 1.0, }, { - rules: `[{"service": "*abcd", "sample_rate": 1.0}]`, + rules: `[{"service": "*abcd", "sample_rate":0.5}]`, nameRegex: "^.*$", srvRegex: "^.*abcd$", + rate: 0.5, }, { - rules: `[{"service": "*abcd", "sample_rate": 1.0}]`, + rules: `[{"service": "*abcd", "sample_rate": 0.5}]`, nameRegex: "^.*$", srvRegex: "^.*abcd$", + rate: 0.5, }, } { t.Run(fmt.Sprintf("%v", i), func(t *testing.T) { @@ -350,6 +375,7 @@ func TestRuleEnvVars(t *testing.T) { assert.NoError(err) assert.Equal(tt.srvRegex, rules[0].Service.String()) assert.Equal(tt.nameRegex, rules[0].Name.String()) + assert.Equal(tt.rate, rules[0].Rate) }) } }) From 8be0648beca67bd2d02d1da1772eec3d2a0cc5e2 Mon Sep 17 00:00:00 2001 From: "diana.shevchenko" Date: Wed, 12 Oct 2022 11:45:40 +0100 Subject: [PATCH 4/4] updated the sampling section in doc.go --- ddtrace/tracer/doc.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ddtrace/tracer/doc.go b/ddtrace/tracer/doc.go index 246bd4ddae..cc49a2b590 100644 --- a/ddtrace/tracer/doc.go +++ b/ddtrace/tracer/doc.go @@ -48,11 +48,13 @@ // // Sampling rules can also be configured at runtime using the DD_TRACE_SAMPLING_RULES and // DD_SPAN_SAMPLING_RULES environment variables. When set, it overrides rules set by tracer.WithSamplingRules. -// The value is a JSON array of objects. All rule objects must have a "sample_rate". -// For trace sampling rules the "name" and "service" fields are optional. +// The value is a JSON array of objects. +// For trace sampling rules, the "sample_rate" field is required, the "name" and "service" fields are optional. // For span sampling rules, the "name" and "service", if specified, must be a valid glob pattern, // i.e. a string where "*" matches any contiguous substring, even an empty string, // and "?" character matches exactly one of any character. +// The "sample_rate" field is optional, and if not specified, defaults to "1.0", sampling 100% of the spans. +// The "max_per_second" field is optional, and if not specified, defaults to 0, keeping all the previously sampled spans. // // export DD_TRACE_SAMPLING_RULES='[{"name": "web.request", "sample_rate": 1.0}]' // export DD_SPAN_SAMPLING_RULES='[{"service":"test.?","name": "web.*", "sample_rate": 1.0, "max_per_second":100}]'