Skip to content

Commit

Permalink
feature: use peroperationsamplerupdater first (#2137)
Browse files Browse the repository at this point in the history
* feature: use peroperationsamplerupdater first; ratelimiter balance init with min(maxbalance, creditspersecond)

* feautre: polish ratelimiter test and add TestRemotelyControlledSampler_multiStrategyResponse

* feature: update changelog regarding PR #2137

* feature: recover ratelimiter first pass strategy

* feature: fix tests of withUpdaters

Co-authored-by: Chester Cheung <[email protected]>
  • Loading branch information
vuuihc and hanyuancheung authored Apr 6, 2022
1 parent 8cf7954 commit 1e34aa3
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 7 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ 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 strategy set. (#2137)

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

### Added
Expand Down
10 changes: 4 additions & 6 deletions samplers/jaegerremote/sampler_remote_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,10 @@ func newConfig(options ...Option) config {
for _, option := range options {
option.apply(&c)
}
c.updaters = append(c.updaters,
&perOperationSamplerUpdater{
MaxOperations: c.posParams.MaxOperations,
OperationNameLateBinding: c.posParams.OperationNameLateBinding,
})

c.updaters = append([]samplerUpdater{&perOperationSamplerUpdater{
MaxOperations: c.posParams.MaxOperations,
OperationNameLateBinding: c.posParams.OperationNameLateBinding,
}}, c.updaters...)
return c
}

Expand Down
41 changes: 40 additions & 1 deletion samplers/jaegerremote/sampler_remote_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func TestRemoteSamplerOptions(t *testing.T) {
assert.Equal(t, 42*time.Second, sampler.samplingRefreshInterval)
assert.Same(t, fetcher, sampler.samplingFetcher)
assert.Same(t, parser, sampler.samplingParser)
assert.Same(t, updaters[0], sampler.updaters[0])
assert.EqualValues(t, sampler.updaters[0], &perOperationSamplerUpdater{MaxOperations: 42, OperationNameLateBinding: true})
}

func TestRemoteSamplerOptionsDefaults(t *testing.T) {
Expand Down Expand Up @@ -281,6 +281,45 @@ func TestRemotelyControlledSampler_updateSampler(t *testing.T) {
}
}

func TestRemotelyControlledSampler_multiStrategyResponse(t *testing.T) {
agent, sampler := initAgent(t)
defer agent.Close()
initSampler, ok := sampler.sampler.(*probabilisticSampler)
assert.True(t, ok)

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

res := &jaeger_api_v2.SamplingStrategyResponse{
StrategyType: jaeger_api_v2.SamplingStrategyType_PROBABILISTIC,
ProbabilisticSampling: &jaeger_api_v2.ProbabilisticSamplingStrategy{SamplingRate: defaultSampingRate},
OperationSampling: &jaeger_api_v2.PerOperationSamplingStrategies{
DefaultSamplingProbability: defaultSampingRate,
DefaultLowerBoundTracesPerSecond: 0.001,
PerOperationStrategies: []*jaeger_api_v2.OperationSamplingStrategy{
{
Operation: testUnusedOpName,
ProbabilisticSampling: &jaeger_api_v2.ProbabilisticSamplingStrategy{
SamplingRate: testUnusedOpSamplingRate,
}},
},
},
}

agent.AddSamplingStrategy("client app", res)
sampler.UpdateSampler()
s, ok := sampler.sampler.(*perOperationSampler)
assert.True(t, ok)
assert.NotEqual(t, initSampler, sampler.sampler, "Sampler should have been updated")
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) {
agent, sampler := initAgent(t)
defer agent.Close()
Expand Down

0 comments on commit 1e34aa3

Please sign in to comment.