Skip to content
This repository has been archived by the owner on Aug 30, 2019. It is now read-only.

cmd/trace-agent: force sampling of transactions with priority 2 #463

Merged
merged 2 commits into from
Aug 3, 2018
Merged

cmd/trace-agent: force sampling of transactions with priority 2 #463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7560336
Force sampling of transactions with priority 2
bmermet Jul 25, 2018
f903d5f
Address comments
bmermet Jul 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions cmd/trace-agent/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (

const (
processStatsInterval = time.Minute
samplingPriorityKey = "_sampling_priority_v1"
)

type processedTrace struct {
Expand All @@ -39,6 +38,14 @@ func (pt *processedTrace) weight() float64 {
return pt.Root.Weight()
}

func (pt *processedTrace) getSamplingPriority() (int, bool) {
if pt.Root == nil {
return 0, false
}
p, ok := pt.Root.Metrics[sampler.SamplingPriorityKey]
return int(p), ok
}

// Agent struct holds all the sub-routines structs and make the data flow between them
type Agent struct {
Receiver *HTTPReceiver
Expand Down Expand Up @@ -193,7 +200,7 @@ func (a *Agent) Process(t model.Trace) {
samplers = append(samplers, a.ScoreSampler)
}

priority, hasPriority := root.Metrics[samplingPriorityKey]
priority, hasPriority := root.Metrics[sampler.SamplingPriorityKey]
if hasPriority {
// If Priority is defined, send to priority sampling, regardless of priority value.
// The sampler will keep or discard the trace, but we send everything so that it
Expand Down
10 changes: 7 additions & 3 deletions cmd/trace-agent/transaction_sampler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,20 +43,24 @@ func newTransactionSampler(analyzedSpansByService map[string]map[string]float64)
func (s *transactionSampler) Extract(t processedTrace) []*model.Span {
var transactions []*model.Span

// Get the trace priority
priority, hasPriority := t.getSamplingPriority()
// inspect the WeightedTrace so that we can identify top-level spans
for _, span := range t.WeightedTrace {
if s.shouldAnalyze(span) {
if s.shouldAnalyze(span, hasPriority, priority) {
transactions = append(transactions, span.Span)
}
}

return transactions
}

func (s *transactionSampler) shouldAnalyze(span *model.WeightedSpan) bool {
func (s *transactionSampler) shouldAnalyze(span *model.WeightedSpan, hasPriority bool, priority int) bool {
if operations, ok := s.analyzedSpansByService[span.Service]; ok {
if analyzeRate, ok := operations[span.Name]; ok {
if sampler.SampleByRate(span.TraceID, analyzeRate) {
// If the trace has been manually sampled, we keep all matching spans
highPriority := hasPriority && priority >= 2
if highPriority || sampler.SampleByRate(span.TraceID, analyzeRate) {
return true
}
}
Expand Down
31 changes: 21 additions & 10 deletions cmd/trace-agent/transaction_sampler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,18 @@ import (
"testing"

"github.com/DataDog/datadog-trace-agent/model"
"github.com/DataDog/datadog-trace-agent/sampler"
"github.com/stretchr/testify/assert"
)

func createTrace(serviceName string, operationName string, topLevel bool) processedTrace {
func createTrace(serviceName string, operationName string, topLevel bool, hasPriority bool, priority int) processedTrace {
ws := model.WeightedSpan{TopLevel: topLevel, Span: &model.Span{Service: serviceName, Name: operationName}}
if hasPriority {
ws.Metrics = make(map[string]float64)
ws.Metrics[sampler.SamplingPriorityKey] = float64(priority)
}
wt := model.WeightedTrace{&ws}
return processedTrace{WeightedTrace: wt}
return processedTrace{WeightedTrace: wt, Root: ws.Span}
}

func TestTransactionSampler(t *testing.T) {
Expand All @@ -21,19 +26,25 @@ func TestTransactionSampler(t *testing.T) {
config["myService"] = make(map[string]float64)
config["myService"]["myOperation"] = 1

config["mySampledService"] = make(map[string]float64)
config["mySampledService"]["myOperation"] = 0

tests := []struct {
name string
trace processedTrace
expectedSampling bool
}{
{"Top-level service and span name match", createTrace("myService", "myOperation", true), true},
{"Top-level service name doesn't match", createTrace("otherService", "myOperation", true), false},
{"Top-level span name doesn't match", createTrace("myService", "otherOperation", true), false},
{"Top-level service and span name don't match", createTrace("otherService", "otherOperation", true), false},
{"Non top-level service and span name match", createTrace("myService", "myOperation", false), true},
{"Non top-level service name doesn't match", createTrace("otherService", "myOperation", false), false},
{"Non top-level span name doesn't match", createTrace("myService", "otherOperation", false), false},
{"Non top-level service and span name don't match", createTrace("otherService", "otherOperation", false), false},
{"Top-level service and span name match", createTrace("myService", "myOperation", true, false, 0), true},
{"Top-level service name doesn't match", createTrace("otherService", "myOperation", true, false, 0), false},
{"Top-level span name doesn't match", createTrace("myService", "otherOperation", true, false, 0), false},
{"Top-level service and span name don't match", createTrace("otherService", "otherOperation", true, false, 0), false},
{"Non top-level service and span name match", createTrace("myService", "myOperation", false, false, 0), true},
{"Non top-level service name doesn't match", createTrace("otherService", "myOperation", false, false, 0), false},
{"Non top-level span name doesn't match", createTrace("myService", "otherOperation", false, false, 0), false},
{"Non top-level service and span name don't match", createTrace("otherService", "otherOperation", false, false, 0), false},
{"Match, sampling rate 0, no priority", createTrace("mySampledService", "myOperation", true, false, 0), false},
{"Match, sampling rate 0, priority 1", createTrace("mySampledService", "myOperation", true, true, 1), false},
{"Match, sampling rate 0, priority 2", createTrace("mySampledService", "myOperation", true, true, 2), true},
}

for _, test := range tests {
Expand Down
5 changes: 3 additions & 2 deletions sampler/prioritysampler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ import (
)

const (
samplingPriorityKey = "_sampling_priority_v1"
// SamplingPriorityKey is the key of the sampling priority value in the metrics map of the root span
SamplingPriorityKey = "_sampling_priority_v1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to use KeySamplingPriority? We could make it a convention for constants representing keys. It would be in the same style as the standard library http package constants (link)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how much of a common practice it is in go. If you think it is the idiomatic way to do it, let's do that.

syncPeriod = 3 * time.Second
)

Expand Down Expand Up @@ -91,7 +92,7 @@ func (s *PriorityEngine) Sample(trace model.Trace, root *model.Span, env string)
return false
}

samplingPriority := root.Metrics[samplingPriorityKey]
samplingPriority := root.Metrics[SamplingPriorityKey]

// Regardless of rates, sampling here is based on the metadata set
// by the client library. Which, is turn, is based on agent hints,
Expand Down
14 changes: 7 additions & 7 deletions sampler/prioritysampler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func getTestTraceWithService(t *testing.T, service string, s *PriorityEngine) (m
if r <= rate {
priority = 1
}
trace[0].Metrics = map[string]float64{samplingPriorityKey: priority}
trace[0].Metrics = map[string]float64{SamplingPriorityKey: priority}
return trace, trace[0]
}

Expand All @@ -67,44 +67,44 @@ func TestPrioritySample(t *testing.T) {
s = getTestPriorityEngine()
trace, root = getTestTraceWithService(t, "my-service", s)

root.Metrics[samplingPriorityKey] = -1
root.Metrics[SamplingPriorityKey] = -1
assert.False(s.Sample(trace, root, env), "trace with negative priority is dropped")
assert.Equal(0.0, s.Sampler.Backend.GetTotalScore(), "sampling a priority -1 trace should *NOT* impact sampler backend")
assert.Equal(0.0, s.Sampler.Backend.GetSampledScore(), "sampling a priority -1 trace should *NOT* impact sampler backend")

s = getTestPriorityEngine()
trace, root = getTestTraceWithService(t, "my-service", s)

root.Metrics[samplingPriorityKey] = 0
root.Metrics[SamplingPriorityKey] = 0
assert.False(s.Sample(trace, root, env), "trace with priority 0 is dropped")
assert.True(0.0 < s.Sampler.Backend.GetTotalScore(), "sampling a priority 0 trace should increase total score")
assert.Equal(0.0, s.Sampler.Backend.GetSampledScore(), "sampling a priority 0 trace should *NOT* increase sampled score")

s = getTestPriorityEngine()
trace, root = getTestTraceWithService(t, "my-service", s)

root.Metrics[samplingPriorityKey] = 1
root.Metrics[SamplingPriorityKey] = 1
assert.True(s.Sample(trace, root, env), "trace with priority 1 is kept")
assert.True(0.0 < s.Sampler.Backend.GetTotalScore(), "sampling a priority 0 trace should increase total score")
assert.True(0.0 < s.Sampler.Backend.GetSampledScore(), "sampling a priority 0 trace should increase sampled score")

s = getTestPriorityEngine()
trace, root = getTestTraceWithService(t, "my-service", s)

root.Metrics[samplingPriorityKey] = 2
root.Metrics[SamplingPriorityKey] = 2
assert.True(s.Sample(trace, root, env), "trace with priority 2 is kept")
assert.Equal(0.0, s.Sampler.Backend.GetTotalScore(), "sampling a priority 2 trace should *NOT* increase total score")
assert.Equal(0.0, s.Sampler.Backend.GetSampledScore(), "sampling a priority 2 trace should *NOT* increase sampled score")

s = getTestPriorityEngine()
trace, root = getTestTraceWithService(t, "my-service", s)

root.Metrics[samplingPriorityKey] = 999
root.Metrics[SamplingPriorityKey] = 999
assert.True(s.Sample(trace, root, env), "trace with high priority is kept")
assert.Equal(0.0, s.Sampler.Backend.GetTotalScore(), "sampling a high priority trace should *NOT* increase total score")
assert.Equal(0.0, s.Sampler.Backend.GetSampledScore(), "sampling a high priority trace should *NOT* increase sampled score")

delete(root.Metrics, samplingPriorityKey)
delete(root.Metrics, SamplingPriorityKey)
assert.False(s.Sample(trace, root, env), "this should not happen but a trace without priority sampling set should be dropped")
}

Expand Down