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

* Cap quotaBalance to 1x quota

* add changelog

* update CHANGELOG

* update CHANGELOG

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

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
 		}
 	}
 }

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.