diff --git a/.github/workflows/orchestrion.yml b/.github/workflows/orchestrion.yml index f7578e4ecd..0603c8b139 100644 --- a/.github/workflows/orchestrion.yml +++ b/.github/workflows/orchestrion.yml @@ -17,7 +17,7 @@ concurrency: jobs: test: name: 'Run Tests' - uses: DataDog/orchestrion/.github/workflows/workflow_call.yml@eliott.bouhana/APPSEC-53773 # we don't want to pin our own action + uses: DataDog/orchestrion/.github/workflows/workflow_call.yml@main # we don't want to pin our own action with: dd-trace-go-ref: ${{ github.sha }} runs-on: ubuntu-latest-16-cores diff --git a/contrib/internal/httptrace/config.go b/contrib/internal/httptrace/config.go index b545a447ce..691a529400 100644 --- a/contrib/internal/httptrace/config.go +++ b/contrib/internal/httptrace/config.go @@ -47,7 +47,7 @@ func newConfig() config { } else if r, err := regexp.Compile(s); err == nil { c.queryStringRegexp = r } else { - log.Debug("Could not compile regexp from %s. Using default regexp instead.", envQueryStringRegexp) + log.Error("Could not compile regexp from %s. Using default regexp instead.", envQueryStringRegexp) } return c } diff --git a/ddtrace/tracer/sampler_test.go b/ddtrace/tracer/sampler_test.go index b28a040037..117518b533 100644 --- a/ddtrace/tracer/sampler_test.go +++ b/ddtrace/tracer/sampler_test.go @@ -1637,3 +1637,87 @@ func TestSetGlobalSampleRate(t *testing.T) { assert.Equal(t, 0.0, rs.globalRate) assert.False(t, b) } + +func TestSampleTagsRootOnly(t *testing.T) { + assert := assert.New(t) + + t.Run("no-ctx-propagation", func(t *testing.T) { + Start(WithSamplingRules([]SamplingRule{ + TagsResourceRule(map[string]string{"tag": "20"}, "", "", "", 1), + TagsResourceRule(nil, "root", "", "", 0), + })) + tr := internal.GetGlobalTracer() + defer tr.Stop() + + root := tr.StartSpan("mysql.root", ResourceName("root")) + child := tr.StartSpan("mysql.child", ChildOf(root.Context())) + child.SetTag("tag", 20) + + // root span should be sampled with the second rule + // sampling decision is 0, thus "_dd.limit_psr" is not present + assert.Contains(root.(*span).Metrics, keyRulesSamplerAppliedRate) + assert.Equal(0., root.(*span).Metrics[keyRulesSamplerAppliedRate]) + assert.NotContains(root.(*span).Metrics, keyRulesSamplerLimiterRate) + + // neither"_dd.limit_psr", nor "_dd.rule_psr" should be present + // on the child span + assert.NotContains(child.(*span).Metrics, keyRulesSamplerAppliedRate) + assert.NotContains(child.(*span).Metrics, keyRulesSamplerLimiterRate) + + // setting this tag would change the result of sampling, + // which will occur after the span is finished + root.SetTag("tag", 20) + child.Finish() + + // first sampling rule is applied, the sampling decision is 1 + // and the "_dd.limit_psr" is present + root.Finish() + assert.Equal(1., root.(*span).Metrics[keyRulesSamplerAppliedRate]) + assert.Contains(root.(*span).Metrics, keyRulesSamplerLimiterRate) + + // neither"_dd.limit_psr", nor "_dd.rule_psr" should be present + // on the child span + assert.NotContains(child.(*span).Metrics, keyRulesSamplerAppliedRate) + assert.NotContains(child.(*span).Metrics, keyRulesSamplerLimiterRate) + }) + + t.Run("with-ctx-propagation", func(t *testing.T) { + Start(WithSamplingRules([]SamplingRule{ + TagsResourceRule(map[string]string{"tag": "20"}, "", "", "", 1), + TagsResourceRule(nil, "root", "", "", 0), + })) + tr := internal.GetGlobalTracer() + defer tr.Stop() + + root := tr.StartSpan("mysql.root", ResourceName("root")) + child := tr.StartSpan("mysql.child", ChildOf(root.Context())) + child.SetTag("tag", 20) + + // root span should be sampled with the second rule + // sampling decision is 0, thus "_dd.limit_psr" is not present + assert.Equal(0., root.(*span).Metrics[keyRulesSamplerAppliedRate]) + assert.Contains(root.(*span).Metrics, keyRulesSamplerAppliedRate) + assert.NotContains(root.(*span).Metrics, keyRulesSamplerLimiterRate) + + // neither"_dd.limit_psr", nor "_dd.rule_psr" should be present + // on the child span + assert.NotContains(child.(*span).Metrics, keyRulesSamplerAppliedRate) + assert.NotContains(child.(*span).Metrics, keyRulesSamplerLimiterRate) + + // context propagation locks the span, so no re-sampling should occur + tr.Inject(root.Context(), TextMapCarrier(map[string]string{})) + root.SetTag("tag", 20) + + child.Finish() + + // re-sampling should not occur + root.Finish() + assert.NotContains(child.(*span).Metrics, keyRulesSamplerAppliedRate) + assert.NotContains(root.(*span).Metrics, keyRulesSamplerLimiterRate) + + // neither"_dd.limit_psr", nor "_dd.rule_psr" should be present + // on the child span + assert.NotContains(child.(*span).Metrics, keyRulesSamplerAppliedRate) + assert.NotContains(child.(*span).Metrics, keyRulesSamplerLimiterRate) + }) +} diff --git a/ddtrace/tracer/span.go b/ddtrace/tracer/span.go index 0537c82ab3..117577f221 100644 --- a/ddtrace/tracer/span.go +++ b/ddtrace/tracer/span.go @@ -500,9 +500,11 @@ func (s *span) Finish(opts ...ddtrace.FinishOption) { s.SetTag("go_execution_traced", "partial") } - if tr, ok := internal.GetGlobalTracer().(*tracer); ok && tr.rulesSampling.traces.enabled() { - if !s.context.trace.isLocked() && s.context.trace.propagatingTag(keyDecisionMaker) != "-4" { - tr.rulesSampling.SampleTrace(s) + if s.root() == s { + if tr, ok := internal.GetGlobalTracer().(*tracer); ok && tr.rulesSampling.traces.enabled() { + if !s.context.trace.isLocked() && s.context.trace.propagatingTag(keyDecisionMaker) != "-4" { + tr.rulesSampling.SampleTrace(s) + } } } diff --git a/ddtrace/tracer/span_test.go b/ddtrace/tracer/span_test.go index ddf2337ef0..51ff73f3fb 100644 --- a/ddtrace/tracer/span_test.go +++ b/ddtrace/tracer/span_test.go @@ -164,42 +164,6 @@ func TestShouldComputeStats(t *testing.T) { } } -// -//func TestNewAggregableSpan(t *testing.T) { -// c := newConcentrator(&config{}, 1) -// t.Run("obfuscating", func(t *testing.T) { -// o := obfuscate.NewObfuscator(obfuscate.Config{}) -// aggspan, _ := c.newAggregableSpan(&span{ -// Name: "name", -// Resource: "SELECT * FROM table WHERE password='secret'", -// Service: "service", -// Type: "sql", -// }, o) -// assert.Equal(t, "SELECT * FROM table WHERE password = ?", aggspan.) -// assert.Equal(t, aggregation{ -// Name: "name", -// Type: "sql", -// Resource: , -// Service: "service", -// }, aggspan.key) -// }) -// -// t.Run("nil-obfuscator", func(t *testing.T) { -// aggspan := newAggregableSpan(&span{ -// Name: "name", -// Resource: "SELECT * FROM table WHERE password='secret'", -// Service: "service", -// Type: "sql", -// }, nil) -// assert.Equal(t, aggregation{ -// Name: "name", -// Type: "sql", -// Resource: "SELECT * FROM table WHERE password='secret'", -// Service: "service", -// }, aggspan.key) -// }) -//} - func TestSpanFinishWithTime(t *testing.T) { assert := assert.New(t) diff --git a/ddtrace/tracer/tracer.go b/ddtrace/tracer/tracer.go index af794c7857..f0c74eecef 100644 --- a/ddtrace/tracer/tracer.go +++ b/ddtrace/tracer/tracer.go @@ -735,6 +735,9 @@ func (t *tracer) sample(span *span) { if t.rulesSampling.SampleTraceGlobalRate(span) { return } + if t.rulesSampling.SampleTrace(span) { + return + } t.prioritySampling.apply(span) } diff --git a/docker-compose.yaml b/docker-compose.yaml index 1b6008ac61..17788504ee 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -100,9 +100,9 @@ services: TRACE_LANGUAGE: golang ENABLED_CHECKS: trace_stall,trace_count_header,trace_peer_service,trace_dd_service PORT: 9126 - DD_SUPPRESS_TRACE_PARSE_ERRORS: true - DD_POOL_TRACE_CHECK_FAILURES: true - DD_DISABLE_ERROR_RESPONSES: true + DD_SUPPRESS_TRACE_PARSE_ERRORS: "true" + DD_POOL_TRACE_CHECK_FAILURES: "true" + DD_DISABLE_ERROR_RESPONSES: "true" ports: - "127.0.0.1:9126:9126" mongodb: diff --git a/internal/appsec/emitter/httpsec/http.go b/internal/appsec/emitter/httpsec/http.go index 37c56ef283..4ead65e548 100644 --- a/internal/appsec/emitter/httpsec/http.go +++ b/internal/appsec/emitter/httpsec/http.go @@ -93,7 +93,7 @@ func WrapHandler(handler http.Handler, span ddtrace.Span, pathParams map[string] r = r.WithContext(ctx) defer func() { - events := op.Finish(MakeHandlerOperationRes(w)) + events := op.Finish(MakeHandlerOperationRes(w, opts.ResponseHeaderCopier)) // Execute the onBlock functions to make sure blocking works properly // in case we are instrumenting the Gin framework @@ -149,12 +149,12 @@ func MakeHandlerOperationArgs(r *http.Request, clientIP netip.Addr, pathParams m } // MakeHandlerOperationRes creates the HandlerOperationRes value. -func MakeHandlerOperationRes(w http.ResponseWriter) types.HandlerOperationRes { +func MakeHandlerOperationRes(w http.ResponseWriter, responseHeadersCopier func(http.ResponseWriter) http.Header) types.HandlerOperationRes { var status int if mw, ok := w.(interface{ Status() int }); ok { status = mw.Status() } - return types.HandlerOperationRes{Status: status, Headers: headersRemoveCookies(w.Header())} + return types.HandlerOperationRes{Status: status, Headers: headersRemoveCookies(responseHeadersCopier(w))} } // Remove cookies from the request headers and return the map of headers