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.

Sign up for GitHub

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

Add parsing for nested json fields in the rules sampler #418

Merged
merged 6 commits into from
Mar 29, 2022
Merged

Add parsing for nested json fields in the rules sampler #418

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f239586
added username in config for redis auth
Feb 9, 2022
7376603
Merge branch 'honeycombio:main' into main
ecobrien29 Mar 2, 2022
554d932
rules sampler checks nested fields
Mar 3, 2022
9dc0283
fixed err check
Mar 4, 2022
8be1efd
added test for nested fields
Mar 7, 2022
962c29d
add config to check nested fields
Mar 14, 2022
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
3 changes: 2 additions & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ require (
github.com/gorilla/mux v1.8.0
github.com/hashicorp/golang-lru v0.5.4
github.com/honeycombio/dynsampler-go v0.2.1
github.com/honeycombio/husky v0.9.0
github.com/honeycombio/husky v0.9.0 // indirect
github.com/honeycombio/libhoney-go v1.15.8
github.com/jessevdk/go-flags v1.5.0
github.com/json-iterator/go v1.1.12
Expand All @@ -27,6 +27,7 @@ require (
github.com/sirupsen/logrus v1.8.1
github.com/spf13/viper v1.10.1
github.com/stretchr/testify v1.7.0
github.com/tidwall/gjson v1.14.0
github.com/vmihailenco/msgpack/v4 v4.3.11
go.opentelemetry.io/proto/otlp v0.9.0
golang.org/x/net v0.0.0-20210913180222-943fd674d43e // indirect
Expand Down
8 changes: 8 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,8 @@ github.com/honeycombio/husky v0.9.0 h1:TppxWwGCZb54qwHuPRAkxhht4b3btFcM2OvV1/Zs3
github.com/honeycombio/husky v0.9.0/go.mod h1:KltmTfiasGGV0L3Hv6KEzm9YSvv3vRTz/JRSq1K+d78=
github.com/honeycombio/libhoney-go v1.15.8 h1:TECEltZ48K6J4NG1JVYqmi0vCJNnHYooFor83fgKesA=
github.com/honeycombio/libhoney-go v1.15.8/go.mod h1:+tnL2etFnJmVx30yqmoUkVyQjp7uRJw0a2QGu48lSyY=
github.com/honeycombio/refinery v1.12.0 h1:o1392RE741TZp8+JMsj1sSpmHOs9+hVunPuozn4JPRY=
github.com/honeycombio/refinery v1.12.0/go.mod h1:ct0vuSsQrUiZZo7sMIQ2grCfeO985A0dwADjUuG81qE=
github.com/iancoleman/strcase v0.2.0/go.mod h1:iwCmte+B7n89clKwxIoIXy/HfoL7AsD47ZCWhYzw7ho=
github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
Expand Down Expand Up @@ -399,6 +401,12 @@ github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5Cc
github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/subosito/gotenv v1.2.0 h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s=
github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw=
github.com/tidwall/gjson v1.14.0 h1:6aeJ0bzojgWLa82gDQHcx3S0Lr/O51I9bJ5nv6JFx5w=
github.com/tidwall/gjson v1.14.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk=
github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA=
github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM=
github.com/tidwall/pretty v1.2.0 h1:RWIZEg2iJ8/g6fDDYzMpobmaoGh5OLl4AXtGUGPcqCs=
github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU=
github.com/tv42/httpunix v0.0.0-20150427012821-b75d8614f926/go.mod h1:9ESjWnEqriFuLhtthL60Sar/7RFoluCcXsuvEwTV5KM=
github.com/vmihailenco/msgpack/v4 v4.3.11 h1:Q47CePddpNGNhk4GCnAx9DDtASi2rasatE0cd26cZoE=
github.com/vmihailenco/msgpack/v4 v4.3.11/go.mod h1:gborTTJjAo/GWTqqRjrLCn9pgNN+NXzzngzBKDPIqw4=
Expand Down
12 changes: 12 additions & 0 deletions sample/rules.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
package sample

import (
"encoding/json"
"math/rand"
"strings"

"github.com/honeycombio/refinery/config"
"github.com/honeycombio/refinery/logger"
"github.com/honeycombio/refinery/metrics"
"github.com/honeycombio/refinery/types"
"github.com/tidwall/gjson"
)

type RulesBasedSampler struct {
Expand Down Expand Up @@ -70,6 +72,16 @@ func (s *RulesBasedSampler) GetSampleRate(trace *types.Trace) (rate uint, keep b
for _, span := range trace.GetSpans() {
var match bool
value, exists := span.Data[condition.Field]
if !exists {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm concerned this would impact users that does not want to pay the cost of attempting to JSON parse span data when trying to resolve rule conditions.

I think we should look into adding an opt-in configuration option to enable this behaviour. Default would be disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. I can add an optional parameter in the rules config file to enable it (similar to the AddSampleRateKeyToTrace and UseTraceLength fields), unless you think there is a better place for it.

jsonStr, err := json.Marshal(span.Data)
if err == nil {
result := gjson.Get(string(jsonStr), condition.Field)
if result.Exists() {
value = result.String()
exists = true
}
}
}

switch exists {
case true:
Expand Down
115 changes: 115 additions & 0 deletions sample/rules_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,121 @@ func TestRules(t *testing.T) {
}
}

func TestRulesWithNestedFields(t *testing.T) {
data := []TestRulesData{
{
Rules: &config.RulesBasedSamplerConfig{
Rule: []*config.RulesBasedSamplerRule{
{
Name: "nested field",
SampleRate: 10,
Condition: []*config.RulesBasedSamplerCondition{
{
Field: "test.test1",
Operator: "=",
Value: "a",
},
},
},
},
},
Spans: []*types.Span{
{
Event: types.Event{
Data: map[string]interface{}{
"test": map[string]interface{}{
"test1": "a",
},
},
},
},
},
ExpectedKeep: true,
ExpectedRate: 10,
},
{
Rules: &config.RulesBasedSamplerConfig{
Rule: []*config.RulesBasedSamplerRule{
{
Name: "field not nested",
SampleRate: 10,
Condition: []*config.RulesBasedSamplerCondition{
{
Field: "test.test1",
Operator: "=",
Value: "a",
},
},
},
},
},
Spans: []*types.Span{
{
Event: types.Event{
Data: map[string]interface{}{
"test.test1": "a",
},
},
},
},
ExpectedKeep: true,
ExpectedRate: 10,
},
{
Rules: &config.RulesBasedSamplerConfig{
Rule: []*config.RulesBasedSamplerRule{
{
Name: "not exists test",
SampleRate: 4,
Condition: []*config.RulesBasedSamplerCondition{
{
Field: "test.test1",
Operator: "not-exists",
},
},
},
},
},
Spans: []*types.Span{
{
Event: types.Event{
Data: map[string]interface{}{
"test": map[string]interface{}{
"test2": "b",
},
},
},
},
},
ExpectedKeep: true,
ExpectedRate: 4,
},
}

for _, d := range data {
sampler := &RulesBasedSampler{
Config: d.Rules,
Logger: &logger.NullLogger{},
Metrics: &metrics.NullMetrics{},
}

trace := &types.Trace{}

for _, span := range d.Spans {
trace.AddSpan(span)
}

rate, keep := sampler.GetSampleRate(trace)

assert.Equal(t, d.ExpectedRate, rate, d.Rules)

// we can only test when we don't expect to keep the trace
if !d.ExpectedKeep {
assert.Equal(t, d.ExpectedKeep, keep, d.Rules)
}
}
}

func TestRulesWithDynamicSampler(t *testing.T) {
data := []TestRulesData{
{
Expand Down