Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Remove trace key params -> add meta sample_key #685

Merged
merged 1 commit into from
May 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 7 additions & 1 deletion collect/collect.go
Original file line number Diff line number Diff line change
Expand Up @@ -688,10 +688,13 @@ func (i *InMemCollector) send(trace *types.Trace, reason string) {
}

// make sampling decision and update the trace
rate, shouldSend, reason := sampler.GetSampleRate(trace)
rate, shouldSend, reason, key := sampler.GetSampleRate(trace)
trace.SampleRate = rate
trace.KeepSample = shouldSend
logFields["reason"] = reason
if key != "" {
logFields["sample_key"] = key
}

i.sampleTraceCache.Record(trace, shouldSend)

Expand All @@ -711,6 +714,9 @@ func (i *InMemCollector) send(trace *types.Trace, reason string) {
for _, sp := range trace.GetSpans() {
if i.Config.GetAddRuleReasonToTrace() {
sp.Data["meta.refinery.reason"] = reason
if key != "" {
sp.Data["meta.refinery.sample_key"] = key
}
}

// update the root span (if we have one, which we might not if the trace timed out)
Expand Down
6 changes: 3 additions & 3 deletions collect/collect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,14 +362,14 @@ func TestDryRunMode(t *testing.T) {
var traceID2 = "def456"
var traceID3 = "ghi789"
// sampling decisions based on trace ID
sampleRate1, keepTraceID1, _ := sampler.GetSampleRate(&types.Trace{TraceID: traceID1})
sampleRate1, keepTraceID1, _, _ := sampler.GetSampleRate(&types.Trace{TraceID: traceID1})
// would be dropped if dry run mode was not enabled
assert.False(t, keepTraceID1)
assert.Equal(t, uint(10), sampleRate1)
sampleRate2, keepTraceID2, _ := sampler.GetSampleRate(&types.Trace{TraceID: traceID2})
sampleRate2, keepTraceID2, _, _ := sampler.GetSampleRate(&types.Trace{TraceID: traceID2})
assert.True(t, keepTraceID2)
assert.Equal(t, uint(10), sampleRate2)
sampleRate3, keepTraceID3, _ := sampler.GetSampleRate(&types.Trace{TraceID: traceID3})
sampleRate3, keepTraceID3, _, _ := sampler.GetSampleRate(&types.Trace{TraceID: traceID3})
// would be dropped if dry run mode was not enabled
assert.False(t, keepTraceID3)
assert.Equal(t, uint(10), sampleRate3)
Expand Down
29 changes: 0 additions & 29 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -370,35 +370,6 @@ func TestPeerManagementType(t *testing.T) {
}
}

func TestAbsentTraceKeyField(t *testing.T) {
config, rules := createTempConfigs(t, `
[InMemCollector]
CacheCapacity=1000

[HoneycombMetrics]
MetricsHoneycombAPI="http://honeycomb.io"
MetricsAPIKey="1234"
MetricsDataset="testDatasetName"
MetricsReportingInterval=3
`, `
[dataset1]
Sampler = "EMADynamicSampler"
GoalSampleRate = 10
UseTraceLength = true
AddSampleRateKeyToTrace = true
FieldList = [ "request.method" ]
Weight = 0.4
`)
defer os.Remove(rules)
defer os.Remove(config)
cfg, err := getConfig([]string{"--config", config, "--rules_config", rules})
assert.NoError(t, err)
_, samplerName, err := cfg.GetSamplerConfigForDestName("dataset1")
assert.Error(t, err)
assert.Equal(t, "EMADynamicSampler", samplerName)
assert.Contains(t, err.Error(), "Error:Field validation for 'AddSampleRateKeyToTraceField'")
}

func TestDebugServiceAddr(t *testing.T) {
config, rules := createTempConfigs(t, `
DebugServiceAddr = "localhost:8085"
Expand Down
40 changes: 17 additions & 23 deletions config/sampler_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,35 +29,29 @@ type DeterministicSamplerConfig struct {
}

type DynamicSamplerConfig struct {
SampleRate int64 `json:"samplerate" yaml:"SampleRate,omitempty" validate:"required,gte=1"`
ClearFrequencySec int64 `json:"clearfrequencysec" yaml:"ClearFrequencySec,omitempty"`
FieldList []string `json:"fieldlist" yaml:"FieldList,omitempty" validate:"required"`
UseTraceLength bool `json:"usetracelength" yaml:"UseTraceLength,omitempty"`
AddSampleRateKeyToTrace bool `json:"addsampleratekeytotrace" yaml:"AddSampleRateKeyToTrace,omitempty"`
AddSampleRateKeyToTraceField string `json:"addsampleratekeytotracefield" yaml:"AddSampleRateKeyToTraceField,omitempty" validate:"required_with=AddSampleRateKeyToTrace"`
SampleRate int64 `json:"samplerate" yaml:"SampleRate,omitempty" validate:"required,gte=1"`
ClearFrequencySec int64 `json:"clearfrequencysec" yaml:"ClearFrequencySec,omitempty"`
FieldList []string `json:"fieldlist" yaml:"FieldList,omitempty" validate:"required"`
UseTraceLength bool `json:"usetracelength" yaml:"UseTraceLength,omitempty"`
}

type EMADynamicSamplerConfig struct {
GoalSampleRate int `json:"goalsamplerate" yaml:"GoalSampleRate,omitempty" validate:"gte=1"`
AdjustmentInterval int `json:"adjustmentinterval" yaml:"AdjustmentInterval,omitempty"`
Weight float64 `json:"weight" yaml:"Weight,omitempty" validate:"gt=0,lt=1"`
AgeOutValue float64 `json:"ageoutvalue" yaml:"AgeOutValue,omitempty"`
BurstMultiple float64 `json:"burstmultiple" yaml:"BurstMultiple,omitempty"`
BurstDetectionDelay uint `json:"burstdetectiondelay" yaml:"BurstDetectionDelay,omitempty"`
MaxKeys int `json:"maxkeys" yaml:"MaxKeys,omitempty"`
FieldList []string `json:"fieldlist" yaml:"FieldList,omitempty" validate:"required"`
UseTraceLength bool `json:"usetracelength" yaml:"UseTraceLength,omitempty"`
AddSampleRateKeyToTrace bool `json:"addsampleratekeytotrace" yaml:"AddSampleRateKeyToTrace,omitempty"`
AddSampleRateKeyToTraceField string `json:"addsampleratekeytotracefield" yaml:"AddSampleRateKeyToTraceField,omitempty" validate:"required_with=AddSampleRateKeyToTrace"`
GoalSampleRate int `json:"goalsamplerate" yaml:"GoalSampleRate,omitempty" validate:"gte=1"`
AdjustmentInterval int `json:"adjustmentinterval" yaml:"AdjustmentInterval,omitempty"`
Weight float64 `json:"weight" yaml:"Weight,omitempty" validate:"gt=0,lt=1"`
AgeOutValue float64 `json:"ageoutvalue" yaml:"AgeOutValue,omitempty"`
BurstMultiple float64 `json:"burstmultiple" yaml:"BurstMultiple,omitempty"`
BurstDetectionDelay uint `json:"burstdetectiondelay" yaml:"BurstDetectionDelay,omitempty"`
MaxKeys int `json:"maxkeys" yaml:"MaxKeys,omitempty"`
FieldList []string `json:"fieldlist" yaml:"FieldList,omitempty" validate:"required"`
UseTraceLength bool `json:"usetracelength" yaml:"UseTraceLength,omitempty"`
}

type TotalThroughputSamplerConfig struct {
GoalThroughputPerSec int64 `json:"goalthroughputpersec" yaml:"GoalThroughputPerSec,omitempty" validate:"gte=1"`
ClearFrequencySec int64 `json:"clearfrequencysec" yaml:"ClearFrequencySec,omitempty"`
FieldList []string `json:"fieldlist" yaml:"FieldList,omitempty" validate:"required"`
UseTraceLength bool `json:"usetracelength" yaml:"UseTraceLength,omitempty"`
AddSampleRateKeyToTrace bool `json:"addsampleratekeytotrace" yaml:"AddSampleRateKeyToTrace,omitempty"`
AddSampleRateKeyToTraceField string `json:"addsampleratekeytotracefield" yaml:"AddSampleRateKeyToTraceField,omitempty" validate:"required_with=AddSampleRateKeyToTrace"`
GoalThroughputPerSec int64 `json:"goalthroughputpersec" yaml:"GoalThroughputPerSec,omitempty" validate:"gte=1"`
ClearFrequencySec int64 `json:"clearfrequencysec" yaml:"ClearFrequencySec,omitempty"`
FieldList []string `json:"fieldlist" yaml:"FieldList,omitempty" validate:"required"`
UseTraceLength bool `json:"usetracelength" yaml:"UseTraceLength,omitempty"`
}

type RulesBasedSamplerConfig struct {
Expand Down
6 changes: 3 additions & 3 deletions sample/deterministic.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ func (d *DeterministicSampler) Start() error {
return nil
}

func (d *DeterministicSampler) GetSampleRate(trace *types.Trace) (rate uint, keep bool, reason string) {
func (d *DeterministicSampler) GetSampleRate(trace *types.Trace) (rate uint, keep bool, reason string, key string) {
if d.sampleRate <= 1 {
return 1, true, "deterministic/always"
return 1, true, "deterministic/always", ""
}
sum := sha1.Sum([]byte(trace.TraceID + shardingSalt))
v := binary.BigEndian.Uint32(sum[:4])
return uint(d.sampleRate), v <= d.upperBound, "deterministic/chance"
return uint(d.sampleRate), v <= d.upperBound, "deterministic/chance", ""
}
3 changes: 2 additions & 1 deletion sample/deterministic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,11 @@ func TestGetSampleRate(t *testing.T) {
ds.Start()

for i, tst := range tsts {
rate, keep, reason := ds.GetSampleRate(tst.trace)
rate, keep, reason, key := ds.GetSampleRate(tst.trace)
assert.Equal(t, uint(10), rate, "sample rate should be fixed")
assert.Equal(t, tst.sampled, keep, "%d: trace ID %s should be %v", i, tst.trace.TraceID, tst.sampled)
assert.Equal(t, "deterministic/chance", reason)
assert.Equal(t, "", key)
}

}
11 changes: 5 additions & 6 deletions sample/dynamic.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ type DynamicSampler struct {

sampleRate int64
clearFrequencySec int64
configName string

key *traceKey

Expand All @@ -33,7 +32,7 @@ func (d *DynamicSampler) Start() error {
d.Config.ClearFrequencySec = 30
}
d.clearFrequencySec = d.Config.ClearFrequencySec
d.key = newTraceKey(d.Config.FieldList, d.Config.UseTraceLength, d.Config.AddSampleRateKeyToTrace, d.Config.AddSampleRateKeyToTraceField)
d.key = newTraceKey(d.Config.FieldList, d.Config.UseTraceLength)

// spin up the actual dynamic sampler
d.dynsampler = &dynsampler.AvgSampleRate{
Expand All @@ -50,9 +49,9 @@ func (d *DynamicSampler) Start() error {
return nil
}

func (d *DynamicSampler) GetSampleRate(trace *types.Trace) (uint, bool, string) {
key := d.key.buildAndAdd(trace)
rate := d.dynsampler.GetSampleRate(key)
func (d *DynamicSampler) GetSampleRate(trace *types.Trace) (rate uint, keep bool, reason string, key string) {
key = d.key.build(trace)
rate = uint(d.dynsampler.GetSampleRate(key))
if rate < 1 { // protect against dynsampler being broken even though it shouldn't be
rate = 1
}
Expand All @@ -69,5 +68,5 @@ func (d *DynamicSampler) GetSampleRate(trace *types.Trace) (uint, bool, string)
d.Metrics.Increment("dynsampler_num_dropped")
}
d.Metrics.Histogram("dynsampler_sample_rate", float64(rate))
return uint(rate), shouldKeep, "dynamic/" + key
return rate, shouldKeep, "dynamic", key
}
11 changes: 5 additions & 6 deletions sample/dynamic_ema.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ type EMADynamicSampler struct {
burstMultiple float64
burstDetectionDelay uint
maxKeys int
configName string

key *traceKey

Expand All @@ -40,7 +39,7 @@ func (d *EMADynamicSampler) Start() error {
d.burstMultiple = d.Config.BurstMultiple
d.burstDetectionDelay = d.Config.BurstDetectionDelay
d.maxKeys = d.Config.MaxKeys
d.key = newTraceKey(d.Config.FieldList, d.Config.UseTraceLength, d.Config.AddSampleRateKeyToTrace, d.Config.AddSampleRateKeyToTraceField)
d.key = newTraceKey(d.Config.FieldList, d.Config.UseTraceLength)

// spin up the actual dynamic sampler
d.dynsampler = &dynsampler.EMASampleRate{
Expand All @@ -62,9 +61,9 @@ func (d *EMADynamicSampler) Start() error {
return nil
}

func (d *EMADynamicSampler) GetSampleRate(trace *types.Trace) (uint, bool, string) {
key := d.key.buildAndAdd(trace)
rate := d.dynsampler.GetSampleRate(key)
func (d *EMADynamicSampler) GetSampleRate(trace *types.Trace) (rate uint, keep bool, reason string, key string) {
key = d.key.build(trace)
rate = uint(d.dynsampler.GetSampleRate(key))
if rate < 1 { // protect against dynsampler being broken even though it shouldn't be
rate = 1
}
Expand All @@ -81,5 +80,5 @@ func (d *EMADynamicSampler) GetSampleRate(trace *types.Trace) (uint, bool, strin
d.Metrics.Increment("dynsampler_num_dropped")
}
d.Metrics.Histogram("dynsampler_sample_rate", float64(rate))
return uint(rate), shouldKeep, "emadynamic/" + key
return rate, shouldKeep, "emadynamic", key
}
19 changes: 6 additions & 13 deletions sample/dynamic_ema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ func TestDynamicEMAAddSampleRateKeyToTrace(t *testing.T) {

sampler := &EMADynamicSampler{
Config: &config.EMADynamicSamplerConfig{
FieldList: []string{"http.status_code", "request.path", "app.team.id", "important_field"},
AddSampleRateKeyToTrace: true,
AddSampleRateKeyToTraceField: "meta.key",
FieldList: []string{"http.status_code", "request.path", "app.team.id", "important_field"},
},
Logger: &logger.NullLogger{},
Metrics: &metrics,
Expand All @@ -41,18 +39,13 @@ func TestDynamicEMAAddSampleRateKeyToTrace(t *testing.T) {
})
}
sampler.Start()
sampler.GetSampleRate(trace)
rate, _, reason, key := sampler.GetSampleRate(trace)

spans := trace.GetSpans()

assert.Len(t, spans, spanCount, "should have the same number of spans as input")
for _, span := range spans {
assert.Equal(t, span.Event.Data, map[string]interface{}{
"http.status_code": 200,
"app.team.id": float64(4),
"important_field": true,
"request.path": "/{slug}/fun",
"meta.key": "4•,200•,true•,/{slug}/fun•,",
}, "should add the sampling key to all spans in the trace")
}
assert.Equal(t, uint(10), rate, "sample rate should be 10")
assert.Equal(t, "emadynamic", reason)
assert.Equal(t, "4•,200•,true•,/{slug}/fun•,", key)

}
10 changes: 1 addition & 9 deletions sample/dynamic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ func TestDynamicAddSampleRateKeyToTrace(t *testing.T) {

sampler := &DynamicSampler{
Config: &config.DynamicSamplerConfig{
FieldList: []string{"http.status_code"},
AddSampleRateKeyToTrace: true,
AddSampleRateKeyToTraceField: "meta.key",
FieldList: []string{"http.status_code"},
},
Logger: &logger.NullLogger{},
Metrics: &metrics,
Expand All @@ -42,10 +40,4 @@ func TestDynamicAddSampleRateKeyToTrace(t *testing.T) {

spans := trace.GetSpans()
assert.Len(t, spans, spanCount, "should have the same number of spans as input")
for _, span := range spans {
assert.Equal(t, span.Event.Data, map[string]interface{}{
"http.status_code": "200",
"meta.key": "200•,",
}, "should add the sampling key to all spans in the trace")
}
}
11 changes: 6 additions & 5 deletions sample/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func (s *RulesBasedSampler) Start() error {
return nil
}

func (s *RulesBasedSampler) GetSampleRate(trace *types.Trace) (rate uint, keep bool, reason string) {
func (s *RulesBasedSampler) GetSampleRate(trace *types.Trace) (rate uint, keep bool, reason string, key string) {
logger := s.Logger.Debug().WithFields(map[string]interface{}{
"trace_id": trace.TraceID,
})
Expand Down Expand Up @@ -97,6 +97,7 @@ func (s *RulesBasedSampler) GetSampleRate(trace *types.Trace) (rate uint, keep b
var rate uint
var keep bool
var samplerReason string
var key string

if rule.Sampler != nil {
var sampler Sampler
Expand All @@ -105,9 +106,9 @@ func (s *RulesBasedSampler) GetSampleRate(trace *types.Trace) (rate uint, keep b
logger.WithFields(map[string]interface{}{
"rule_name": rule.Name,
}).Logf("could not find downstream sampler for rule: %s", rule.Name)
return 1, true, reason + "bad_rule:" + rule.Name
return 1, true, reason + "bad_rule:" + rule.Name, ""
}
rate, keep, samplerReason = sampler.GetSampleRate(trace)
rate, keep, samplerReason, key = sampler.GetSampleRate(trace)
reason += rule.Name + ":" + samplerReason
} else {
rate = uint(rule.SampleRate)
Expand All @@ -126,11 +127,11 @@ func (s *RulesBasedSampler) GetSampleRate(trace *types.Trace) (rate uint, keep b
"keep": keep,
"drop_rule": rule.Drop,
}).Logf("got sample rate and decision")
return rate, keep, reason
return rate, keep, reason, key
}
}

return 1, true, "no rule matched"
return 1, true, "no rule matched", ""
}

func ruleMatchesTrace(t *types.Trace, rule *config.RulesBasedSamplerRule, checkNestedFields bool) bool {
Expand Down
Loading