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

Log the source of sampling strategies #3166

Merged
merged 2 commits into from
Jul 28, 2021
Merged

Log the source of sampling strategies #3166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
38a4ff1
Log the source of sampling strategies
yurishkuro Jul 27, 2021
31a2f9e
fix tests
yurishkuro Jul 27, 2021
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
10 changes: 6 additions & 4 deletions plugin/sampling/strategystore/static/strategy_store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func NewStrategyStore(options Options, logger *zap.Logger) (ss.StrategyStore, er
return h, nil
}

loadFn := samplingStrategyLoader(options.StrategiesFile)
loadFn := h.samplingStrategyLoader(options.StrategiesFile)
strategies, err := loadStrategies(loadFn)
if err != nil {
return nil, err
Expand Down Expand Up @@ -95,7 +95,8 @@ func (h *strategyStore) Close() {
h.cancelFunc()
}

func downloadSamplingStrategies(url string) ([]byte, error) {
func (h *strategyStore) downloadSamplingStrategies(url string) ([]byte, error) {
h.logger.Info("Downloading sampling strategies", zap.String("url", url))
resp, err := http.Get(url)
if err != nil {
return nil, fmt.Errorf("failed to download sampling strategies: %w", err)
Expand Down Expand Up @@ -126,14 +127,15 @@ func isURL(str string) bool {
return err == nil && u.Scheme != "" && u.Host != ""
}

func samplingStrategyLoader(strategiesFile string) strategyLoader {
func (h *strategyStore) samplingStrategyLoader(strategiesFile string) strategyLoader {
if isURL(strategiesFile) {
return func() ([]byte, error) {
return downloadSamplingStrategies(strategiesFile)
return h.downloadSamplingStrategies(strategiesFile)
}
}

return func() ([]byte, error) {
h.logger.Info("Loading sampling strategies", zap.String("filename", strategiesFile))
currBytes, err := ioutil.ReadFile(filepath.Clean(strategiesFile))
if err != nil {
return nil, fmt.Errorf("failed to read strategies file %s: %w", strategiesFile, err)
Expand Down
21 changes: 11 additions & 10 deletions plugin/sampling/strategystore/static/strategy_store_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ func TestAutoUpdateStrategyWithFile(t *testing.T) {
assert.EqualValues(t, makeResponse(sampling.SamplingStrategyType_PROBABILISTIC, 0.8), *s)

// verify that reloading is a no-op
value := store.reloadSamplingStrategy(samplingStrategyLoader(dstFile), string(srcBytes))
value := store.reloadSamplingStrategy(store.samplingStrategyLoader(dstFile), string(srcBytes))
assert.Equal(t, string(srcBytes), value)

// update file with new probability of 0.9
Expand Down Expand Up @@ -394,7 +394,7 @@ func TestAutoUpdateStrategyWithURL(t *testing.T) {
assert.EqualValues(t, makeResponse(sampling.SamplingStrategyType_PROBABILISTIC, 0.8), *s)

// verify that reloading in no-op
value := store.reloadSamplingStrategy(samplingStrategyLoader(mockServer.URL), mockStrategy.Load())
value := store.reloadSamplingStrategy(store.samplingStrategyLoader(mockServer.URL), mockStrategy.Load())
assert.Equal(t, mockStrategy.Load(), value)

// update original strategies with new probability of 0.9
Expand Down Expand Up @@ -431,25 +431,25 @@ func TestAutoUpdateStrategyErrors(t *testing.T) {
defer store.Close()

// check invalid file path or read failure
assert.Equal(t, "blah", store.reloadSamplingStrategy(samplingStrategyLoader(tempFile.Name()+"bad-path"), "blah"))
assert.Equal(t, "blah", store.reloadSamplingStrategy(store.samplingStrategyLoader(tempFile.Name()+"bad-path"), "blah"))
assert.Len(t, logs.FilterMessage("failed to re-load sampling strategies").All(), 1)

// check bad file content
require.NoError(t, ioutil.WriteFile(tempFile.Name(), []byte("bad value"), 0644))
assert.Equal(t, "blah", store.reloadSamplingStrategy(samplingStrategyLoader(tempFile.Name()), "blah"))
assert.Equal(t, "blah", store.reloadSamplingStrategy(store.samplingStrategyLoader(tempFile.Name()), "blah"))
assert.Len(t, logs.FilterMessage("failed to update sampling strategies").All(), 1)

// check invalid url
assert.Equal(t, "duh", store.reloadSamplingStrategy(samplingStrategyLoader("bad-url"), "duh"))
assert.Equal(t, "duh", store.reloadSamplingStrategy(store.samplingStrategyLoader("bad-url"), "duh"))
assert.Len(t, logs.FilterMessage("failed to re-load sampling strategies").All(), 2)

// check status code other than 200
mockServer, _ := mockStrategyServer()
assert.Equal(t, "duh", store.reloadSamplingStrategy(samplingStrategyLoader(mockServer.URL+"/bad-status"), "duh"))
assert.Equal(t, "duh", store.reloadSamplingStrategy(store.samplingStrategyLoader(mockServer.URL+"/bad-status"), "duh"))
assert.Len(t, logs.FilterMessage("failed to re-load sampling strategies").All(), 3)

// check bad content from url
assert.Equal(t, "duh", store.reloadSamplingStrategy(samplingStrategyLoader(mockServer.URL+"/bad-content"), "duh"))
assert.Equal(t, "duh", store.reloadSamplingStrategy(store.samplingStrategyLoader(mockServer.URL+"/bad-content"), "duh"))
assert.Len(t, logs.FilterMessage("failed to update sampling strategies").All(), 2)
}

Expand All @@ -469,19 +469,20 @@ func TestServiceNoPerOperationStrategies(t *testing.T) {
}

func TestSamplingStrategyLoader(t *testing.T) {
store := &strategyStore{logger: zap.NewNop()}
// invalid file path
loader := samplingStrategyLoader("not-exists")
loader := store.samplingStrategyLoader("not-exists")
_, err := loader()
assert.Contains(t, err.Error(), "failed to read strategies file not-exists")

// status code other than 200
mockServer, _ := mockStrategyServer()
loader = samplingStrategyLoader(mockServer.URL + "/bad-status")
loader = store.samplingStrategyLoader(mockServer.URL + "/bad-status")
_, err = loader()
assert.Contains(t, err.Error(), "receiving 404 Not Found while downloading strategies file")

// should download content from URL
loader = samplingStrategyLoader(mockServer.URL + "/bad-content")
loader = store.samplingStrategyLoader(mockServer.URL + "/bad-content")
content, err := loader()
require.NoError(t, err)
assert.Equal(t, "bad-content", string(content))
Expand Down