Skip to content

Commit

Permalink
feat: add IN operator (#1302)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?

- Fixes #1074 
- Adds `In` and `NotIn` operators to Refinery rules, and allows the
Value field to become a list of values.

## Short description of the changes

- Change validation so that we can specify multiple Values
- Implement In and NotIn operators
- Write tests
- Document
  • Loading branch information
kentquirk authored Aug 30, 2024
1 parent 8317a10 commit 0c38e22
Show file tree
Hide file tree
Showing 7 changed files with 238 additions and 74 deletions.
8 changes: 4 additions & 4 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ func TestReadRulesConfig(t *testing.T) {
d, name = c.GetSamplerConfigForDestName("env4")
switch r := d.(type) {
case *config.RulesBasedSamplerConfig:
assert.Len(t, r.Rules, 6)
assert.Len(t, r.Rules, 7)

var rule *config.RulesBasedSamplerRule

Expand All @@ -360,16 +360,16 @@ func TestReadRulesConfig(t *testing.T) {
assert.Equal(t, 0, rule.SampleRate)
assert.Len(t, rule.Conditions, 1)

rule = r.Rules[1]
rule = r.Rules[2]
assert.Equal(t, 1, rule.SampleRate)
assert.Equal(t, "keep slow 500 errors across semantic conventions", rule.Name)
assert.Len(t, rule.Conditions, 2)

rule = r.Rules[3]
rule = r.Rules[4]
assert.Equal(t, 5, rule.SampleRate)
assert.Equal(t, "span", rule.Scope)

rule = r.Rules[5]
rule = r.Rules[6]
assert.Equal(t, 10, rule.SampleRate)
assert.Equal(t, "", rule.Scope)

Expand Down
15 changes: 11 additions & 4 deletions config/metadata/rulesMeta.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -669,22 +669,25 @@ groups:
- not-exists
- has-root-span
- matches
- in
- not-in
summary: is the comparison operator to use.
description: >
The comparison operator to use. String comparisons are case-sensitive.
For most cases, use negative operators (`!=`, `does-not-contain`, and
`not-exists`) in a rule with a scope of "span".
For most cases, use negative operators (`!=`, `does-not-contain`,
`not-exists`, and `not-in`) in a rule with a scope of "span".
WARNING: Rules can have `Scope: trace` or `Scope: span`. Using a negative
operator with `Scope: trace` will cause the condition be true if **any**
single span in the entire trace matches. Use `Scope: span` with negative
operators.
- name: Value
type: anyscalar
type: sliceorscalar
summary: is the value to compare against.
description: >
The value to compare against. If `Datatype` is not specified, then
the value and the field will be compared based on the type of the
field.
field. The `in` and `not-in` operators can accept a list of values,
which should all be of the same datatype.
- name: Datatype
type: string
validations:
Expand All @@ -703,3 +706,7 @@ groups:
especially useful when a field like `http status code` may be
rendered as strings by some environments and as numbers or booleans
by others.
The best practice is to always specify `Datatype`; this avoids
ambiguity, allows for more accurate comparisons, and offers a
minor performance improvement.
158 changes: 96 additions & 62 deletions config/sampler_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"regexp"
"strconv"
"strings"

"github.com/honeycombio/refinery/generics"
)

// Define some constants for rule comparison operators
Expand Down Expand Up @@ -287,6 +289,11 @@ func (r *RulesBasedSamplerCondition) setMatchesFunction() error {
if err != nil {
return err
}
case In, NotIn:
err := setInBasedOperators(r, r.Operator)
if err != nil {
return err
}
case MatchesRegexp:
err := setRegexStringMatchOperator(r)
if err != nil {
Expand Down Expand Up @@ -344,16 +351,13 @@ func tryConvertToFloat(v any) (float64, bool) {
// "standard" format, which we are defining as whatever Go does with the %v
// operator to sprintf. This will make sure that no matter how people encode
// their values, they compare on an equal footing.
func tryConvertToString(v any) (string, bool) {
return fmt.Sprintf("%v", v), true
// This function can never fail, so it's not named "tryConvert" like the others.
func convertToString(v any) string {
return fmt.Sprintf("%v", v)
}

func TryConvertToBool(v any) bool {
value, ok := tryConvertToString(v)
if !ok {
return false
}
str, err := strconv.ParseBool(value)
str, err := strconv.ParseBool(convertToString(v))
if err != nil {
return false
}
Expand All @@ -367,58 +371,37 @@ func TryConvertToBool(v any) bool {
func setCompareOperators(r *RulesBasedSamplerCondition, condition string) error {
switch r.Datatype {
case "string":
conditionValue, ok := tryConvertToString(r.Value)
if !ok {
return fmt.Errorf("could not convert %v to string", r.Value)
}
conditionValue := convertToString(r.Value)

switch condition {
case NEQ:
r.Matches = func(spanValue any, exists bool) bool {
if n, ok := tryConvertToString(spanValue); exists && ok {
return n != conditionValue
}
return false
return convertToString(spanValue) != conditionValue
}
return nil
case EQ:
r.Matches = func(spanValue any, exists bool) bool {
if n, ok := tryConvertToString(spanValue); exists && ok {
return n == conditionValue
}
return false
return convertToString(spanValue) == conditionValue
}
return nil
case GT:
r.Matches = func(spanValue any, exists bool) bool {
if n, ok := tryConvertToString(spanValue); exists && ok {
return n > conditionValue
}
return false
return convertToString(spanValue) > conditionValue
}
return nil
case GTE:
r.Matches = func(spanValue any, exists bool) bool {
if n, ok := tryConvertToString(spanValue); exists && ok {
return n >= conditionValue
}
return false
return convertToString(spanValue) >= conditionValue
}
return nil
case LT:
r.Matches = func(spanValue any, exists bool) bool {
if n, ok := tryConvertToString(spanValue); exists && ok {
return n < conditionValue
}
return false
return convertToString(spanValue) < conditionValue
}
return nil
case LTE:
r.Matches = func(spanValue any, exists bool) bool {
if n, ok := tryConvertToString(spanValue); exists && ok {
return n <= conditionValue
}
return false
return convertToString(spanValue) <= conditionValue
}
return nil
}
Expand Down Expand Up @@ -564,58 +547,109 @@ func setCompareOperators(r *RulesBasedSamplerCondition, condition string) error
}

func setMatchStringBasedOperators(r *RulesBasedSamplerCondition, condition string) error {
conditionValue, ok := tryConvertToString(r.Value)
if !ok {
return fmt.Errorf("%s value must be a string, but was '%s'", condition, r.Value)
}
conditionValue := convertToString(r.Value)

switch condition {
case StartsWith:
r.Matches = func(spanValue any, exists bool) bool {
s, ok := tryConvertToString(spanValue)
if ok {
return strings.HasPrefix(s, conditionValue)
}
return false
return strings.HasPrefix(convertToString(spanValue), conditionValue)
}
case Contains:
r.Matches = func(spanValue any, exists bool) bool {
s, ok := tryConvertToString(spanValue)
if ok {
return strings.Contains(s, conditionValue)
}
return false
return strings.Contains(convertToString(spanValue), conditionValue)
}
case DoesNotContain:
r.Matches = func(spanValue any, exists bool) bool {
s, ok := tryConvertToString(spanValue)
if ok {
return !strings.Contains(s, conditionValue)
return !strings.Contains(convertToString(spanValue), conditionValue)
}
}

return nil
}

func setInBasedOperators(r *RulesBasedSamplerCondition, condition string) error {
var matches func(spanValue any, exists bool) bool

// we'll support having r.Value be either a single scalar or a list of scalars
// so to avoid having to check the type of r.Value every time, we'll just convert
// it to a list of scalars and then check the type of each scalar as we iterate
var value []any
switch v := r.Value.(type) {
case []any:
value = v
case string, int, float64:
value = []any{v}
default:
return fmt.Errorf("value must be a list of scalars")
}

switch r.Datatype {
// if datatype is not specified, we'll always convert the values to strings
case "string", "":
values := generics.NewSet[string]()
for _, v := range value {
value := convertToString(v)
values.Add(value)
}
matches = func(spanValue any, exists bool) bool {
s := convertToString(spanValue)
return values.Contains(s)
}
case "int":
values := generics.NewSet[int]()
for _, v := range value {
value, ok := tryConvertToInt(v)
if !ok {
// validation should have caught this, so we'll just skip it
continue
}
return false
values.Add(value)
}
matches = func(spanValue any, exists bool) bool {
i, ok := tryConvertToInt(spanValue)
return ok && values.Contains(i)
}
case "float":
values := generics.NewSet[float64]()
for _, v := range value {
value, ok := tryConvertToFloat(v)
if !ok {
// validation should have caught this, so we'll just skip it
continue
}
values.Add(value)
}
matches = func(spanValue any, exists bool) bool {
f, ok := tryConvertToFloat(spanValue)
return ok && values.Contains(f)
}
case "bool":
return fmt.Errorf("cannot use %s operator with boolean datatype", condition)
}

switch condition {
case In:
r.Matches = matches
case NotIn:
r.Matches = func(spanValue any, exists bool) bool {
return !matches(spanValue, exists)
}
}

return nil
}

func setRegexStringMatchOperator(r *RulesBasedSamplerCondition) error {
conditionValue, ok := tryConvertToString(r.Value)
if !ok {
return fmt.Errorf("regex value must be a string, but was '%s'", r.Value)
}
conditionValue := convertToString(r.Value)

regex, err := regexp.Compile(conditionValue)
if err != nil {
return fmt.Errorf("'matches' pattern must be a valid Go regexp, but was '%s'", r.Value)
}

r.Matches = func(spanValue any, exists bool) bool {
s, ok := tryConvertToString(spanValue)
if ok {
return regex.MatchString(s)
}
return false
s := convertToString(spanValue)
return regex.MatchString(s)
}

return nil
Expand Down
Loading

0 comments on commit 0c38e22

Please sign in to comment.