Skip to content

Commit

Permalink
fix: increase the possible random numbers used for stickiness id (#241)
Browse files Browse the repository at this point in the history
This update changes how we calculate random percentages in the SDK. Instead of multiplying a random int with 100 (which absolutely could cause overflows), we now use a ranged `Next` call, to generate one of 10k different numbers. This is in line with how we get the random number in other sdks.

I've added a test to ensure that random distribution works to within the accepted deviation.
  • Loading branch information
thomasheartman authored Sep 24, 2024
1 parent 07578e6 commit f682075
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/Unleash/Strategies/FlexibleRolloutStrategy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public class FlexibleRolloutStrategy : IStrategy
public FlexibleRolloutStrategy()
{
var random = new Random();
randomGenerator = () => (random.Next() * 100).ToString();
randomGenerator = () => random.Next(1, 10001).ToString();
}

public FlexibleRolloutStrategy(Func<string> randomGenerator)
Expand Down
38 changes: 38 additions & 0 deletions tests/Unleash.Tests/Strategy/FlexibleRolloutStrategyTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -361,5 +361,43 @@ public void Should_be_enabled_for_empty_userid()
// Assert
enabled.Should().BeTrue();
}

[Test]
public void Should_only_at_most_miss_by_one_percent()
{
// Arrange
var strategy = new FlexibleRolloutStrategy();
var percentage = 25;
var groupId = "groupId";
var rounds = 200_000;
var enabledCount = 0;

// Act
for (int i = 0; i < rounds; i++)
{
var parameters = new Dictionary<string, string>
{
{ "rollout", percentage.ToString() },
{ "groupId", groupId }
};
var context = new UnleashContext
{
SessionId = i.ToString()
};

if (strategy.IsEnabled(parameters, context))
{
enabledCount++;
}
}

var actualPercentage = (int)Math.Round((enabledCount / (double)rounds) * 100);
var highMark = percentage + 1;
var lowMark = percentage - 1;

// Assert
actualPercentage.Should().BeGreaterOrEqualTo(lowMark);
actualPercentage.Should().BeLessOrEqualTo(highMark);
}
}
}

0 comments on commit f682075

Please sign in to comment.