Skip to content

Commit

Permalink
Merge branch 'main' into andrew.glaude/spanConcentrator2
Browse files Browse the repository at this point in the history
  • Loading branch information
ajgajg1134 committed Sep 3, 2024
2 parents 04aa447 + 2c85bdc commit 521f7d4
Show file tree
Hide file tree
Showing 8 changed files with 100 additions and 47 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/orchestrion.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion contrib/internal/httptrace/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
84 changes: 84 additions & 0 deletions ddtrace/tracer/sampler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
8 changes: 5 additions & 3 deletions ddtrace/tracer/span.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}

Expand Down
36 changes: 0 additions & 36 deletions ddtrace/tracer/span_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
3 changes: 3 additions & 0 deletions ddtrace/tracer/tracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
6 changes: 3 additions & 3 deletions docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 3 additions & 3 deletions internal/appsec/emitter/httpsec/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 521f7d4

Please sign in to comment.