From bd057b8a9e16fc8981c68ef4fd6be6dce0104b0c Mon Sep 17 00:00:00 2001 From: Yingrong Zhao <22300958+VinozzZ@users.noreply.github.com> Date: Thu, 14 Dec 2023 14:01:43 -0500 Subject: [PATCH 1/2] feat: use a computed field for current descendant count in rules --- config/sampler_config.go | 15 +++++ sample/rules.go | 14 ++++- sample/rules_test.go | 120 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 146 insertions(+), 3 deletions(-) diff --git a/config/sampler_config.go b/config/sampler_config.go index 13ed606487..bc722c7283 100644 --- a/config/sampler_config.go +++ b/config/sampler_config.go @@ -30,6 +30,13 @@ const ( NotIn = "not-in" ) +const ( + ComputedFieldPrefix = "?." + NUM_DESCENDANTS ComputedField = ComputedFieldPrefix + "NUM_DESCENDANTS" +) + +type ComputedField string + // The json tags in this file are used for conversion from the old format (see tools/convert for details). // They are deliberately all lowercase. // The yaml tags are used for the new format and are PascalCase. @@ -250,6 +257,14 @@ func (r *RulesBasedSamplerCondition) String() string { return fmt.Sprintf("%+v", *r) } +func (r *RulesBasedSamplerCondition) GetComputedField() (ComputedField, bool) { + if strings.HasPrefix(r.Field, ComputedFieldPrefix) { + return ComputedField(r.Field), true + } + return "", false + +} + func (r *RulesBasedSamplerCondition) setMatchesFunction() error { switch r.Operator { case Exists: diff --git a/sample/rules.go b/sample/rules.go index 1f9bb9b09f..c1d9930d96 100644 --- a/sample/rules.go +++ b/sample/rules.go @@ -167,7 +167,7 @@ func ruleMatchesTrace(t *types.Trace, rule *config.RulesBasedSamplerRule, checkN span: for _, span := range t.GetSpans() { - value, exists := extractValueFromSpan(span, condition, checkNestedFields) + value, exists := extractValueFromSpan(t, span, condition, checkNestedFields) if condition.Matches == nil { if conditionMatchesValue(condition, value, exists) { matched++ @@ -194,7 +194,7 @@ func ruleMatchesSpanInTrace(trace *types.Trace, rule *config.RulesBasedSamplerRu ruleMatched := true for _, condition := range rule.Conditions { // whether this condition is matched by this span. - value, exists := extractValueFromSpan(span, condition, checkNestedFields) + value, exists := extractValueFromSpan(trace, span, condition, checkNestedFields) if condition.Matches == nil { if !conditionMatchesValue(condition, value, exists) { ruleMatched = false @@ -220,7 +220,15 @@ func ruleMatchesSpanInTrace(trace *types.Trace, rule *config.RulesBasedSamplerRu return false } -func extractValueFromSpan(span *types.Span, condition *config.RulesBasedSamplerCondition, checkNestedFields bool) (interface{}, bool) { +func extractValueFromSpan(trace *types.Trace, span *types.Span, condition *config.RulesBasedSamplerCondition, checkNestedFields bool) (interface{}, bool) { + // If the condition is a descendant count, we extract the count from trace and return it. + if f, ok := condition.GetComputedField(); ok { + switch f { + case config.NUM_DESCENDANTS: + return int64(trace.DescendantCount()), true + } + } + // whether this condition is matched by this span. var value any var exists bool diff --git a/sample/rules_test.go b/sample/rules_test.go index 557ae5cdaa..2493fceffe 100644 --- a/sample/rules_test.go +++ b/sample/rules_test.go @@ -772,6 +772,126 @@ func TestRules(t *testing.T) { ExpectedName: "no rule matched", ExpectedRate: 1, }, + { + Rules: &config.RulesBasedSamplerConfig{ + Rules: []*config.RulesBasedSamplerRule{ + { + Name: "Check that the number of descendants is greater than 3", + SampleRate: 1, + Conditions: []*config.RulesBasedSamplerCondition{ + { + Field: string(config.NUM_DESCENDANTS), + Operator: config.GT, + Value: int(3), + Datatype: "int", + }, + }, + Drop: true, + }, + }, + }, + Spans: []*types.Span{ + { + Event: types.Event{ + Data: map[string]interface{}{ + "trace.trace_id": "12345", + "trace.span_id": "54322", + "trace.parent_id": "54321", + "meta.span_count": int64(2), + }, + }, + }, + { + Event: types.Event{ + Data: map[string]interface{}{ + "trace.trace_id": "12345", + "trace.span_id": "654321", + "trace.parent_id": "54322", + }, + }, + }, + { + Event: types.Event{ + Data: map[string]interface{}{ + "trace.trace_id": "12345", + "trace.span_id": "754321", + "trace.parent_id": "54322", + }, + }, + }, + { + Event: types.Event{ + Data: map[string]interface{}{ + "trace.trace_id": "12345", + "trace.span_id": "754321", + "trace.parent_id": "54322", + }, + }, + }, + }, + ExpectedName: "Check that the number of descendants is greater than 3", + ExpectedKeep: false, + ExpectedRate: 1, + }, + { + Rules: &config.RulesBasedSamplerConfig{ + Rules: []*config.RulesBasedSamplerRule{ + { + Name: "Check that the number of descendants is less than 3", + SampleRate: 1, + Conditions: []*config.RulesBasedSamplerCondition{ + { + Field: string(config.NUM_DESCENDANTS), + Operator: config.LT, + Value: int(3), + }, + }, + }, + }, + }, + Spans: []*types.Span{ + { + Event: types.Event{ + Data: map[string]interface{}{ + "trace.trace_id": "12345", + "trace.span_id": "54322", + "trace.parent_id": "54321", + "meta.span_count": int64(2), + }, + }, + }, + { + Event: types.Event{ + Data: map[string]interface{}{ + "trace.trace_id": "12345", + "trace.span_id": "654321", + "trace.parent_id": "54322", + }, + }, + }, + { + Event: types.Event{ + Data: map[string]interface{}{ + "trace.trace_id": "12345", + "trace.span_id": "754321", + "trace.parent_id": "54322", + }, + }, + }, + { + Event: types.Event{ + Data: map[string]interface{}{ + "trace.trace_id": "12345", + "trace.span_id": "754321", + "trace.parent_id": "54322", + }, + }, + }, + }, + ExpectedName: "no rule matched", + ExpectedKeep: true, + ExpectedRate: 1, + }, } for _, d := range data { From d664e5210a289f708d52bf89668bb0e39c4fcd65 Mon Sep 17 00:00:00 2001 From: Yingrong Zhao <22300958+VinozzZ@users.noreply.github.com> Date: Thu, 14 Dec 2023 16:42:10 -0500 Subject: [PATCH 2/2] add comments and place type definition before its usage --- config/sampler_config.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/config/sampler_config.go b/config/sampler_config.go index bc722c7283..3826a32d63 100644 --- a/config/sampler_config.go +++ b/config/sampler_config.go @@ -30,13 +30,16 @@ const ( NotIn = "not-in" ) +// ComputedField is a virtual field. It's value is calculated during rule evaluation. +// We use the `?.` prefix to distinguish computed fields from regular fields. +type ComputedField string + const ( + // ComputedFieldPrefix is the prefix for computed fields. ComputedFieldPrefix = "?." NUM_DESCENDANTS ComputedField = ComputedFieldPrefix + "NUM_DESCENDANTS" ) -type ComputedField string - // The json tags in this file are used for conversion from the old format (see tools/convert for details). // They are deliberately all lowercase. // The yaml tags are used for the new format and are PascalCase.