Skip to content

Commit

Permalink
feature: recover ratelimiter first pass strategy
Browse files Browse the repository at this point in the history
  • Loading branch information
vuuihc committed Apr 1, 2022
1 parent ffc210a commit e9ccfcc
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 5 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
## [Unreleased]

### Fixed
- Fixed jaegerremote sampler not behaving properly with per operation startegy set. (#2137)
- Fixed jaegerremote sampler not behaving properly with per operation strategy set. (#2137)

## [1.6.0/0.31.0] - 2022-03-28

Expand Down
3 changes: 1 addition & 2 deletions samplers/jaegerremote/internal/utils/rate_limiter.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package utils // import "go.opentelemetry.io/contrib/samplers/jaegerremote/internal/utils"

import (
"math"
"sync"
"time"
)
Expand Down Expand Up @@ -53,7 +52,7 @@ type RateLimiter struct {
func NewRateLimiter(creditsPerSecond, maxBalance float64) *RateLimiter {
return &RateLimiter{
creditsPerSecond: creditsPerSecond,
balance: math.Min(creditsPerSecond, maxBalance),
balance: maxBalance,
maxBalance: maxBalance,
lastTick: time.Now(),
timeNow: time.Now,
Expand Down
12 changes: 10 additions & 2 deletions samplers/jaegerremote/sampler_remote_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,11 @@ func TestRemotelyControlledSampler_updateSampler(t *testing.T) {
assert.NotEqual(t, initSampler, sampler.sampler, "Sampler should have been updated")
assert.Equal(t, test.expectedDefaultProbability, s.defaultSampler.SamplingRate())

result := sampler.ShouldSample(makeSamplingParameters(testMaxID-10, testOperationName))
// First call is always sampled
result := sampler.ShouldSample(makeSamplingParameters(testMaxID+10, testOperationName))
assert.Equal(t, trace.RecordAndSample, result.Decision)

result = sampler.ShouldSample(makeSamplingParameters(testMaxID-10, testOperationName))
assert.Equal(t, trace.RecordAndSample, result.Decision)
})
}
Expand All @@ -285,6 +289,7 @@ func TestRemotelyControlledSampler_multiStrategyResponse(t *testing.T) {

defaultSampingRate := 1.0
testUnusedOpName := "unused_op"
testUnusedOpSamplingRate := 0.0

res := &jaeger_api_v2.SamplingStrategyResponse{
StrategyType: jaeger_api_v2.SamplingStrategyType_PROBABILISTIC,
Expand All @@ -296,7 +301,7 @@ func TestRemotelyControlledSampler_multiStrategyResponse(t *testing.T) {
{
Operation: testUnusedOpName,
ProbabilisticSampling: &jaeger_api_v2.ProbabilisticSamplingStrategy{
SamplingRate: 0.0,
SamplingRate: testUnusedOpSamplingRate,
}},
},
},
Expand All @@ -310,7 +315,10 @@ func TestRemotelyControlledSampler_multiStrategyResponse(t *testing.T) {
assert.Equal(t, defaultSampingRate, s.defaultSampler.SamplingRate())

result := sampler.ShouldSample(makeSamplingParameters(testMaxID-10, testUnusedOpName))
assert.Equal(t, trace.RecordAndSample, result.Decision) // first call always pass
result = sampler.ShouldSample(makeSamplingParameters(testMaxID, testUnusedOpName))
assert.Equal(t, trace.Drop, result.Decision)

}

func TestSamplerQueryError(t *testing.T) {
Expand Down
2 changes: 2 additions & 0 deletions samplers/jaegerremote/sampler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ func TestRateLimitingSampler(t *testing.T) {

sampler = newRateLimitingSampler(0.1)
result = sampler.ShouldSample(trace.SamplingParameters{Name: testOperationName})
assert.Equal(t, trace.RecordAndSample, result.Decision)
result = sampler.ShouldSample(trace.SamplingParameters{Name: testOperationName})
assert.Equal(t, trace.Drop, result.Decision)
}

Expand Down

0 comments on commit e9ccfcc

Please sign in to comment.