From 64eba7f429e450b416620db1943726dce530fc98 Mon Sep 17 00:00:00 2001 From: Rutger Broekhoff Date: Tue, 20 Aug 2019 20:21:12 +0200 Subject: [PATCH 1/9] Allow default operation strategies in strategy config Makes it possible to use operation_strategies in default_strategy in the sampling strategy configuration file. Signed-off-by: Rutger Broekhoff --- .../sampling/strategystore/static/strategy.go | 2 +- .../strategystore/static/strategy_store.go | 49 +++++++++---------- 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/plugin/sampling/strategystore/static/strategy.go b/plugin/sampling/strategystore/static/strategy.go index f16a41795ec..488f08d481d 100644 --- a/plugin/sampling/strategystore/static/strategy.go +++ b/plugin/sampling/strategystore/static/strategy.go @@ -19,6 +19,7 @@ package static type strategy struct { Type string `json:"type"` Param float64 `json:"param"` + OperationStrategies []*operationStrategy `json:"operation_strategies"` } // operationStrategy defines an operation specific sampling strategy. @@ -30,7 +31,6 @@ type operationStrategy struct { // serviceStrategy defines a service specific sampling strategy. type serviceStrategy struct { Service string `json:"service"` - OperationStrategies []*operationStrategy `json:"operation_strategies"` strategy } diff --git a/plugin/sampling/strategystore/static/strategy_store.go b/plugin/sampling/strategystore/static/strategy_store.go index fce2c080d10..2656a11ef16 100644 --- a/plugin/sampling/strategystore/static/strategy_store.go +++ b/plugin/sampling/strategystore/static/strategy_store.go @@ -83,12 +83,33 @@ func (h *strategyStore) parseStrategies(strategies *strategies) { h.defaultStrategy = h.parseStrategy(strategies.DefaultStrategy) } for _, s := range strategies.ServiceStrategies { - h.serviceStrategies[s.Service] = h.parseServiceStrategies(s) + h.serviceStrategies[s.Service] = h.parseStrategy(s) } } -func (h *strategyStore) parseServiceStrategies(strategy *serviceStrategy) *sampling.SamplingStrategyResponse { - resp := h.parseStrategy(&strategy.strategy) +func (h *strategyStore) parseStrategy(strategy *strategy) *sampling.SamplingStrategyResponse { + var resp *sampling.SamplingStrategyResponse + + switch strategy.Type { + case samplerTypeProbabilistic: + resp = &sampling.SamplingStrategyResponse{ + StrategyType: sampling.SamplingStrategyType_PROBABILISTIC, + ProbabilisticSampling: &sampling.ProbabilisticSamplingStrategy{ + SamplingRate: strategy.Param, + }, + } + case samplerTypeRateLimiting: + resp = &sampling.SamplingStrategyResponse{ + StrategyType: sampling.SamplingStrategyType_RATE_LIMITING, + RateLimitingSampling: &sampling.RateLimitingSamplingStrategy{ + MaxTracesPerSecond: int16(strategy.Param), + }, + } + default: + h.logger.Warn("Failed to parse sampling strategy", zap.Any("strategy", strategy)) + resp = deepCopy(&defaultStrategy) + } + if len(strategy.OperationStrategies) == 0 { return resp } @@ -120,28 +141,6 @@ func (h *strategyStore) parseServiceStrategies(strategy *serviceStrategy) *sampl return resp } -func (h *strategyStore) parseStrategy(strategy *strategy) *sampling.SamplingStrategyResponse { - switch strategy.Type { - case samplerTypeProbabilistic: - return &sampling.SamplingStrategyResponse{ - StrategyType: sampling.SamplingStrategyType_PROBABILISTIC, - ProbabilisticSampling: &sampling.ProbabilisticSamplingStrategy{ - SamplingRate: strategy.Param, - }, - } - case samplerTypeRateLimiting: - return &sampling.SamplingStrategyResponse{ - StrategyType: sampling.SamplingStrategyType_RATE_LIMITING, - RateLimitingSampling: &sampling.RateLimitingSamplingStrategy{ - MaxTracesPerSecond: int16(strategy.Param), - }, - } - default: - h.logger.Warn("Failed to parse sampling strategy", zap.Any("strategy", strategy)) - return deepCopy(&defaultStrategy) - } -} - func deepCopy(s *sampling.SamplingStrategyResponse) *sampling.SamplingStrategyResponse { var buf bytes.Buffer enc := gob.NewEncoder(&buf) From c286db882e4bdfcf2ff0498b2e32a0d8333fd68a Mon Sep 17 00:00:00 2001 From: Rutger Broekhoff Date: Tue, 20 Aug 2019 20:35:37 +0200 Subject: [PATCH 2/9] Fix code style Signed-off-by: Rutger Broekhoff --- plugin/sampling/strategystore/static/strategy.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugin/sampling/strategystore/static/strategy.go b/plugin/sampling/strategystore/static/strategy.go index 488f08d481d..59e69489161 100644 --- a/plugin/sampling/strategystore/static/strategy.go +++ b/plugin/sampling/strategystore/static/strategy.go @@ -17,8 +17,8 @@ package static // strategy defines a sampling strategy. Type can be "probabilistic" or "ratelimiting" // and Param will represent "sampling probability" and "max traces per second" respectively. type strategy struct { - Type string `json:"type"` - Param float64 `json:"param"` + Type string `json:"type"` + Param float64 `json:"param"` OperationStrategies []*operationStrategy `json:"operation_strategies"` } @@ -30,7 +30,7 @@ type operationStrategy struct { // serviceStrategy defines a service specific sampling strategy. type serviceStrategy struct { - Service string `json:"service"` + Service string `json:"service"` strategy } From 0bcab5e91f558dea38cebde414f09617362cc312 Mon Sep 17 00:00:00 2001 From: Rutger Broekhoff Date: Tue, 20 Aug 2019 22:40:50 +0200 Subject: [PATCH 3/9] Fix compilation error Signed-off-by: Rutger Broekhoff --- plugin/sampling/strategystore/static/strategy_store.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/sampling/strategystore/static/strategy_store.go b/plugin/sampling/strategystore/static/strategy_store.go index 2656a11ef16..ef4f7df19c5 100644 --- a/plugin/sampling/strategystore/static/strategy_store.go +++ b/plugin/sampling/strategystore/static/strategy_store.go @@ -83,7 +83,7 @@ func (h *strategyStore) parseStrategies(strategies *strategies) { h.defaultStrategy = h.parseStrategy(strategies.DefaultStrategy) } for _, s := range strategies.ServiceStrategies { - h.serviceStrategies[s.Service] = h.parseStrategy(s) + h.serviceStrategies[s.Service] = h.parseStrategy(&s.strategy) } } From 000259e3530db7108c27449356437055da8495f1 Mon Sep 17 00:00:00 2001 From: Rutger Broekhoff Date: Tue, 5 Nov 2019 08:45:07 +0100 Subject: [PATCH 4/9] Apply suggestions for sampling strategies and add tests Signed-off-by: Rutger Broekhoff --- .../static/fixtures/operation_strategies.json | 12 ++ .../sampling/strategystore/static/strategy.go | 13 +- .../strategystore/static/strategy_store.go | 139 ++++++++++++++---- .../static/strategy_store_test.go | 33 +++-- 4 files changed, 153 insertions(+), 44 deletions(-) diff --git a/plugin/sampling/strategystore/static/fixtures/operation_strategies.json b/plugin/sampling/strategystore/static/fixtures/operation_strategies.json index d0c00fb3247..9f22c7f4381 100644 --- a/plugin/sampling/strategystore/static/fixtures/operation_strategies.json +++ b/plugin/sampling/strategystore/static/fixtures/operation_strategies.json @@ -3,12 +3,24 @@ "type": "probabilistic", "param": 0.5 }, + "default_operation_strategies": [ + { + "operation": "/health", + "type": "probabilistic", + "param": 0 + } + ], "service_strategies": [ { "service": "foo", "type": "probabilistic", "param": 0.8, "operation_strategies": [ + { + "operation": "/health", + "type": "probabilistic", + "param": 0.5 + }, { "operation": "op1", "type": "probabilistic", diff --git a/plugin/sampling/strategystore/static/strategy.go b/plugin/sampling/strategystore/static/strategy.go index 59e69489161..93d58e08145 100644 --- a/plugin/sampling/strategystore/static/strategy.go +++ b/plugin/sampling/strategystore/static/strategy.go @@ -17,9 +17,8 @@ package static // strategy defines a sampling strategy. Type can be "probabilistic" or "ratelimiting" // and Param will represent "sampling probability" and "max traces per second" respectively. type strategy struct { - Type string `json:"type"` - Param float64 `json:"param"` - OperationStrategies []*operationStrategy `json:"operation_strategies"` + Type string `json:"type"` + Param float64 `json:"param"` } // operationStrategy defines an operation specific sampling strategy. @@ -30,12 +29,14 @@ type operationStrategy struct { // serviceStrategy defines a service specific sampling strategy. type serviceStrategy struct { - Service string `json:"service"` + Service string `json:"service"` + OperationStrategies []*operationStrategy `json:"operation_strategies"` strategy } // strategies holds a default sampling strategy and service specific sampling strategies. type strategies struct { - DefaultStrategy *strategy `json:"default_strategy"` - ServiceStrategies []*serviceStrategy `json:"service_strategies"` + DefaultStrategy *strategy `json:"default_strategy"` + DefaultOperationStrategies []*operationStrategy `json:"default_operation_strategies"` + ServiceStrategies []*serviceStrategy `json:"service_strategies"` } diff --git a/plugin/sampling/strategystore/static/strategy_store.go b/plugin/sampling/strategystore/static/strategy_store.go index ef4f7df19c5..15c0fcd75e7 100644 --- a/plugin/sampling/strategystore/static/strategy_store.go +++ b/plugin/sampling/strategystore/static/strategy_store.go @@ -20,6 +20,7 @@ import ( "encoding/json" "fmt" "io/ioutil" + "sort" "github.com/pkg/errors" "go.uber.org/zap" @@ -82,34 +83,80 @@ func (h *strategyStore) parseStrategies(strategies *strategies) { if strategies.DefaultStrategy != nil { h.defaultStrategy = h.parseStrategy(strategies.DefaultStrategy) } + for _, s := range strategies.DefaultOperationStrategies { + opS := h.defaultStrategy.OperationSampling + if opS == nil { + opS = sampling.NewPerOperationSamplingStrategies() + h.defaultStrategy.OperationSampling = opS + } + + strategy, ok := h.parseOperationStrategy(s, opS) + if !ok { + continue + } + + opS.PerOperationStrategies = append(opS.PerOperationStrategies, + &sampling.OperationSamplingStrategy{ + Operation: s.Operation, + ProbabilisticSampling: strategy.ProbabilisticSampling, + }) + } for _, s := range strategies.ServiceStrategies { - h.serviceStrategies[s.Service] = h.parseStrategy(&s.strategy) + h.serviceStrategies[s.Service] = h.parseServiceStrategies(s) + + // Merge with the default operation strategies, because only merging with + // the default strategy has no effect on service strategies (the default strategy + // is not merged with and only used as a fallback). + opS := h.serviceStrategies[s.Service].OperationSampling + if opS == nil { + // It has no use to merge, just reference the default settings. + h.serviceStrategies[s.Service].OperationSampling = h.defaultStrategy.OperationSampling + continue + } + if h.defaultStrategy.OperationSampling == nil || + h.defaultStrategy.OperationSampling.PerOperationStrategies == nil { + continue + } + opS.PerOperationStrategies = mergePerOperationSamplingStrategies( + opS.PerOperationStrategies, + h.defaultStrategy.OperationSampling.PerOperationStrategies) } } -func (h *strategyStore) parseStrategy(strategy *strategy) *sampling.SamplingStrategyResponse { - var resp *sampling.SamplingStrategyResponse +// mergePerOperationStrategies merges two operation strategies a and b, where a takes precedence over b. +func mergePerOperationSamplingStrategies( + a, b []*sampling.OperationSamplingStrategy, +) []*sampling.OperationSamplingStrategy { + // Guess the size of the slice of the two merged. + merged := make([]*sampling.OperationSamplingStrategy, 0, (len(a)+len(b))/4*3) - switch strategy.Type { - case samplerTypeProbabilistic: - resp = &sampling.SamplingStrategyResponse{ - StrategyType: sampling.SamplingStrategyType_PROBABILISTIC, - ProbabilisticSampling: &sampling.ProbabilisticSamplingStrategy{ - SamplingRate: strategy.Param, - }, - } - case samplerTypeRateLimiting: - resp = &sampling.SamplingStrategyResponse{ - StrategyType: sampling.SamplingStrategyType_RATE_LIMITING, - RateLimitingSampling: &sampling.RateLimitingSamplingStrategy{ - MaxTracesPerSecond: int16(strategy.Param), - }, + ossLess := func(s []*sampling.OperationSamplingStrategy, i, j int) bool { + return s[i].Operation < s[j].Operation + } + sort.Slice(a, func(i, j int) bool { return ossLess(a, i, j) }) + sort.Slice(b, func(i, j int) bool { return ossLess(b, i, j) }) + + j := 0 + for i := range a { + // Increment j till b[j] > a[i], such that in the loop after the + // loop over a no remaining element of b with the same operation + // as a[i] is added to the merged slice. + for ; j < len(b) && b[j].Operation <= a[i].Operation; j++ { + if b[j].Operation < a[i].Operation { + merged = append(merged, b[j]) + } } - default: - h.logger.Warn("Failed to parse sampling strategy", zap.Any("strategy", strategy)) - resp = deepCopy(&defaultStrategy) + merged = append(merged, a[i]) + } + for ; j < len(b); j++ { + merged = append(merged, b[j]) } + return merged +} + +func (h *strategyStore) parseServiceStrategies(strategy *serviceStrategy) *sampling.SamplingStrategyResponse { + resp := h.parseStrategy(&strategy.strategy) if len(strategy.OperationStrategies) == 0 { return resp } @@ -120,17 +167,11 @@ func (h *strategyStore) parseStrategy(strategy *strategy) *sampling.SamplingStra opS.DefaultSamplingProbability = resp.ProbabilisticSampling.SamplingRate } for _, operationStrategy := range strategy.OperationStrategies { - s := h.parseStrategy(&operationStrategy.strategy) - if s.StrategyType == sampling.SamplingStrategyType_RATE_LIMITING { - // TODO OperationSamplingStrategy only supports probabilistic sampling - h.logger.Warn( - fmt.Sprintf( - "Operation strategies only supports probabilistic sampling at the moment,"+ - "'%s' defaulting to probabilistic sampling with probability %f", - operationStrategy.Operation, opS.DefaultSamplingProbability), - zap.Any("strategy", operationStrategy)) + s, ok := h.parseOperationStrategy(operationStrategy, opS) + if !ok { continue } + opS.PerOperationStrategies = append(opS.PerOperationStrategies, &sampling.OperationSamplingStrategy{ Operation: operationStrategy.Operation, @@ -141,6 +182,46 @@ func (h *strategyStore) parseStrategy(strategy *strategy) *sampling.SamplingStra return resp } +func (h *strategyStore) parseOperationStrategy( + strategy *operationStrategy, + parent *sampling.PerOperationSamplingStrategies, +) (s *sampling.SamplingStrategyResponse, ok bool) { + s = h.parseStrategy(&strategy.strategy) + if s.StrategyType == sampling.SamplingStrategyType_RATE_LIMITING { + // TODO OperationSamplingStrategy only supports probabilistic sampling + h.logger.Warn( + fmt.Sprintf( + "Operation strategies only supports probabilistic sampling at the moment,"+ + "'%s' defaulting to probabilistic sampling with probability %f", + strategy.Operation, parent.DefaultSamplingProbability), + zap.Any("strategy", strategy)) + return nil, false + } + return s, true +} + +func (h *strategyStore) parseStrategy(strategy *strategy) *sampling.SamplingStrategyResponse { + switch strategy.Type { + case samplerTypeProbabilistic: + return &sampling.SamplingStrategyResponse{ + StrategyType: sampling.SamplingStrategyType_PROBABILISTIC, + ProbabilisticSampling: &sampling.ProbabilisticSamplingStrategy{ + SamplingRate: strategy.Param, + }, + } + case samplerTypeRateLimiting: + return &sampling.SamplingStrategyResponse{ + StrategyType: sampling.SamplingStrategyType_RATE_LIMITING, + RateLimitingSampling: &sampling.RateLimitingSamplingStrategy{ + MaxTracesPerSecond: int16(strategy.Param), + }, + } + default: + h.logger.Warn("Failed to parse sampling strategy", zap.Any("strategy", strategy)) + return deepCopy(&defaultStrategy) + } +} + func deepCopy(s *sampling.SamplingStrategyResponse) *sampling.SamplingStrategyResponse { var buf bytes.Buffer enc := gob.NewEncoder(&buf) diff --git a/plugin/sampling/strategystore/static/strategy_store_test.go b/plugin/sampling/strategystore/static/strategy_store_test.go index fd587e1aba5..714da6a6c44 100644 --- a/plugin/sampling/strategystore/static/strategy_store_test.go +++ b/plugin/sampling/strategystore/static/strategy_store_test.go @@ -77,9 +77,11 @@ func TestPerOperationSamplingStrategies(t *testing.T) { require.NotNil(t, s.OperationSampling) os := s.OperationSampling assert.EqualValues(t, os.DefaultSamplingProbability, 0.8) - require.Len(t, os.PerOperationStrategies, 1) - assert.Equal(t, "op1", os.PerOperationStrategies[0].Operation) - assert.EqualValues(t, 0.2, os.PerOperationStrategies[0].ProbabilisticSampling.SamplingRate) + require.Len(t, os.PerOperationStrategies, 2) + assert.Equal(t, "/health", os.PerOperationStrategies[0].Operation) + assert.EqualValues(t, 0.5, os.PerOperationStrategies[0].ProbabilisticSampling.SamplingRate) + assert.Equal(t, "op1", os.PerOperationStrategies[1].Operation) + assert.EqualValues(t, 0.2, os.PerOperationStrategies[1].ProbabilisticSampling.SamplingRate) expected = makeResponse(sampling.SamplingStrategyType_RATE_LIMITING, 5) @@ -91,15 +93,28 @@ func TestPerOperationSamplingStrategies(t *testing.T) { require.NotNil(t, s.OperationSampling) os = s.OperationSampling assert.EqualValues(t, os.DefaultSamplingProbability, 0.001) - require.Len(t, os.PerOperationStrategies, 2) - assert.Equal(t, "op3", os.PerOperationStrategies[0].Operation) - assert.EqualValues(t, 0.3, os.PerOperationStrategies[0].ProbabilisticSampling.SamplingRate) - assert.Equal(t, "op5", os.PerOperationStrategies[1].Operation) - assert.EqualValues(t, 0.4, os.PerOperationStrategies[1].ProbabilisticSampling.SamplingRate) + require.Len(t, os.PerOperationStrategies, 3) + assert.Equal(t, "/health", os.PerOperationStrategies[0].Operation) + assert.EqualValues(t, 0, os.PerOperationStrategies[0].ProbabilisticSampling.SamplingRate) + assert.Equal(t, "op3", os.PerOperationStrategies[1].Operation) + assert.EqualValues(t, 0.3, os.PerOperationStrategies[1].ProbabilisticSampling.SamplingRate) + assert.Equal(t, "op5", os.PerOperationStrategies[2].Operation) + assert.EqualValues(t, 0.4, os.PerOperationStrategies[2].ProbabilisticSampling.SamplingRate) s, err = store.GetSamplingStrategy("default") require.NoError(t, err) - assert.EqualValues(t, makeResponse(sampling.SamplingStrategyType_PROBABILISTIC, 0.5), *s) + expectedRsp := makeResponse(sampling.SamplingStrategyType_PROBABILISTIC, 0.5) + expectedRsp.OperationSampling = &sampling.PerOperationSamplingStrategies{ + PerOperationStrategies: []*sampling.OperationSamplingStrategy{ + { + Operation: "/health", + ProbabilisticSampling: &sampling.ProbabilisticSamplingStrategy{ + SamplingRate: 0, + }, + }, + }, + } + assert.EqualValues(t, expectedRsp, *s) } func TestMissingServiceSamplingStrategyTypes(t *testing.T) { From fa309478107a75825207d08e2ae4e569f1be154d Mon Sep 17 00:00:00 2001 From: Rutger Broekhoff Date: Tue, 5 Nov 2019 17:14:42 +0100 Subject: [PATCH 5/9] Increase strategy store coverage Signed-off-by: Rutger Broekhoff --- .../static/fixtures/operation_strategies.json | 19 ++++++++-- .../static/strategy_store_test.go | 36 +++++++++++++++---- 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/plugin/sampling/strategystore/static/fixtures/operation_strategies.json b/plugin/sampling/strategystore/static/fixtures/operation_strategies.json index 9f22c7f4381..1590fbf6374 100644 --- a/plugin/sampling/strategystore/static/fixtures/operation_strategies.json +++ b/plugin/sampling/strategystore/static/fixtures/operation_strategies.json @@ -5,9 +5,24 @@ }, "default_operation_strategies": [ { - "operation": "/health", + "operation": "op0", + "type": "probabilistic", + "param": 0.2 + }, + { + "operation": "op6", "type": "probabilistic", "param": 0 + }, + { + "operation": "spam", + "type": "ratelimiting", + "param": 1 + }, + { + "operation": "op7", + "type": "probabilistic", + "param": 1 } ], "service_strategies": [ @@ -17,7 +32,7 @@ "param": 0.8, "operation_strategies": [ { - "operation": "/health", + "operation": "op6", "type": "probabilistic", "param": 0.5 }, diff --git a/plugin/sampling/strategystore/static/strategy_store_test.go b/plugin/sampling/strategystore/static/strategy_store_test.go index 714da6a6c44..a86fb769af6 100644 --- a/plugin/sampling/strategystore/static/strategy_store_test.go +++ b/plugin/sampling/strategystore/static/strategy_store_test.go @@ -15,6 +15,7 @@ package static import ( + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -77,11 +78,16 @@ func TestPerOperationSamplingStrategies(t *testing.T) { require.NotNil(t, s.OperationSampling) os := s.OperationSampling assert.EqualValues(t, os.DefaultSamplingProbability, 0.8) - require.Len(t, os.PerOperationStrategies, 2) - assert.Equal(t, "/health", os.PerOperationStrategies[0].Operation) - assert.EqualValues(t, 0.5, os.PerOperationStrategies[0].ProbabilisticSampling.SamplingRate) + require.Len(t, os.PerOperationStrategies, 4) + fmt.Println(os) + assert.Equal(t, "op0", os.PerOperationStrategies[0].Operation) + assert.EqualValues(t, 0.2, os.PerOperationStrategies[0].ProbabilisticSampling.SamplingRate) assert.Equal(t, "op1", os.PerOperationStrategies[1].Operation) assert.EqualValues(t, 0.2, os.PerOperationStrategies[1].ProbabilisticSampling.SamplingRate) + assert.Equal(t, "op6", os.PerOperationStrategies[2].Operation) + assert.EqualValues(t, 0.5, os.PerOperationStrategies[2].ProbabilisticSampling.SamplingRate) + assert.Equal(t, "op7", os.PerOperationStrategies[3].Operation) + assert.EqualValues(t, 1, os.PerOperationStrategies[3].ProbabilisticSampling.SamplingRate) expected = makeResponse(sampling.SamplingStrategyType_RATE_LIMITING, 5) @@ -93,13 +99,17 @@ func TestPerOperationSamplingStrategies(t *testing.T) { require.NotNil(t, s.OperationSampling) os = s.OperationSampling assert.EqualValues(t, os.DefaultSamplingProbability, 0.001) - require.Len(t, os.PerOperationStrategies, 3) - assert.Equal(t, "/health", os.PerOperationStrategies[0].Operation) - assert.EqualValues(t, 0, os.PerOperationStrategies[0].ProbabilisticSampling.SamplingRate) + require.Len(t, os.PerOperationStrategies, 5) + assert.Equal(t, "op0", os.PerOperationStrategies[0].Operation) + assert.EqualValues(t, 0.2, os.PerOperationStrategies[0].ProbabilisticSampling.SamplingRate) assert.Equal(t, "op3", os.PerOperationStrategies[1].Operation) assert.EqualValues(t, 0.3, os.PerOperationStrategies[1].ProbabilisticSampling.SamplingRate) assert.Equal(t, "op5", os.PerOperationStrategies[2].Operation) assert.EqualValues(t, 0.4, os.PerOperationStrategies[2].ProbabilisticSampling.SamplingRate) + assert.Equal(t, "op6", os.PerOperationStrategies[3].Operation) + assert.EqualValues(t, 0, os.PerOperationStrategies[3].ProbabilisticSampling.SamplingRate) + assert.Equal(t, "op7", os.PerOperationStrategies[4].Operation) + assert.EqualValues(t, 1, os.PerOperationStrategies[4].ProbabilisticSampling.SamplingRate) s, err = store.GetSamplingStrategy("default") require.NoError(t, err) @@ -107,11 +117,23 @@ func TestPerOperationSamplingStrategies(t *testing.T) { expectedRsp.OperationSampling = &sampling.PerOperationSamplingStrategies{ PerOperationStrategies: []*sampling.OperationSamplingStrategy{ { - Operation: "/health", + Operation: "op0", + ProbabilisticSampling: &sampling.ProbabilisticSamplingStrategy{ + SamplingRate: 0.2, + }, + }, + { + Operation: "op6", ProbabilisticSampling: &sampling.ProbabilisticSamplingStrategy{ SamplingRate: 0, }, }, + { + Operation: "op7", + ProbabilisticSampling: &sampling.ProbabilisticSamplingStrategy{ + SamplingRate: 1, + }, + }, }, } assert.EqualValues(t, expectedRsp, *s) From f9cfc97003aea7c1cfe98a11bae9eac0b1fe0d06 Mon Sep 17 00:00:00 2001 From: Rutger Broekhoff Date: Sat, 21 Dec 2019 11:47:22 +0100 Subject: [PATCH 6/9] Add operation_strategies back to default_strategy Signed-off-by: Rutger Broekhoff --- .../strategystore/static/constants.go | 2 +- .../sampling/strategystore/static/strategy.go | 11 ++-- .../strategystore/static/strategy_store.go | 54 ++++++++++++------- 3 files changed, 43 insertions(+), 24 deletions(-) diff --git a/plugin/sampling/strategystore/static/constants.go b/plugin/sampling/strategystore/static/constants.go index 32b1f74b823..981fb749ac3 100644 --- a/plugin/sampling/strategystore/static/constants.go +++ b/plugin/sampling/strategystore/static/constants.go @@ -35,7 +35,7 @@ const ( var ( // defaultStrategy is the default sampling strategy the Strategy Store will return // if none is provided. - defaultStrategy = sampling.SamplingStrategyResponse{ + defaultStrategyResponse = sampling.SamplingStrategyResponse{ StrategyType: sampling.SamplingStrategyType_PROBABILISTIC, ProbabilisticSampling: &sampling.ProbabilisticSamplingStrategy{ SamplingRate: defaultSamplingProbability, diff --git a/plugin/sampling/strategystore/static/strategy.go b/plugin/sampling/strategystore/static/strategy.go index 93d58e08145..5503650e201 100644 --- a/plugin/sampling/strategystore/static/strategy.go +++ b/plugin/sampling/strategystore/static/strategy.go @@ -34,9 +34,14 @@ type serviceStrategy struct { strategy } +// defaultStrategy defines the default strategy. +type defaultStrategy struct { + OperationStrategies []*operationStrategy `json:"operation_strategies"` + strategy +} + // strategies holds a default sampling strategy and service specific sampling strategies. type strategies struct { - DefaultStrategy *strategy `json:"default_strategy"` - DefaultOperationStrategies []*operationStrategy `json:"default_operation_strategies"` - ServiceStrategies []*serviceStrategy `json:"service_strategies"` + DefaultStrategy *defaultStrategy `json:"default_strategy"` + ServiceStrategies []*serviceStrategy `json:"service_strategies"` } diff --git a/plugin/sampling/strategystore/static/strategy_store.go b/plugin/sampling/strategystore/static/strategy_store.go index 15c0fcd75e7..09010899379 100644 --- a/plugin/sampling/strategystore/static/strategy_store.go +++ b/plugin/sampling/strategystore/static/strategy_store.go @@ -75,32 +75,15 @@ func loadStrategies(strategiesFile string) (*strategies, error) { } func (h *strategyStore) parseStrategies(strategies *strategies) { - h.defaultStrategy = &defaultStrategy + h.defaultStrategy = &defaultStrategyResponse if strategies == nil { h.logger.Info("No sampling strategies provided, using defaults") return } if strategies.DefaultStrategy != nil { - h.defaultStrategy = h.parseStrategy(strategies.DefaultStrategy) + h.defaultStrategy = h.parseDefaultStrategy(strategies.DefaultStrategy) } - for _, s := range strategies.DefaultOperationStrategies { - opS := h.defaultStrategy.OperationSampling - if opS == nil { - opS = sampling.NewPerOperationSamplingStrategies() - h.defaultStrategy.OperationSampling = opS - } - - strategy, ok := h.parseOperationStrategy(s, opS) - if !ok { - continue - } - opS.PerOperationStrategies = append(opS.PerOperationStrategies, - &sampling.OperationSamplingStrategy{ - Operation: s.Operation, - ProbabilisticSampling: strategy.ProbabilisticSampling, - }) - } for _, s := range strategies.ServiceStrategies { h.serviceStrategies[s.Service] = h.parseServiceStrategies(s) @@ -182,6 +165,37 @@ func (h *strategyStore) parseServiceStrategies(strategy *serviceStrategy) *sampl return resp } +func (h *strategyStore) parseDefaultStrategy(strategy *defaultStrategy) *sampling.SamplingStrategyResponse { + resp := h.parseStrategy(&strategy.strategy) + if len(strategy.OperationStrategies) == 0 { + return resp + } + + opS := resp.OperationSampling + if opS == nil { + opS = &sampling.PerOperationSamplingStrategies{ + DefaultSamplingProbability: defaultSamplingProbability, + } + resp.OperationSampling = opS + } + + for _, s := range strategy.OperationStrategies { + strategy, ok := h.parseOperationStrategy(s, opS) + if !ok { + continue + } + + opS.PerOperationStrategies = append(opS.PerOperationStrategies, + &sampling.OperationSamplingStrategy{ + Operation: s.Operation, + ProbabilisticSampling: strategy.ProbabilisticSampling, + }) + } + + resp.OperationSampling = opS + return resp +} + func (h *strategyStore) parseOperationStrategy( strategy *operationStrategy, parent *sampling.PerOperationSamplingStrategies, @@ -218,7 +232,7 @@ func (h *strategyStore) parseStrategy(strategy *strategy) *sampling.SamplingStra } default: h.logger.Warn("Failed to parse sampling strategy", zap.Any("strategy", strategy)) - return deepCopy(&defaultStrategy) + return deepCopy(&defaultStrategyResponse) } } From af57fdaab42b2c1635905ecd30456282fa1580f0 Mon Sep 17 00:00:00 2001 From: Rutger Broekhoff Date: Sat, 21 Dec 2019 12:05:45 +0100 Subject: [PATCH 7/9] Fix tests Signed-off-by: Rutger Broekhoff --- .../static/fixtures/operation_strategies.json | 46 +++++++++---------- .../static/strategy_store_test.go | 1 + 2 files changed, 24 insertions(+), 23 deletions(-) diff --git a/plugin/sampling/strategystore/static/fixtures/operation_strategies.json b/plugin/sampling/strategystore/static/fixtures/operation_strategies.json index 1590fbf6374..8a1b7677aab 100644 --- a/plugin/sampling/strategystore/static/fixtures/operation_strategies.json +++ b/plugin/sampling/strategystore/static/fixtures/operation_strategies.json @@ -1,30 +1,30 @@ { "default_strategy": { "type": "probabilistic", - "param": 0.5 + "param": 0.5, + "operation_strategies": [ + { + "operation": "op0", + "type": "probabilistic", + "param": 0.2 + }, + { + "operation": "op6", + "type": "probabilistic", + "param": 0 + }, + { + "operation": "spam", + "type": "ratelimiting", + "param": 1 + }, + { + "operation": "op7", + "type": "probabilistic", + "param": 1 + } + ] }, - "default_operation_strategies": [ - { - "operation": "op0", - "type": "probabilistic", - "param": 0.2 - }, - { - "operation": "op6", - "type": "probabilistic", - "param": 0 - }, - { - "operation": "spam", - "type": "ratelimiting", - "param": 1 - }, - { - "operation": "op7", - "type": "probabilistic", - "param": 1 - } - ], "service_strategies": [ { "service": "foo", diff --git a/plugin/sampling/strategystore/static/strategy_store_test.go b/plugin/sampling/strategystore/static/strategy_store_test.go index a86fb769af6..9886c271fb6 100644 --- a/plugin/sampling/strategystore/static/strategy_store_test.go +++ b/plugin/sampling/strategystore/static/strategy_store_test.go @@ -115,6 +115,7 @@ func TestPerOperationSamplingStrategies(t *testing.T) { require.NoError(t, err) expectedRsp := makeResponse(sampling.SamplingStrategyType_PROBABILISTIC, 0.5) expectedRsp.OperationSampling = &sampling.PerOperationSamplingStrategies{ + DefaultSamplingProbability: 0.001, PerOperationStrategies: []*sampling.OperationSamplingStrategy{ { Operation: "op0", From 4bb92447202a361d05496ea0201d1a72bdac35ac Mon Sep 17 00:00:00 2001 From: Rutger Broekhoff Date: Mon, 23 Dec 2019 08:11:00 +0100 Subject: [PATCH 8/9] Apply suggested changes Signed-off-by: Rutger Broekhoff --- .../strategystore/static/constants.go | 10 +- .../sampling/strategystore/static/strategy.go | 8 +- .../strategystore/static/strategy_store.go | 98 +++++-------------- .../static/strategy_store_test.go | 22 ++--- 4 files changed, 44 insertions(+), 94 deletions(-) diff --git a/plugin/sampling/strategystore/static/constants.go b/plugin/sampling/strategystore/static/constants.go index 981fb749ac3..d5f7c44c56a 100644 --- a/plugin/sampling/strategystore/static/constants.go +++ b/plugin/sampling/strategystore/static/constants.go @@ -32,13 +32,13 @@ const ( defaultSamplingProbability = 0.001 ) -var ( - // defaultStrategy is the default sampling strategy the Strategy Store will return - // if none is provided. - defaultStrategyResponse = sampling.SamplingStrategyResponse{ +// defaultStrategy is the default sampling strategy the Strategy Store will return +// if none is provided. +func defaultStrategyResponse() *sampling.SamplingStrategyResponse { + return &sampling.SamplingStrategyResponse{ StrategyType: sampling.SamplingStrategyType_PROBABILISTIC, ProbabilisticSampling: &sampling.ProbabilisticSamplingStrategy{ SamplingRate: defaultSamplingProbability, }, } -) +} diff --git a/plugin/sampling/strategystore/static/strategy.go b/plugin/sampling/strategystore/static/strategy.go index 5503650e201..993f4de982a 100644 --- a/plugin/sampling/strategystore/static/strategy.go +++ b/plugin/sampling/strategystore/static/strategy.go @@ -34,14 +34,8 @@ type serviceStrategy struct { strategy } -// defaultStrategy defines the default strategy. -type defaultStrategy struct { - OperationStrategies []*operationStrategy `json:"operation_strategies"` - strategy -} - // strategies holds a default sampling strategy and service specific sampling strategies. type strategies struct { - DefaultStrategy *defaultStrategy `json:"default_strategy"` + DefaultStrategy *serviceStrategy `json:"default_strategy"` ServiceStrategies []*serviceStrategy `json:"service_strategies"` } diff --git a/plugin/sampling/strategystore/static/strategy_store.go b/plugin/sampling/strategystore/static/strategy_store.go index 09010899379..5e9698d06c1 100644 --- a/plugin/sampling/strategystore/static/strategy_store.go +++ b/plugin/sampling/strategystore/static/strategy_store.go @@ -19,14 +19,11 @@ import ( "encoding/gob" "encoding/json" "fmt" - "io/ioutil" - "sort" - - "github.com/pkg/errors" - "go.uber.org/zap" - ss "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore" "github.com/jaegertracing/jaeger/thrift-gen/sampling" + "github.com/pkg/errors" + "go.uber.org/zap" + "io/ioutil" ) type strategyStore struct { @@ -75,13 +72,19 @@ func loadStrategies(strategiesFile string) (*strategies, error) { } func (h *strategyStore) parseStrategies(strategies *strategies) { - h.defaultStrategy = &defaultStrategyResponse + h.defaultStrategy = defaultStrategyResponse() if strategies == nil { h.logger.Info("No sampling strategies provided, using defaults") return } if strategies.DefaultStrategy != nil { - h.defaultStrategy = h.parseDefaultStrategy(strategies.DefaultStrategy) + h.defaultStrategy = h.parseServiceStrategies(strategies.DefaultStrategy) + } + + merge := true + if h.defaultStrategy.OperationSampling == nil || + h.defaultStrategy.OperationSampling.PerOperationStrategies == nil { + merge = false } for _, s := range strategies.ServiceStrategies { @@ -92,17 +95,16 @@ func (h *strategyStore) parseStrategies(strategies *strategies) { // is not merged with and only used as a fallback). opS := h.serviceStrategies[s.Service].OperationSampling if opS == nil { - // It has no use to merge, just reference the default settings. + // Service has no per-operation strategies, so just reference the default settings. h.serviceStrategies[s.Service].OperationSampling = h.defaultStrategy.OperationSampling continue } - if h.defaultStrategy.OperationSampling == nil || - h.defaultStrategy.OperationSampling.PerOperationStrategies == nil { - continue + + if merge { + opS.PerOperationStrategies = mergePerOperationSamplingStrategies( + opS.PerOperationStrategies, + h.defaultStrategy.OperationSampling.PerOperationStrategies) } - opS.PerOperationStrategies = mergePerOperationSamplingStrategies( - opS.PerOperationStrategies, - h.defaultStrategy.OperationSampling.PerOperationStrategies) } } @@ -110,32 +112,17 @@ func (h *strategyStore) parseStrategies(strategies *strategies) { func mergePerOperationSamplingStrategies( a, b []*sampling.OperationSamplingStrategy, ) []*sampling.OperationSamplingStrategy { - // Guess the size of the slice of the two merged. - merged := make([]*sampling.OperationSamplingStrategy, 0, (len(a)+len(b))/4*3) - - ossLess := func(s []*sampling.OperationSamplingStrategy, i, j int) bool { - return s[i].Operation < s[j].Operation - } - sort.Slice(a, func(i, j int) bool { return ossLess(a, i, j) }) - sort.Slice(b, func(i, j int) bool { return ossLess(b, i, j) }) - - j := 0 - for i := range a { - // Increment j till b[j] > a[i], such that in the loop after the - // loop over a no remaining element of b with the same operation - // as a[i] is added to the merged slice. - for ; j < len(b) && b[j].Operation <= a[i].Operation; j++ { - if b[j].Operation < a[i].Operation { - merged = append(merged, b[j]) - } - } - merged = append(merged, a[i]) + m := make(map[string]bool) + for _, aOp := range a { + m[aOp.Operation] = true } - for ; j < len(b); j++ { - merged = append(merged, b[j]) + for _, bOp := range b { + if m[bOp.Operation] { + continue + } + a = append(a, bOp) } - - return merged + return a } func (h *strategyStore) parseServiceStrategies(strategy *serviceStrategy) *sampling.SamplingStrategyResponse { @@ -165,37 +152,6 @@ func (h *strategyStore) parseServiceStrategies(strategy *serviceStrategy) *sampl return resp } -func (h *strategyStore) parseDefaultStrategy(strategy *defaultStrategy) *sampling.SamplingStrategyResponse { - resp := h.parseStrategy(&strategy.strategy) - if len(strategy.OperationStrategies) == 0 { - return resp - } - - opS := resp.OperationSampling - if opS == nil { - opS = &sampling.PerOperationSamplingStrategies{ - DefaultSamplingProbability: defaultSamplingProbability, - } - resp.OperationSampling = opS - } - - for _, s := range strategy.OperationStrategies { - strategy, ok := h.parseOperationStrategy(s, opS) - if !ok { - continue - } - - opS.PerOperationStrategies = append(opS.PerOperationStrategies, - &sampling.OperationSamplingStrategy{ - Operation: s.Operation, - ProbabilisticSampling: strategy.ProbabilisticSampling, - }) - } - - resp.OperationSampling = opS - return resp -} - func (h *strategyStore) parseOperationStrategy( strategy *operationStrategy, parent *sampling.PerOperationSamplingStrategies, @@ -232,7 +188,7 @@ func (h *strategyStore) parseStrategy(strategy *strategy) *sampling.SamplingStra } default: h.logger.Warn("Failed to parse sampling strategy", zap.Any("strategy", strategy)) - return deepCopy(&defaultStrategyResponse) + return defaultStrategyResponse() } } diff --git a/plugin/sampling/strategystore/static/strategy_store_test.go b/plugin/sampling/strategystore/static/strategy_store_test.go index 9886c271fb6..61715842d5f 100644 --- a/plugin/sampling/strategystore/static/strategy_store_test.go +++ b/plugin/sampling/strategystore/static/strategy_store_test.go @@ -80,12 +80,12 @@ func TestPerOperationSamplingStrategies(t *testing.T) { assert.EqualValues(t, os.DefaultSamplingProbability, 0.8) require.Len(t, os.PerOperationStrategies, 4) fmt.Println(os) - assert.Equal(t, "op0", os.PerOperationStrategies[0].Operation) - assert.EqualValues(t, 0.2, os.PerOperationStrategies[0].ProbabilisticSampling.SamplingRate) + assert.Equal(t, "op6", os.PerOperationStrategies[0].Operation) + assert.EqualValues(t, 0.5, os.PerOperationStrategies[0].ProbabilisticSampling.SamplingRate) assert.Equal(t, "op1", os.PerOperationStrategies[1].Operation) assert.EqualValues(t, 0.2, os.PerOperationStrategies[1].ProbabilisticSampling.SamplingRate) - assert.Equal(t, "op6", os.PerOperationStrategies[2].Operation) - assert.EqualValues(t, 0.5, os.PerOperationStrategies[2].ProbabilisticSampling.SamplingRate) + assert.Equal(t, "op0", os.PerOperationStrategies[2].Operation) + assert.EqualValues(t, 0.2, os.PerOperationStrategies[2].ProbabilisticSampling.SamplingRate) assert.Equal(t, "op7", os.PerOperationStrategies[3].Operation) assert.EqualValues(t, 1, os.PerOperationStrategies[3].ProbabilisticSampling.SamplingRate) @@ -100,12 +100,12 @@ func TestPerOperationSamplingStrategies(t *testing.T) { os = s.OperationSampling assert.EqualValues(t, os.DefaultSamplingProbability, 0.001) require.Len(t, os.PerOperationStrategies, 5) - assert.Equal(t, "op0", os.PerOperationStrategies[0].Operation) - assert.EqualValues(t, 0.2, os.PerOperationStrategies[0].ProbabilisticSampling.SamplingRate) - assert.Equal(t, "op3", os.PerOperationStrategies[1].Operation) - assert.EqualValues(t, 0.3, os.PerOperationStrategies[1].ProbabilisticSampling.SamplingRate) - assert.Equal(t, "op5", os.PerOperationStrategies[2].Operation) - assert.EqualValues(t, 0.4, os.PerOperationStrategies[2].ProbabilisticSampling.SamplingRate) + assert.Equal(t, "op3", os.PerOperationStrategies[0].Operation) + assert.EqualValues(t, 0.3, os.PerOperationStrategies[0].ProbabilisticSampling.SamplingRate) + assert.Equal(t, "op5", os.PerOperationStrategies[1].Operation) + assert.EqualValues(t, 0.4, os.PerOperationStrategies[1].ProbabilisticSampling.SamplingRate) + assert.Equal(t, "op0", os.PerOperationStrategies[2].Operation) + assert.EqualValues(t, 0.2, os.PerOperationStrategies[2].ProbabilisticSampling.SamplingRate) assert.Equal(t, "op6", os.PerOperationStrategies[3].Operation) assert.EqualValues(t, 0, os.PerOperationStrategies[3].ProbabilisticSampling.SamplingRate) assert.Equal(t, "op7", os.PerOperationStrategies[4].Operation) @@ -115,7 +115,7 @@ func TestPerOperationSamplingStrategies(t *testing.T) { require.NoError(t, err) expectedRsp := makeResponse(sampling.SamplingStrategyType_PROBABILISTIC, 0.5) expectedRsp.OperationSampling = &sampling.PerOperationSamplingStrategies{ - DefaultSamplingProbability: 0.001, + DefaultSamplingProbability: 0.5, PerOperationStrategies: []*sampling.OperationSamplingStrategy{ { Operation: "op0", From 7830ebe613b503c5b591114e569ada2d6f4c4614 Mon Sep 17 00:00:00 2001 From: Rutger Broekhoff Date: Mon, 23 Dec 2019 09:34:35 +0100 Subject: [PATCH 9/9] Format Signed-off-by: Rutger Broekhoff --- plugin/sampling/strategystore/static/strategy_store.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/plugin/sampling/strategystore/static/strategy_store.go b/plugin/sampling/strategystore/static/strategy_store.go index 5e9698d06c1..6127ea8b391 100644 --- a/plugin/sampling/strategystore/static/strategy_store.go +++ b/plugin/sampling/strategystore/static/strategy_store.go @@ -19,11 +19,13 @@ import ( "encoding/gob" "encoding/json" "fmt" - ss "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore" - "github.com/jaegertracing/jaeger/thrift-gen/sampling" + "io/ioutil" + "github.com/pkg/errors" "go.uber.org/zap" - "io/ioutil" + + ss "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore" + "github.com/jaegertracing/jaeger/thrift-gen/sampling" ) type strategyStore struct {