diff --git a/CHANGELOG.md b/CHANGELOG.md index 27d8fff9ce6..36e27daf064 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) + ## [1.17.0-rc.1/0.42.0-rc.1/0.11.0-rc.1] - 2023-05-17 ### Changed diff --git a/samplers/aws/xray/internal/reservoir.go b/samplers/aws/xray/internal/reservoir.go index 957b1f976c2..a554142b60c 100644 --- a/samplers/aws/xray/internal/reservoir.go +++ b/samplers/aws/xray/internal/reservoir.go @@ -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 } } } diff --git a/samplers/aws/xray/internal/reservoir_test.go b/samplers/aws/xray/internal/reservoir_test.go index a022d650c77..8e852d8f7ce 100644 --- a/samplers/aws/xray/internal/reservoir_test.go +++ b/samplers/aws/xray/internal/reservoir_test.go @@ -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) { @@ -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.