Skip to content

Commit

Permalink
Add endpoints to help debug refinery rules (#500)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?

There have been multiple reports of people not realizing their configs were not parsing the way they were intended. This adds two endpoints:
- `/debug/allrules/$FORMAT` will retrieve the entire rules configuration
- `/debug/rules/$FORMAT/$DATASET` will retrieve the rule set that refinery will use for the specified dataset. It comes back as a map of the sampler type to its rule set.

## Short description of the changes

- Add the endpoints
- Add a feature to config to return the full rule set
- Extend existing config feature to return the sampler name along with its rules struct
- Add/expand tests for it all
  • Loading branch information
kentquirk authored Sep 6, 2022
1 parent fcb0f80 commit 8442d86
Show file tree
Hide file tree
Showing 13 changed files with 281 additions and 45 deletions.
15 changes: 15 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,23 @@ Refinery emits a number of metrics to give some indication about the health of t

## Troubleshooting

### Logging

The default logging level of `warn` is almost entirely silent. The `debug` level emits too much data to be used in production, but contains excellent information in a pre-production environment. Setting the logging level to `debug` during initial configuration will help understand what's working and what's not, but when traffic volumes increase it should be set to `warn`.

### Configuration

Because the normal configuration file formats (TOML and YAML) can sometimes be confusing to read and write, it may be valuable to check the loaded configuration by using one of the debug endpoints from the command line:

`curl --include --get $REFINERY_HOST/debug/allrules/$FORMAT` will retrieve the entire rules configuration.

`curl --include --get $REFINERY_HOST/debug/rules/$FORMAT/$DATASET` will retrieve the rule set that refinery will use for the specified dataset. It comes back as a map of the sampler type to its rule set.

- `$REFINERY_HOST` should be the url of your refinery.
- `$FORMAT` can be one of `json`, `yaml`, or `toml`.
- `$DATASET` is the name of the dataset you want to check.


## Restarts

Refinery does not yet buffer traces or sampling decisions to disk. When you restart the process all in-flight traces will be flushed (sent upstream to Honeycomb), but you will lose the record of past trace decisions. When started back up, it will start with a clean slate.
Expand Down
7 changes: 5 additions & 2 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,11 @@ type Config interface {
// GetInMemCollectorCacheCapacity returns the config specific to the InMemCollector
GetInMemCollectorCacheCapacity() (InMemoryCollectorCacheCapacity, error)

// GetSamplerConfigForDataset returns the sampler type to use for the given dataset
GetSamplerConfigForDataset(string) (interface{}, error)
// GetSamplerConfigForDataset returns the sampler type and name to use for the given dataset
GetSamplerConfigForDataset(string) (interface{}, string, error)

// GetAllSamplerRules returns all dataset rules in a map, including the default
GetAllSamplerRules() (map[string]interface{}, error)

// GetMetricsType returns the type of metrics to use. Valid types are in the
// metrics package
Expand Down
32 changes: 22 additions & 10 deletions config/config_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build all || race
// +build all race

package config
Expand Down Expand Up @@ -212,9 +213,10 @@ func TestReadDefaults(t *testing.T) {
t.Error("received", d, "expected", time.Hour)
}

d, err := c.GetSamplerConfigForDataset("dataset-doesnt-exist")
d, name, err := c.GetSamplerConfigForDataset("dataset-doesnt-exist")
assert.NoError(t, err)
assert.IsType(t, &DeterministicSamplerConfig{}, d)
assert.Equal(t, "DeterministicSampler", name)

type imcConfig struct {
CacheCapacity int
Expand All @@ -234,15 +236,17 @@ func TestReadRulesConfig(t *testing.T) {
t.Error(err)
}

d, err := c.GetSamplerConfigForDataset("dataset-doesnt-exist")
d, name, err := c.GetSamplerConfigForDataset("dataset-doesnt-exist")
assert.NoError(t, err)
assert.IsType(t, &DeterministicSamplerConfig{}, d)
assert.Equal(t, "DeterministicSampler", name)

d, err = c.GetSamplerConfigForDataset("dataset1")
d, name, err = c.GetSamplerConfigForDataset("dataset1")
assert.NoError(t, err)
assert.IsType(t, &DynamicSamplerConfig{}, d)
assert.Equal(t, "DynamicSampler", name)

d, err = c.GetSamplerConfigForDataset("dataset4")
d, name, err = c.GetSamplerConfigForDataset("dataset4")
assert.NoError(t, err)
switch r := d.(type) {
case *RulesBasedSamplerConfig:
Expand All @@ -268,6 +272,8 @@ func TestReadRulesConfig(t *testing.T) {
assert.Equal(t, 10, rule.SampleRate)
assert.Equal(t, "", rule.Scope)

assert.Equal(t, "RulesBasedSampler", name)

default:
assert.Fail(t, "dataset4 should have a rules based sampler", d)
}
Expand Down Expand Up @@ -512,24 +518,29 @@ func TestGetSamplerTypes(t *testing.T) {
t.Error(err)
}

if d, err := c.GetSamplerConfigForDataset("dataset-doesnt-exist"); assert.Equal(t, nil, err) {
if d, name, err := c.GetSamplerConfigForDataset("dataset-doesnt-exist"); assert.Equal(t, nil, err) {
assert.IsType(t, &DeterministicSamplerConfig{}, d)
assert.Equal(t, "DeterministicSampler", name)
}

if d, err := c.GetSamplerConfigForDataset("dataset 1"); assert.Equal(t, nil, err) {
if d, name, err := c.GetSamplerConfigForDataset("dataset 1"); assert.Equal(t, nil, err) {
assert.IsType(t, &DynamicSamplerConfig{}, d)
assert.Equal(t, "DynamicSampler", name)
}

if d, err := c.GetSamplerConfigForDataset("dataset2"); assert.Equal(t, nil, err) {
if d, name, err := c.GetSamplerConfigForDataset("dataset2"); assert.Equal(t, nil, err) {
assert.IsType(t, &DeterministicSamplerConfig{}, d)
assert.Equal(t, "DeterministicSampler", name)
}

if d, err := c.GetSamplerConfigForDataset("dataset3"); assert.Equal(t, nil, err) {
if d, name, err := c.GetSamplerConfigForDataset("dataset3"); assert.Equal(t, nil, err) {
assert.IsType(t, &EMADynamicSamplerConfig{}, d)
assert.Equal(t, "EMADynamicSampler", name)
}

if d, err := c.GetSamplerConfigForDataset("dataset4"); assert.Equal(t, nil, err) {
if d, name, err := c.GetSamplerConfigForDataset("dataset4"); assert.Equal(t, nil, err) {
assert.IsType(t, &TotalThroughputSamplerConfig{}, d)
assert.Equal(t, "TotalThroughputSampler", name)
}
}

Expand Down Expand Up @@ -563,9 +574,10 @@ func TestDefaultSampler(t *testing.T) {

assert.NoError(t, err)

s, err := c.GetSamplerConfigForDataset("nonexistent")
s, name, err := c.GetSamplerConfigForDataset("nonexistent")

assert.NoError(t, err)
assert.Equal(t, "DeterministicSampler", name)

assert.IsType(t, &DeterministicSamplerConfig{}, s)
}
Expand Down
10 changes: 7 additions & 3 deletions config/config_test_reload_error_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build all || !race
// +build all !race

package config
Expand Down Expand Up @@ -55,9 +56,12 @@ func TestErrorReloading(t *testing.T) {
t.Error(err)
}

d, _ := c.GetSamplerConfigForDataset("dataset5")
d, name, _ := c.GetSamplerConfigForDataset("dataset5")
if _, ok := d.(DeterministicSamplerConfig); ok {
t.Error("received", d, "expected", "DeterministicSampler")
t.Error("type received", d, "expected", "DeterministicSampler")
}
if name != "DeterministicSampler" {
t.Error("name received", d, "expected", "DeterministicSampler")
}

wg := &sync.WaitGroup{}
Expand All @@ -82,7 +86,7 @@ func TestErrorReloading(t *testing.T) {
wg.Wait()

// config should error and not update sampler to invalid type
d, _ = c.GetSamplerConfigForDataset("dataset5")
d, _, _ = c.GetSamplerConfigForDataset("dataset5")
if _, ok := d.(DeterministicSamplerConfig); ok {
t.Error("received", d, "expected", "DeterministicSampler")
}
Expand Down
50 changes: 44 additions & 6 deletions config/file_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -521,10 +521,48 @@ func (f *fileConfig) GetCollectorType() (string, error) {
return f.conf.Collector, nil
}

func (f *fileConfig) GetSamplerConfigForDataset(dataset string) (interface{}, error) {
func (f *fileConfig) GetAllSamplerRules() (map[string]interface{}, error) {
samplers := make(map[string]interface{})

keys := f.rules.AllKeys()
for _, key := range keys {
parts := strings.Split(key, ".")

// extract default sampler rules
if parts[0] == "sampler" {
err := f.rules.Unmarshal(&samplers)
if err != nil {
return nil, fmt.Errorf("failed to unmarshal sampler rule: %w", err)
}
t := f.rules.GetString(key)
samplers["sampler"] = t
continue
}

// extract all dataset sampler rules
if len(parts) > 1 && parts[1] == "sampler" {
t := f.rules.GetString(key)
m := make(map[string]interface{})
datasetName := parts[0]
if sub := f.rules.Sub(datasetName); sub != nil {
err := sub.Unmarshal(&m)
if err != nil {
return nil, fmt.Errorf("failed to unmarshal sampler rule for dataset %s: %w", datasetName, err)
}
}
m["sampler"] = t
samplers[datasetName] = m
}
}
return samplers, nil
}

func (f *fileConfig) GetSamplerConfigForDataset(dataset string) (interface{}, string, error) {
f.mux.RLock()
defer f.mux.RUnlock()

const notfound = "not found"

key := fmt.Sprintf("%s.Sampler", dataset)
if ok := f.rules.IsSet(key); ok {
t := f.rules.GetString(key)
Expand All @@ -542,11 +580,11 @@ func (f *fileConfig) GetSamplerConfigForDataset(dataset string) (interface{}, er
case "TotalThroughputSampler":
i = &TotalThroughputSamplerConfig{}
default:
return nil, errors.New("No Sampler found")
return nil, notfound, errors.New("No Sampler found")
}

if sub := f.rules.Sub(dataset); sub != nil {
return i, sub.Unmarshal(i)
return i, t, sub.Unmarshal(i)
}

} else if ok := f.rules.IsSet("Sampler"); ok {
Expand All @@ -565,13 +603,13 @@ func (f *fileConfig) GetSamplerConfigForDataset(dataset string) (interface{}, er
case "TotalThroughputSampler":
i = &TotalThroughputSamplerConfig{}
default:
return nil, errors.New("No Sampler found")
return nil, notfound, errors.New("No Sampler found")
}

return i, f.rules.Unmarshal(i)
return i, t, f.rules.Unmarshal(i)
}

return nil, errors.New("No Sampler found")
return nil, notfound, errors.New("No Sampler found")
}

func (f *fileConfig) GetInMemCollectorCacheCapacity() (InMemoryCollectorCacheCapacity, error) {
Expand Down
14 changes: 12 additions & 2 deletions config/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ type MockConfig struct {
GetUseTLSInsecureErr error
GetUseTLSInsecureVal bool
GetSamplerTypeErr error
GetSamplerTypeName string
GetSamplerTypeVal interface{}
GetMetricsTypeErr error
GetMetricsTypeVal string
Expand Down Expand Up @@ -240,11 +241,20 @@ func (m *MockConfig) GetMaxBatchSize() uint {
}

// TODO: allow per-dataset mock values
func (m *MockConfig) GetSamplerConfigForDataset(dataset string) (interface{}, error) {
func (m *MockConfig) GetSamplerConfigForDataset(dataset string) (interface{}, string, error) {
m.Mux.RLock()
defer m.Mux.RUnlock()

return m.GetSamplerTypeVal, m.GetSamplerTypeErr
return m.GetSamplerTypeVal, m.GetSamplerTypeName, m.GetSamplerTypeErr
}

// GetAllSamplerRules returns all dataset rules, including the default
func (m *MockConfig) GetAllSamplerRules() (map[string]interface{}, error) {
m.Mux.RLock()
defer m.Mux.RUnlock()

v := map[string]interface{}{"dataset1": m.GetSamplerTypeVal}
return v, m.GetSamplerTypeErr
}

func (m *MockConfig) GetUpstreamBufferSize() int {
Expand Down
10 changes: 5 additions & 5 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ require (
github.com/facebookgo/startstop v0.0.0-20161013234910-bc158412526d
github.com/fsnotify/fsnotify v1.5.4
github.com/go-playground/validator v9.31.0+incompatible
github.com/golang/protobuf v1.5.2
github.com/gomodule/redigo v1.8.9
github.com/gorilla/mux v1.8.0
github.com/hashicorp/golang-lru v0.5.4
Expand All @@ -18,6 +17,7 @@ require (
github.com/jessevdk/go-flags v1.5.0
github.com/json-iterator/go v1.1.12
github.com/klauspost/compress v1.15.7
github.com/pelletier/go-toml/v2 v2.0.1
github.com/pkg/errors v0.9.1
github.com/prometheus/client_golang v1.12.2
github.com/rcrowley/go-metrics v0.0.0-20200313005456-10cdbea86bc0
Expand All @@ -27,9 +27,10 @@ require (
github.com/tidwall/gjson v1.14.1
github.com/vmihailenco/msgpack/v4 v4.3.11
go.opentelemetry.io/proto/otlp v0.11.0
golang.org/x/net v0.0.0-20220520000938-2e3eb7b945c2 // indirect
google.golang.org/grpc v1.48.0
google.golang.org/protobuf v1.28.0
gopkg.in/alexcesaro/statsd.v2 v2.0.0
gopkg.in/yaml.v2 v2.4.0
)

require (
Expand All @@ -41,6 +42,7 @@ require (
github.com/facebookgo/structtag v0.0.0-20150214074306-217e25fb9691 // indirect
github.com/go-playground/locales v0.13.0 // indirect
github.com/go-playground/universal-translator v0.17.0 // indirect
github.com/golang/protobuf v1.5.2 // indirect
github.com/grpc-ecosystem/grpc-gateway v1.16.0 // indirect
github.com/hashicorp/hcl v1.0.0 // indirect
github.com/leodido/go-urn v1.2.0 // indirect
Expand All @@ -50,7 +52,6 @@ require (
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
github.com/modern-go/reflect2 v1.0.2 // indirect
github.com/pelletier/go-toml v1.9.5 // indirect
github.com/pelletier/go-toml/v2 v2.0.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/prometheus/client_model v0.2.0 // indirect
github.com/prometheus/common v0.32.1 // indirect
Expand All @@ -65,13 +66,12 @@ require (
github.com/vmihailenco/msgpack/v5 v5.3.5 // indirect
github.com/vmihailenco/tagparser v0.1.1 // indirect
github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect
golang.org/x/net v0.0.0-20220520000938-2e3eb7b945c2 // indirect
golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8 // indirect
golang.org/x/text v0.3.7 // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/genproto v0.0.0-20220519153652-3a47de7e79bd // indirect
google.golang.org/protobuf v1.28.0 // indirect
gopkg.in/go-playground/assert.v1 v1.2.1 // indirect
gopkg.in/ini.v1 v1.66.4 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
1 change: 1 addition & 0 deletions internal/peer/peers_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build all || race
// +build all race

package peer
Expand Down
2 changes: 1 addition & 1 deletion route/otlp_trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"testing"
"time"

"github.com/golang/protobuf/proto"
huskyotlp "github.com/honeycombio/husky/otlp"
"github.com/honeycombio/refinery/config"
"github.com/honeycombio/refinery/logger"
Expand All @@ -24,6 +23,7 @@ import (
resource "go.opentelemetry.io/proto/otlp/resource/v1"
trace "go.opentelemetry.io/proto/otlp/trace/v1"
"google.golang.org/grpc/metadata"
"google.golang.org/protobuf/proto"
)

const legacyAPIKey = "c9945edf5d245834089a1bd6cc9ad01e"
Expand Down
3 changes: 1 addition & 2 deletions route/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package route
import (
"bytes"
"io"
"io/ioutil"
"net/http"
"strings"
)
Expand All @@ -25,7 +24,7 @@ func (r *Router) proxy(w http.ResponseWriter, req *http.Request) {
// let's copy the request over to a new one and
// dispatch it upstream
defer req.Body.Close()
reqBod, _ := ioutil.ReadAll(req.Body)
reqBod, _ := io.ReadAll(req.Body)
buf := bytes.NewBuffer(reqBod)
upstreamReq, err := http.NewRequest(req.Method, upstreamTarget+req.URL.String(), buf)
if err != nil {
Expand Down
Loading

0 comments on commit 8442d86

Please sign in to comment.