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

fix: add a seed to variant utils murmur hashing to fix variant distri… #171

Merged
merged 1 commit into from
Oct 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Unleash/Strategies/FlexibleRolloutStrategy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public bool IsEnabled(Dictionary<string, string> parameters, UnleashContext cont

if (!string.IsNullOrEmpty(stickinessId))
{
var normalizedUserId = StrategyUtils.GetNormalizedNumber(stickinessId, groupId);
var normalizedUserId = StrategyUtils.GetNormalizedNumber(stickinessId, groupId, 0);
return percentage > 0 && normalizedUserId <= percentage;
}
else
Expand Down
2 changes: 1 addition & 1 deletion src/Unleash/Strategies/GradualRolloutSessionIdStrategy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public bool IsEnabled(Dictionary<string, string> parameters, UnleashContext cont
var percentage = StrategyUtils.GetPercentage(percentageString);
var groupId = parameters[GroupId] ?? string.Empty;

var normalizedSessionId = StrategyUtils.GetNormalizedNumber(sessionId, groupId);
var normalizedSessionId = StrategyUtils.GetNormalizedNumber(sessionId, groupId, 0);

return percentage > 0 && normalizedSessionId <= percentage;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Unleash/Strategies/GradualRolloutUserIdStrategy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public bool IsEnabled(Dictionary<string, string> parameters, UnleashContext cont
var percentage = StrategyUtils.GetPercentage(percentageString);
var groupId = parameters[GroupIdConst] ?? string.Empty;

var normalizedUserId = StrategyUtils.GetNormalizedNumber(userId, groupId);
var normalizedUserId = StrategyUtils.GetNormalizedNumber(userId, groupId, 0);

return percentage > 0 && normalizedUserId <= percentage;
}
Expand Down
4 changes: 2 additions & 2 deletions src/Unleash/Strategies/StrategyUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ internal class StrategyUtils
/// <summary>
/// Takes to string inputs concat them, produce a hashCode and return a normalized value between 0 and 100;
/// </summary>
public static int GetNormalizedNumber(string identifier, string groupId, int normalizer = 100)
public static int GetNormalizedNumber(string identifier, string groupId, uint randomSeed, int normalizer = 100)
gastonfournier marked this conversation as resolved.
Show resolved Hide resolved
{
const int one = 1;
const string separator = ":";

byte[] bytes = Encoding.UTF8.GetBytes(string.Concat(groupId, separator, identifier));

using (var algorithm = MurmurHash.Create32())
using (var algorithm = MurmurHash.Create32(randomSeed))
{
var hash = algorithm.ComputeHash(bytes);
var value = BitConverter.ToUInt32(hash, 0);
Expand Down
4 changes: 3 additions & 1 deletion src/Unleash/Variants/VariantUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ namespace Unleash.Variants
{
internal class VariantUtils
{
public static readonly uint VARIANT_NORMALIZATION_SEED = 86028157;

public static Variant SelectVariant(string groupId, UnleashContext context, List<VariantDefinition> variantDefinitions)
{
var totalWeight = variantDefinitions.Sum(v => v.Weight);
Expand All @@ -23,7 +25,7 @@ public static Variant SelectVariant(string groupId, UnleashContext context, List
}

var stickiness = variantDefinitions[0].Stickiness ?? "default";
var target = StrategyUtils.GetNormalizedNumber(GetIdentifier(context, stickiness), groupId, totalWeight);
var target = StrategyUtils.GetNormalizedNumber(GetIdentifier(context, stickiness), groupId, VARIANT_NORMALIZATION_SEED, totalWeight);

var counter = 0;
foreach (var variantDefinition in variantDefinitions)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ static TestFactory()

using (var client = new HttpClient())
{
var csTestsVersion = "v4.5.2";
var csTestsVersion = "v5.0.2";
var indexPath = $"https://raw.githubusercontent.com/Unleash/client-specification/{csTestsVersion}/specifications/";
var indexResponse = client.GetStringAsync(indexPath + "index.json").Result;
var indexFilePath = Path.Combine(specificationsPath, "index.json");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public void should_be_enabled_above_minimum_percentage()
{
string sessionId = "1574576830";
string groupId = "";
int minimumPercentage = StrategyUtils.GetNormalizedNumber(sessionId, groupId);
int minimumPercentage = StrategyUtils.GetNormalizedNumber(sessionId, groupId, 0);

var context = UnleashContext.New().SessionId(sessionId).Build();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public void should_be_enabled_above_minimum_percentage()
{
var userId = "1574576830";
var groupId = "";
var minimumPercentage = StrategyUtils.GetNormalizedNumber(userId, groupId);
var minimumPercentage = StrategyUtils.GetNormalizedNumber(userId, groupId, 0);

var context = UnleashContext.New().UserId(userId).Build();
var gradualRolloutStrategy = new GradualRolloutUserIdStrategy();
Expand Down
12 changes: 6 additions & 6 deletions tests/Unleash.Tests/Strategy/StrategyUtilsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,19 @@ public void GetPercentageVariants()
public void GetNormalizedNumber_Variants()
{
// Normal cases
StrategyUtils.GetNormalizedNumber("user1", "group1").Should().BeInRange(0, 100);
StrategyUtils.GetNormalizedNumber("user1", "group1", 0).Should().BeInRange(0, 100);

// Strange inputs
StrategyUtils.GetNormalizedNumber(null, null).Should().BeInRange(0, 100);
StrategyUtils.GetNormalizedNumber("", "").Should().BeInRange(0, 100);
StrategyUtils.GetNormalizedNumber("#%&/(", "§~:<>&nbsp;").Should().BeInRange(0, 100);
StrategyUtils.GetNormalizedNumber(null, null, 0).Should().BeInRange(0, 100);
StrategyUtils.GetNormalizedNumber("", "", 0).Should().BeInRange(0, 100);
StrategyUtils.GetNormalizedNumber("#%&/(", "§~:<>&nbsp;", 0).Should().BeInRange(0, 100);
}

[Test]
public void GetNormalizedNumber_Is_Compatible_With_Java_And_Go_Implementations()
{
Assert.AreEqual(73, StrategyUtils.GetNormalizedNumber("123", "gr1"));
Assert.AreEqual(25, StrategyUtils.GetNormalizedNumber("999", "groupX"));
Assert.AreEqual(73, StrategyUtils.GetNormalizedNumber("123", "gr1", 0));
Assert.AreEqual(25, StrategyUtils.GetNormalizedNumber("999", "groupX", 0));
}
}
}
20 changes: 10 additions & 10 deletions tests/Unleash.Tests/Variants/VariantUtilsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public void ShouldReturnVariant2()

var context = new UnleashContext
{
UserId = "163",
UserId = "15",
SessionId = "sessionId",
RemoteAddress = "remoteAddress",
Properties = new Dictionary<string, string>()
Expand Down Expand Up @@ -114,7 +114,7 @@ public void ShouldReturnVariant3()

var context = new UnleashContext
{
UserId = "40",
UserId = "43",
SessionId = "sessionId",
RemoteAddress = "remoteAddress",
Properties = new Dictionary<string, string>()
Expand Down Expand Up @@ -261,7 +261,7 @@ public void ShouldReturnVariantOverrideOnSessionId()
}

[Test]
public void Custom_Stickiness_CustomField_528_Yields_Blue()
public void Custom_Stickiness_CustomField_419_Yields_Blue()
{
// Arrange
var sessionId = "122221";
Expand All @@ -284,7 +284,7 @@ public void Custom_Stickiness_CustomField_528_Yields_Blue()
UserId = "11",
SessionId = sessionId,
RemoteAddress = "remoteAddress",
Properties = new Dictionary<string, string> { { "env", "prod" }, { "customField", "528" } }
Properties = new Dictionary<string, string> { { "env", "prod" }, { "customField", "419" } }
};

// Act
Expand All @@ -295,7 +295,7 @@ public void Custom_Stickiness_CustomField_528_Yields_Blue()
}

[Test]
public void Custom_Stickiness_CustomField_16_Yields_Blue()
public void Custom_Stickiness_CustomField_21_Yields_Blue()
{
// Arrange
var sessionId = "122221";
Expand All @@ -318,7 +318,7 @@ public void Custom_Stickiness_CustomField_16_Yields_Blue()
UserId = "13",
SessionId = sessionId,
RemoteAddress = "remoteAddress",
Properties = new Dictionary<string, string> { { "env", "prod" }, { "customField", "16" } }
Properties = new Dictionary<string, string> { { "env", "prod" }, { "customField", "21" } }
};

// Act
Expand All @@ -329,7 +329,7 @@ public void Custom_Stickiness_CustomField_16_Yields_Blue()
}

[Test]
public void Custom_Stickiness_CustomField_198_Yields_Red()
public void Custom_Stickiness_CustomField_60_Yields_Red()
{
// Arrange
var sessionId = "122221";
Expand All @@ -352,7 +352,7 @@ public void Custom_Stickiness_CustomField_198_Yields_Red()
UserId = "13",
SessionId = sessionId,
RemoteAddress = "remoteAddress",
Properties = new Dictionary<string, string> { { "env", "prod" }, { "customField", "198" } }
Properties = new Dictionary<string, string> { { "env", "prod" }, { "customField", "60" } }
};

// Act
Expand Down Expand Up @@ -397,7 +397,7 @@ public void Custom_Stickiness_CustomField_43_Yields_Green()
}

[Test]
public void Custom_Stickiness_CustomField_112_Yields_Yellow()
public void Custom_Stickiness_CustomField_13_Yields_Yellow()
{
// Arrange
var sessionId = "122221";
Expand All @@ -420,7 +420,7 @@ public void Custom_Stickiness_CustomField_112_Yields_Yellow()
UserId = "13",
SessionId = sessionId,
RemoteAddress = "remoteAddress",
Properties = new Dictionary<string, string> { { "env", "prod" }, { "customField", "112" } }
Properties = new Dictionary<string, string> { { "env", "prod" }, { "customField", "13" } }
};

// Act
Expand Down
Loading