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.

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

XRay Remote Sampler - Cap quotaBalance to 1x quota #3652

Merged
merged 12 commits into from May 25, 2023
4 changes: 4 additions & 0 deletions CHANGELOG.md
Expand Up @@ -19,6 +19,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

- Use `strings.Cut()` instead of `string.SplitN()` for better readability and memory use. (#3822)

### Fixed

- AWS XRay Remote Sampling to cap quotaBalance to 1x quota in `go.opentelemetry.io/contrib/samplers/aws/xray`. (#3651, #3652)

MrAlias marked this conversation as resolved.
Show resolved Hide resolved
## [1.17.0-rc.1/0.42.0-rc.1/0.11.0-rc.1] - 2023-05-17

### Changed
Expand Down
8 changes: 4 additions & 4 deletions samplers/aws/xray/internal/reservoir.go
Expand Up @@ -106,11 +106,11 @@ func (r *reservoir) refreshQuotaBalanceLocked(now time.Time, borrowed bool) { //
r.quotaBalance += elapsedTime.Seconds()
}
} else {
totalQuotaBalanceCapacity := elapsedTime.Seconds() * r.capacity
r.quotaBalance += elapsedTime.Seconds() * r.quota
elapsedSeconds := elapsedTime.Seconds()
r.quotaBalance += elapsedSeconds * r.quota

if r.quotaBalance > totalQuotaBalanceCapacity {
r.quotaBalance = totalQuotaBalanceCapacity
if r.quotaBalance > r.quota {
r.quotaBalance = r.quota
}
}
}
25 changes: 13 additions & 12 deletions samplers/aws/xray/internal/reservoir_test.go
Expand Up @@ -152,13 +152,15 @@ func TestConsumeFromReservoir(t *testing.T) {
// once assigned quota is consumed reservoir does not allow to consume in that second
assert.False(t, r.take(clock.now(), false, 1.0))

// increase the clock by 5
// increase the clock by 4
clock.nowTime = 1500000005

// elapsedTime is 4 seconds so quota balance should be elapsedTime * quota = 8 and below take would consume 1 so
// ultimately 7
// reservoir updates the quotaBalance with one second worth of quota (even though 4 seconds have passed) and allows to consume
assert.Equal(t, r.quotaBalance, 0.0)
assert.True(t, r.take(clock.now(), false, 1.0))
assert.Equal(t, r.quotaBalance, 7.0)
assert.Equal(t, r.quotaBalance, 1.0)
assert.True(t, r.take(clock.now(), false, 1.0))
assert.Equal(t, r.quotaBalance, 0.0)
}

func TestZeroCapacityFailBorrow(t *testing.T) {
Expand Down Expand Up @@ -213,23 +215,22 @@ func TestResetQuotaUsageRotation(t *testing.T) {
assert.True(t, r.take(clock.now(), false, 1.0))
}

// assert that when quotaBalance exceeds totalQuotaBalanceCapacity then totalQuotaBalanceCapacity
// gets assigned to quotaBalance.
func TestQuotaBalanceNonBorrowExceedsCapacity(t *testing.T) {
// assert that when quotaBalance is assigned the correct value after a portion of a second.
func TestQuotaBalanceAfterPortionOfSecond(t *testing.T) {
clock := &mockClock{
nowTime: 1500000001,
nowTime: 1500000002,
}

r := &reservoir{
quota: 6,
capacity: 5,
lastTick: time.Unix(1500000000, 0),
capacity: 6,
lastTick: time.Unix(1500000001, 500000000),
}

r.refreshQuotaBalanceLocked(clock.now(), false)

// assert that if quotaBalance exceeds capacity then total capacity would be new quotaBalance
assert.Equal(t, r.quotaBalance, r.capacity)
// assert that after half a second, quotaBalance is now quota*0.5 = 3
assert.Equal(t, r.quotaBalance, 3.0)
}

// assert quotaBalance and capacity of borrowing case.
Expand Down