From c53e5f872c26c9d701d4d50577f090bb9508812e Mon Sep 17 00:00:00 2001 From: Yingrong Zhao <22300958+VinozzZ@users.noreply.github.com> Date: Wed, 14 Aug 2024 18:40:13 -0400 Subject: [PATCH 1/2] feat: allow root. in field list for dynamic sampler --- sample/dynamic_test.go | 20 ++++++++++++++++++-- sample/trace_key.go | 37 +++++++++++++++++++++++++++++++++++-- sample/trace_key_test.go | 22 ++++++++-------------- 3 files changed, 61 insertions(+), 18 deletions(-) diff --git a/sample/dynamic_test.go b/sample/dynamic_test.go index 94823044f7..358d8d6f0e 100644 --- a/sample/dynamic_test.go +++ b/sample/dynamic_test.go @@ -19,7 +19,8 @@ func TestDynamicAddSampleRateKeyToTrace(t *testing.T) { sampler := &DynamicSampler{ Config: &config.DynamicSamplerConfig{ - FieldList: []string{"http.status_code"}, + FieldList: []string{"http.status_code", "root.service_name", "root.url"}, + SampleRate: 1, }, Logger: &logger.NullLogger{}, Metrics: &metrics, @@ -27,17 +28,32 @@ func TestDynamicAddSampleRateKeyToTrace(t *testing.T) { trace := &types.Trace{} for i := 0; i < spanCount; i++ { + if i == spanCount-1 { + trace.RootSpan = &types.Span{ + Event: types.Event{ + Data: map[string]interface{}{ + "http.status_code": "200", + "service_name": "test", + }, + }, + } + } trace.AddSpan(&types.Span{ Event: types.Event{ Data: map[string]interface{}{ "http.status_code": "200", + "url": "/test", }, }, }) } sampler.Start() - sampler.GetSampleRate(trace) + rate, keep, reason, key := sampler.GetSampleRate(trace) spans := trace.GetSpans() assert.Len(t, spans, spanCount, "should have the same number of spans as input") + assert.Equal(t, uint(1), rate) + assert.True(t, keep) + assert.Equal(t, "dynamic", reason) + assert.Equal(t, "200•,test,", key) } diff --git a/sample/trace_key.go b/sample/trace_key.go index da65fefa3f..205a4c8605 100644 --- a/sample/trace_key.go +++ b/sample/trace_key.go @@ -4,20 +4,34 @@ import ( "fmt" "sort" "strconv" + "strings" "github.com/honeycombio/refinery/types" ) type traceKey struct { fields []string + rootOnlyFields []string useTraceLength bool } func newTraceKey(fields []string, useTraceLength bool) *traceKey { // always put the field list in sorted order for easier comparison sort.Strings(fields) + rootOnlyFields := make([]string, 0, len(fields)/2) + nonRootFields := make([]string, 0, len(fields)/2) + for _, field := range fields { + if strings.HasPrefix(field, RootPrefix) { + rootOnlyFields = append(rootOnlyFields, field[len(RootPrefix):]) + continue + } + + nonRootFields = append(nonRootFields, field) + } + return &traceKey{ - fields: fields, + fields: nonRootFields, + rootOnlyFields: rootOnlyFields, useTraceLength: useTraceLength, } } @@ -26,7 +40,18 @@ func newTraceKey(fields []string, useTraceLength bool) *traceKey { func (d *traceKey) build(trace *types.Trace) string { // fieldCollector gets all values from the fields listed in the config, even // if they happen multiple times. - fieldCollector := map[string][]string{} + fieldCollector := make(map[string][]string) + rootFieldCollector := make(map[string]string) + + for _, field := range d.rootOnlyFields { + if trace.RootSpan == nil { + break + } + + if val, ok := trace.RootSpan.Data[field]; ok { + rootFieldCollector[field] = fmt.Sprintf("%v", val) + } + } // for each field, for each span, get the value of that field spans := trace.GetSpans() @@ -54,6 +79,14 @@ func (d *traceKey) build(trace *types.Trace) string { key += "," } + for _, field := range d.rootOnlyFields { + if rootFieldCollector[field] == "" { + continue + } + + key += rootFieldCollector[field] + "," + } + if d.useTraceLength { key += strconv.FormatInt(int64(len(spans)), 10) } diff --git a/sample/trace_key_test.go b/sample/trace_key_test.go index e7a3bfec65..b9e29ba25c 100644 --- a/sample/trace_key_test.go +++ b/sample/trace_key_test.go @@ -117,8 +117,9 @@ func TestKeyGeneration(t *testing.T) { assert.Equal(t, expected, generator.build(trace)) - // now test that multiple values across spans in a different order are condensed the same - fields = []string{"http.status_code"} + // test field list with root prefix, only include the field from on the root span + // if it exists + fields = []string{"http.status_code", "root.service_name", "root.another_field"} useTraceLength = true generator = newTraceKey(fields, useTraceLength) @@ -133,31 +134,24 @@ func TestKeyGeneration(t *testing.T) { }, }) - trace.AddSpan(&types.Span{ - Event: types.Event{ - Data: map[string]interface{}{ - "http.status_code": 404, - }, - }, - }) - trace.AddSpan(&types.Span{ Event: types.Event{ Data: map[string]interface{}{ "http.status_code": 200, + "service_name": "another", }, }, }) - trace.AddSpan(&types.Span{ + trace.RootSpan = &types.Span{ Event: types.Event{ Data: map[string]interface{}{ - "http.status_code": 200, + "service_name": "test", }, }, - }) + } - expected = "200•404•,4" + expected = "200•404•,test,2" assert.Equal(t, expected, generator.build(trace)) } From 7e76646f28f334cc462705a7096e67dd80d16a6d Mon Sep 17 00:00:00 2001 From: Yingrong Zhao <22300958+VinozzZ@users.noreply.github.com> Date: Thu, 15 Aug 2024 10:31:16 -0400 Subject: [PATCH 2/2] simplify code --- sample/trace_key.go | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/sample/trace_key.go b/sample/trace_key.go index 205a4c8605..7683397dd3 100644 --- a/sample/trace_key.go +++ b/sample/trace_key.go @@ -41,17 +41,6 @@ func (d *traceKey) build(trace *types.Trace) string { // fieldCollector gets all values from the fields listed in the config, even // if they happen multiple times. fieldCollector := make(map[string][]string) - rootFieldCollector := make(map[string]string) - - for _, field := range d.rootOnlyFields { - if trace.RootSpan == nil { - break - } - - if val, ok := trace.RootSpan.Data[field]; ok { - rootFieldCollector[field] = fmt.Sprintf("%v", val) - } - } // for each field, for each span, get the value of that field spans := trace.GetSpans() @@ -79,12 +68,13 @@ func (d *traceKey) build(trace *types.Trace) string { key += "," } - for _, field := range d.rootOnlyFields { - if rootFieldCollector[field] == "" { - continue - } + if trace.RootSpan != nil { + for _, field := range d.rootOnlyFields { - key += rootFieldCollector[field] + "," + if val, ok := trace.RootSpan.Data[field]; ok { + key += fmt.Sprintf("%v,", val) + } + } } if d.useTraceLength {