diff --git a/collect/collect.go b/collect/collect.go index c20171be3a..eb178d836f 100644 --- a/collect/collect.go +++ b/collect/collect.go @@ -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) @@ -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) diff --git a/collect/collect_test.go b/collect/collect_test.go index 7aaadbd3b7..1d6dd401a1 100644 --- a/collect/collect_test.go +++ b/collect/collect_test.go @@ -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) diff --git a/config/config_test.go b/config/config_test.go index dece4a322b..44c23a2636 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -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" diff --git a/config/sampler_config.go b/config/sampler_config.go index 9207351f76..7ade158e60 100644 --- a/config/sampler_config.go +++ b/config/sampler_config.go @@ -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 { diff --git a/sample/deterministic.go b/sample/deterministic.go index fce06adebe..19c62f866e 100644 --- a/sample/deterministic.go +++ b/sample/deterministic.go @@ -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", "" } diff --git a/sample/deterministic_test.go b/sample/deterministic_test.go index cd9e129ace..3ec6afcf01 100644 --- a/sample/deterministic_test.go +++ b/sample/deterministic_test.go @@ -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) } } diff --git a/sample/dynamic.go b/sample/dynamic.go index 356ce06c9f..0652c86fd1 100644 --- a/sample/dynamic.go +++ b/sample/dynamic.go @@ -18,7 +18,6 @@ type DynamicSampler struct { sampleRate int64 clearFrequencySec int64 - configName string key *traceKey @@ -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{ @@ -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 } @@ -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 } diff --git a/sample/dynamic_ema.go b/sample/dynamic_ema.go index 34a88b4820..0db948f33b 100644 --- a/sample/dynamic_ema.go +++ b/sample/dynamic_ema.go @@ -23,7 +23,6 @@ type EMADynamicSampler struct { burstMultiple float64 burstDetectionDelay uint maxKeys int - configName string key *traceKey @@ -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{ @@ -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 } @@ -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 } diff --git a/sample/dynamic_ema_test.go b/sample/dynamic_ema_test.go index 6b36e56a96..d344071f08 100644 --- a/sample/dynamic_ema_test.go +++ b/sample/dynamic_ema_test.go @@ -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, @@ -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) + } diff --git a/sample/dynamic_test.go b/sample/dynamic_test.go index e2405f4c75..94823044f7 100644 --- a/sample/dynamic_test.go +++ b/sample/dynamic_test.go @@ -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, @@ -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") - } } diff --git a/sample/rules.go b/sample/rules.go index 52556d3725..67463e8f98 100644 --- a/sample/rules.go +++ b/sample/rules.go @@ -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, }) @@ -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 @@ -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) @@ -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 { diff --git a/sample/rules_test.go b/sample/rules_test.go index a6c8d3ea61..32dd79553f 100644 --- a/sample/rules_test.go +++ b/sample/rules_test.go @@ -606,7 +606,7 @@ func TestRules(t *testing.T) { trace.AddSpan(span) } - rate, keep, reason := sampler.GetSampleRate(trace) + rate, keep, reason, key := sampler.GetSampleRate(trace) assert.Equal(t, d.ExpectedRate, rate, d.Rules) name := d.ExpectedName @@ -614,6 +614,7 @@ func TestRules(t *testing.T) { name = d.Rules.Rules[0].Name } assert.Contains(t, reason, name) + assert.Equal(t, "", key) // we can only test when we don't expect to keep the trace if !d.ExpectedKeep { @@ -760,7 +761,7 @@ func TestRulesWithNestedFields(t *testing.T) { trace.AddSpan(span) } - rate, keep, reason := sampler.GetSampleRate(trace) + rate, keep, reason, key := sampler.GetSampleRate(trace) assert.Equal(t, d.ExpectedRate, rate, d.Rules) name := d.ExpectedName @@ -768,6 +769,7 @@ func TestRulesWithNestedFields(t *testing.T) { name = d.Rules.Rules[0].Name } assert.Contains(t, reason, name) + assert.Equal(t, "", key) // we can only test when we don't expect to keep the trace if !d.ExpectedKeep { @@ -792,10 +794,8 @@ func TestRulesWithDynamicSampler(t *testing.T) { }, Sampler: &config.RulesBasedDownstreamSampler{ DynamicSampler: &config.DynamicSamplerConfig{ - SampleRate: 10, - FieldList: []string{"http.status_code"}, - AddSampleRateKeyToTrace: true, - AddSampleRateKeyToTraceField: "meta.key", + SampleRate: 10, + FieldList: []string{"http.status_code"}, }, }, }, @@ -838,7 +838,7 @@ func TestRulesWithDynamicSampler(t *testing.T) { } sampler.Start() - rate, keep, reason := sampler.GetSampleRate(trace) + rate, keep, reason, key := sampler.GetSampleRate(trace) assert.Equal(t, d.ExpectedRate, rate, d.Rules) name := d.ExpectedName @@ -846,6 +846,7 @@ func TestRulesWithDynamicSampler(t *testing.T) { name = d.Rules.Rules[0].Name } assert.Contains(t, reason, name) + assert.Equal(t, "200•,", key) // we can only test when we don't expect to keep the trace if !d.ExpectedKeep { @@ -854,13 +855,6 @@ func TestRulesWithDynamicSampler(t *testing.T) { spans := trace.GetSpans() assert.Len(t, spans, len(d.Spans), "should have the same number of spans as input") - for _, span := range spans { - assert.Equal(t, span.Event.Data, map[string]interface{}{ - "rule_test": int64(1), - "http.status_code": "200", - "meta.key": "200•,", - }, "should add the sampling key to all spans in the trace") - } } } @@ -880,10 +874,8 @@ func TestRulesWithEMADynamicSampler(t *testing.T) { }, Sampler: &config.RulesBasedDownstreamSampler{ EMADynamicSampler: &config.EMADynamicSamplerConfig{ - GoalSampleRate: 10, - FieldList: []string{"http.status_code"}, - AddSampleRateKeyToTrace: true, - AddSampleRateKeyToTraceField: "meta.key", + GoalSampleRate: 10, + FieldList: []string{"http.status_code"}, }, }, }, @@ -926,7 +918,7 @@ func TestRulesWithEMADynamicSampler(t *testing.T) { } sampler.Start() - rate, keep, reason := sampler.GetSampleRate(trace) + rate, keep, reason, key := sampler.GetSampleRate(trace) assert.Equal(t, d.ExpectedRate, rate, d.Rules) name := d.ExpectedName @@ -934,6 +926,7 @@ func TestRulesWithEMADynamicSampler(t *testing.T) { name = d.Rules.Rules[0].Name } assert.Contains(t, reason, name) + assert.Equal(t, "200•,", key) // we can only test when we don't expect to keep the trace if !d.ExpectedKeep { @@ -942,13 +935,6 @@ func TestRulesWithEMADynamicSampler(t *testing.T) { spans := trace.GetSpans() assert.Len(t, spans, len(d.Spans), "should have the same number of spans as input") - for _, span := range spans { - assert.Equal(t, span.Event.Data, map[string]interface{}{ - "rule_test": int64(1), - "http.status_code": "200", - "meta.key": "200•,", - }, "should add the sampling key to all spans in the trace") - } } } @@ -1048,7 +1034,7 @@ func TestRuleMatchesSpanMatchingSpan(t *testing.T) { } sampler.Start() - rate, keep, _ := sampler.GetSampleRate(trace) + rate, keep, _, _ := sampler.GetSampleRate(trace) assert.Equal(t, uint(1), rate, rate) if scope == "span" { @@ -1574,7 +1560,7 @@ func TestRulesDatatypes(t *testing.T) { trace.AddSpan(span) } - rate, keep, _ := sampler.GetSampleRate(trace) + rate, keep, _, _ := sampler.GetSampleRate(trace) assert.Equal(t, d.ExpectedRate, rate, d.Rules) // because keep depends on sampling rate, we can only test expectedKeep when it should be false if !d.ExpectedKeep { diff --git a/sample/sample.go b/sample/sample.go index 697482bd91..11a26a24cc 100644 --- a/sample/sample.go +++ b/sample/sample.go @@ -11,7 +11,7 @@ import ( ) type Sampler interface { - GetSampleRate(trace *types.Trace) (rate uint, keep bool, reason string) + GetSampleRate(trace *types.Trace) (rate uint, keep bool, reason string, key string) Start() error } diff --git a/sample/totalthroughput.go b/sample/totalthroughput.go index 53ac306412..674535adae 100644 --- a/sample/totalthroughput.go +++ b/sample/totalthroughput.go @@ -36,7 +36,7 @@ func (d *TotalThroughputSampler) 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.TotalThroughput{ @@ -53,9 +53,9 @@ func (d *TotalThroughputSampler) Start() error { return nil } -func (d *TotalThroughputSampler) GetSampleRate(trace *types.Trace) (uint, bool, string) { - key := d.key.buildAndAdd(trace) - rate := d.dynsampler.GetSampleRate(key) +func (d *TotalThroughputSampler) 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 } @@ -72,5 +72,5 @@ func (d *TotalThroughputSampler) GetSampleRate(trace *types.Trace) (uint, bool, d.Metrics.Increment("dynsampler_num_dropped") } d.Metrics.Histogram("dynsampler_sample_rate", float64(rate)) - return uint(rate), shouldKeep, "totalthroughput/" + key + return rate, shouldKeep, "totalthroughput", key } diff --git a/sample/totalthroughput_test.go b/sample/totalthroughput_test.go index 2d885bb799..c8990bc597 100644 --- a/sample/totalthroughput_test.go +++ b/sample/totalthroughput_test.go @@ -19,9 +19,7 @@ func TestTotalThroughputAddSampleRateKeyToTrace(t *testing.T) { sampler := &TotalThroughputSampler{ Config: &config.TotalThroughputSamplerConfig{ - FieldList: []string{"http.status_code"}, - AddSampleRateKeyToTrace: true, - AddSampleRateKeyToTraceField: "meta.key", + FieldList: []string{"http.status_code"}, }, Logger: &logger.NullLogger{}, Metrics: &metrics, @@ -42,10 +40,4 @@ func TestTotalThroughputAddSampleRateKeyToTrace(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") - } } diff --git a/sample/trace_key.go b/sample/trace_key.go index 80d0c6f1e5..da65fefa3f 100644 --- a/sample/trace_key.go +++ b/sample/trace_key.go @@ -9,37 +9,19 @@ import ( ) type traceKey struct { - fields []string - useTraceLength bool - addDynsampleKey bool - addDynsampleField string + fields []string + useTraceLength bool } -func newTraceKey(fields []string, useTraceLength, addDynsampleKey bool, addDynsampleField string) *traceKey { +func newTraceKey(fields []string, useTraceLength bool) *traceKey { // always put the field list in sorted order for easier comparison sort.Strings(fields) return &traceKey{ - fields: fields, - useTraceLength: useTraceLength, - addDynsampleKey: addDynsampleKey, - addDynsampleField: addDynsampleField, + fields: fields, + useTraceLength: useTraceLength, } } -// buildAndAdd, builds the trace key and adds it to the trace if configured to -// do so -func (d *traceKey) buildAndAdd(trace *types.Trace) string { - key := d.build(trace) - - if d.addDynsampleKey { - for _, span := range trace.GetSpans() { - span.Data[d.addDynsampleField] = key - } - } - - return key -} - // build, builds the trace key based on the configuration of the traceKeyGenerator func (d *traceKey) build(trace *types.Trace) string { // fieldCollector gets all values from the fields listed in the config, even diff --git a/sample/trace_key_test.go b/sample/trace_key_test.go index c6ce600004..e7a3bfec65 100644 --- a/sample/trace_key_test.go +++ b/sample/trace_key_test.go @@ -9,11 +9,9 @@ import ( func TestKeyGeneration(t *testing.T) { fields := []string{"http.status_code", "request.path", "app.team.id", "important_field"} - addKey := true - key := "meta.key" useTraceLength := true - generator := newTraceKey(fields, useTraceLength, addKey, key) + generator := newTraceKey(fields, useTraceLength) trace := &types.Trace{} @@ -33,11 +31,9 @@ func TestKeyGeneration(t *testing.T) { assert.Equal(t, expected, generator.build(trace)) fields = []string{"http.status_code", "request.path", "app.team.id", "important_field"} - addKey = true - key = "meta.key" useTraceLength = true - generator = newTraceKey(fields, useTraceLength, addKey, key) + generator = newTraceKey(fields, useTraceLength) trace = &types.Trace{} @@ -79,11 +75,9 @@ func TestKeyGeneration(t *testing.T) { // now test that multiple values across spans are condensed correctly fields = []string{"http.status_code"} - addKey = true - key = "meta.key" useTraceLength = true - generator = newTraceKey(fields, useTraceLength, addKey, key) + generator = newTraceKey(fields, useTraceLength) trace = &types.Trace{} @@ -125,11 +119,9 @@ func TestKeyGeneration(t *testing.T) { // now test that multiple values across spans in a different order are condensed the same fields = []string{"http.status_code"} - addKey = true - key = "meta.key" useTraceLength = true - generator = newTraceKey(fields, useTraceLength, addKey, key) + generator = newTraceKey(fields, useTraceLength) trace = &types.Trace{}