From 17535bf7a4e40a01364f65a4271c751c7894085a Mon Sep 17 00:00:00 2001 From: vikrambe Date: Sun, 22 Aug 2021 21:07:44 +0530 Subject: [PATCH] Fixing review comments --- processor/tailsamplingprocessor/README.md | 2 +- processor/tailsamplingprocessor/composite_helper.go | 11 +++++------ .../internal/sampling/composite_test.go | 8 ++++---- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/processor/tailsamplingprocessor/README.md b/processor/tailsamplingprocessor/README.md index ea820f27faae..8b78be20ef00 100644 --- a/processor/tailsamplingprocessor/README.md +++ b/processor/tailsamplingprocessor/README.md @@ -22,7 +22,7 @@ Multiple policies exist today and it is straight forward to add more. These incl - `composite`: Sample based on a combination of above samplers, with ordering and rate allocation per sampler. Rate allocation allocates certain percentages of spans per policy order. For example if we have set max_total_spans_per_second as 100 then we can set rate_allocation as follows 1. test-composite-policy-1 = 50 % of max_total_spans_per_second = 50 spans_per_second - 2. test-composite-policy-1 = 25 % of max_total_spans_per_second = 25 spans_per_second + 2. test-composite-policy-2 = 25 % of max_total_spans_per_second = 25 spans_per_second 3. To ensure remaining capacity is filled use always_sample as one of the policies The following configuration options can also be modified: diff --git a/processor/tailsamplingprocessor/composite_helper.go b/processor/tailsamplingprocessor/composite_helper.go index f9d72ab30314..ae5a7f06aa4a 100644 --- a/processor/tailsamplingprocessor/composite_helper.go +++ b/processor/tailsamplingprocessor/composite_helper.go @@ -25,9 +25,8 @@ import ( func getNewCompositePolicy(logger *zap.Logger, config CompositeCfg) (sampling.PolicyEvaluator, error) { var subPolicyEvalParams []sampling.SubPolicyEvalParams rateAllocationsMap := getRateAllocationMap(config) - for i := range config.SubPolicyCfg { - policyCfg := &config.SubPolicyCfg[i] - policy, _ := getSubPolicyEvaluator(logger, policyCfg) + for _, policyCfg := range config.SubPolicyCfg { + policy, _ := getSubPolicyEvaluator(logger, &policyCfg) evalParams := sampling.SubPolicyEvalParams{ Evaluator: policy, @@ -44,11 +43,11 @@ func getRateAllocationMap(config CompositeCfg) map[string]float64 { maxTotalSPS := float64(config.MaxTotalSpansPerSecond) // Default SPS determined by equally diving number of sub policies defaultSPS := maxTotalSPS / float64(len(config.SubPolicyCfg)) - for i := 0; i < len(config.RateAllocation); i++ { - rAlloc := &config.RateAllocation[i] - rateAllocationsMap[rAlloc.Policy] = defaultSPS + for _, rAlloc := range config.RateAllocation{ if rAlloc.Percent > 0 { rateAllocationsMap[rAlloc.Policy] = (float64(rAlloc.Percent) / 100) * maxTotalSPS + }else { + rateAllocationsMap[rAlloc.Policy] = defaultSPS } } return rateAllocationsMap diff --git a/processor/tailsamplingprocessor/internal/sampling/composite_test.go b/processor/tailsamplingprocessor/internal/sampling/composite_test.go index 19242b034daa..8df9eb215dfe 100644 --- a/processor/tailsamplingprocessor/internal/sampling/composite_test.go +++ b/processor/tailsamplingprocessor/internal/sampling/composite_test.go @@ -107,7 +107,7 @@ func TestCompositeEvaluatorThrottling(t *testing.T) { // Create only one subpolicy, with 100% Sampled policy. n1 := NewAlwaysSample(zap.NewNop()) timeProvider := &FakeTimeProvider{second: 0} - const totalSPS = 100 + const totalSPS = 10 c := NewComposite(zap.NewNop(), totalSPS, []SubPolicyEvalParams{{n1, totalSPS}}, timeProvider) trace := createTrace() @@ -159,10 +159,10 @@ func TestOnLateArrivingSpans_Composite(t *testing.T) { n1 := NewNumericAttributeFilter(zap.NewNop(), "tag", 0, 100) n2 := NewAlwaysSample(zap.NewNop()) timeProvider := &FakeTimeProvider{second: 0} - const totalSPS = 100 + const totalSPS = 10 c := NewComposite(zap.NewNop(), totalSPS, []SubPolicyEvalParams{{n1, totalSPS / 2}, {n2, totalSPS / 2}}, timeProvider) e := c.OnLateArrivingSpans(Sampled, nil) - assert.Nil(t, e) + assert.NoError(t, e) } func TestCompositeEvaluator2SubpolicyThrottling(t *testing.T) { @@ -170,7 +170,7 @@ func TestCompositeEvaluator2SubpolicyThrottling(t *testing.T) { n1 := NewNumericAttributeFilter(zap.NewNop(), "tag", 0, 100) n2 := NewAlwaysSample(zap.NewNop()) timeProvider := &FakeTimeProvider{second: 0} - const totalSPS = 100 + const totalSPS = 10 c := NewComposite(zap.NewNop(), totalSPS, []SubPolicyEvalParams{{n1, totalSPS / 2}, {n2, totalSPS / 2}}, timeProvider) trace := createTrace()