From ef0bfd48cce2b0c8528e723e761a9c40c2ad1ec6 Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Mon, 9 Sep 2024 17:22:23 -0700 Subject: [PATCH 1/8] Working allocationId --- .../FeatureManager.cs | 45 +---- .../Telemetry/TelemetryEventHandler.cs | 154 ++++++++++++++++++ 2 files changed, 155 insertions(+), 44 deletions(-) create mode 100644 src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs diff --git a/src/Microsoft.FeatureManagement/FeatureManager.cs b/src/Microsoft.FeatureManagement/FeatureManager.cs index 2661bf88..eb8d54d5 100644 --- a/src/Microsoft.FeatureManagement/FeatureManager.cs +++ b/src/Microsoft.FeatureManagement/FeatureManager.cs @@ -373,56 +373,13 @@ private async ValueTask EvaluateFeature(string featur Activity.Current != null && Activity.Current.IsAllDataRequested) { - AddEvaluationActivityEvent(evaluationEvent); + TelemetryEventHandler.HandleEvaluationEvent(evaluationEvent, Logger); } } return evaluationEvent; } - private void AddEvaluationActivityEvent(EvaluationEvent evaluationEvent) - { - Debug.Assert(evaluationEvent != null); - Debug.Assert(evaluationEvent.FeatureDefinition != null); - - var tags = new ActivityTagsCollection() - { - { "FeatureName", evaluationEvent.FeatureDefinition.Name }, - { "Enabled", evaluationEvent.Enabled }, - { "VariantAssignmentReason", evaluationEvent.VariantAssignmentReason }, - { "Version", ActivitySource.Version } - }; - - if (!string.IsNullOrEmpty(evaluationEvent.TargetingContext?.UserId)) - { - tags["TargetingId"] = evaluationEvent.TargetingContext.UserId; - } - - if (!string.IsNullOrEmpty(evaluationEvent.Variant?.Name)) - { - tags["Variant"] = evaluationEvent.Variant.Name; - } - - if (evaluationEvent.FeatureDefinition.Telemetry.Metadata != null) - { - foreach (KeyValuePair kvp in evaluationEvent.FeatureDefinition.Telemetry.Metadata) - { - if (tags.ContainsKey(kvp.Key)) - { - Logger?.LogWarning($"{kvp.Key} from telemetry metadata will be ignored, as it would override an existing key."); - - continue; - } - - tags[kvp.Key] = kvp.Value; - } - } - - var activityEvent = new ActivityEvent("FeatureFlag", DateTimeOffset.UtcNow, tags); - - Activity.Current.AddEvent(activityEvent); - } - private async ValueTask IsEnabledAsync(FeatureDefinition featureDefinition, TContext appContext, bool useAppContext, CancellationToken cancellationToken) { Debug.Assert(featureDefinition != null); diff --git a/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs b/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs new file mode 100644 index 00000000..b969629e --- /dev/null +++ b/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs @@ -0,0 +1,154 @@ +using Microsoft.Extensions.Logging; +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using System.Security.Cryptography; +using System.Text; +using System.Web; + +namespace Microsoft.FeatureManagement.Telemetry +{ + internal static class TelemetryEventHandler + { + private static readonly string EvaluationEventVersion = "1.0.0"; + + /// + /// Handles an evaluation event by adding it as an activity event to the current Activity. + /// + /// The to publish as an + /// Optional logger to log warnings to + public static void HandleEvaluationEvent(EvaluationEvent evaluationEvent, ILogger logger) + { + Debug.Assert(evaluationEvent != null); + Debug.Assert(evaluationEvent.FeatureDefinition != null); + + var tags = new ActivityTagsCollection() + { + { "FeatureName", evaluationEvent.FeatureDefinition.Name }, + { "Enabled", evaluationEvent.Enabled }, + { "VariantAssignmentReason", evaluationEvent.VariantAssignmentReason }, + { "Version", EvaluationEventVersion } + }; + + if (!string.IsNullOrEmpty(evaluationEvent.TargetingContext?.UserId)) + { + tags["TargetingId"] = evaluationEvent.TargetingContext.UserId; + } + + if (!string.IsNullOrEmpty(evaluationEvent.Variant?.Name)) + { + tags["Variant"] = evaluationEvent.Variant.Name; + } + + if (evaluationEvent.FeatureDefinition.Telemetry.Metadata != null) + { + foreach (KeyValuePair kvp in evaluationEvent.FeatureDefinition.Telemetry.Metadata) + { + if (tags.ContainsKey(kvp.Key)) + { + logger?.LogWarning($"{kvp.Key} from telemetry metadata will be ignored, as it would override an existing key."); + + continue; + } + + tags[kvp.Key] = kvp.Value; + } + } + + // VariantAllocationPercentage + if (evaluationEvent.FeatureDefinition.Allocation?.Percentile != null) + { + tags["VariantAssignmentPercentage"] = evaluationEvent.FeatureDefinition.Allocation.Percentile + .Where(p => p.Variant == evaluationEvent.Variant.Name) + .Sum(p => p.To - p.From); + } + + // DefaultWhenEnabled + if (evaluationEvent.FeatureDefinition.Allocation?.DefaultWhenEnabled != null) + { + tags["DefaultWhenEnabled"] = evaluationEvent.FeatureDefinition.Allocation.DefaultWhenEnabled; + } + + // AllocationId + string allocationId = GenerateAllocationId(evaluationEvent.FeatureDefinition); + + if (allocationId != null) + { + tags["AllocationId"] = allocationId; + } + + var activityEvent = new ActivityEvent("FeatureFlag", DateTimeOffset.UtcNow, tags); + + Activity.Current.AddEvent(activityEvent); + } + + private static string GenerateAllocationId(FeatureDefinition featureDefinition) + { + StringBuilder inputBuilder = new StringBuilder(); + + // Seed + inputBuilder.Append($"seed={featureDefinition.Allocation?.Seed ?? ""}"); + + var allocatedVariants = new HashSet(); + + // DefaultWhenEnabled + if (featureDefinition.Allocation?.DefaultWhenEnabled != null) + { + allocatedVariants.Add(featureDefinition.Allocation.DefaultWhenEnabled); + } + + inputBuilder.Append("\n"); + inputBuilder.Append($"default_when_enabled={featureDefinition.Allocation?.DefaultWhenEnabled ?? ""}"); + + // Percentiles + inputBuilder.Append("\n"); + inputBuilder.Append("percentiles="); + + if (featureDefinition.Allocation?.Percentile != null && featureDefinition.Allocation.Percentile.Any()) + { + var sortedPercentiles = featureDefinition.Allocation.Percentile + .Where(p => p.From != p.To) + .OrderBy(p => p.From) + .ToList(); + + allocatedVariants.UnionWith(sortedPercentiles.Select(p => p.Variant)); + + inputBuilder.Append(string.Join(";", sortedPercentiles.Select(p => $"{p.From},{p.Variant},{p.To}"))); + } + + // Variants + inputBuilder.Append("\n"); + inputBuilder.Append("variants="); + + if (allocatedVariants.Any() && featureDefinition.Variants != null && featureDefinition.Variants.Any()) + { + var sortedVariants = featureDefinition.Variants + .Where(variant => allocatedVariants.Contains(variant.Name)) + .OrderBy(variant => variant.Name) + .ToList(); + + inputBuilder.Append(string.Join(";", sortedVariants.Select(v => $"{v.Name},{v.ConfigurationValue?.Value}"))); + } + + // If there's not a special seed and no variants allocated, return null + if (featureDefinition.Allocation?.Seed == null && + !allocatedVariants.Any()) + { + return null; + } + + // Example input string + // input == "seed=123abc\ndefault_when_enabled=Control\npercentiles=0,Control,20;20,Test,100\nvariants=Control,standard;Test,special" + string input = inputBuilder.ToString(); + + using (SHA256 sha256 = SHA256.Create()) + { + byte[] hash = sha256.ComputeHash(Encoding.UTF8.GetBytes(input)); + byte[] truncatedHash = new byte[15]; + Array.Copy(hash, truncatedHash, 15); + return Convert.ToBase64String(truncatedHash); + } + } + } +} From 27330d741f30ce187f210da0c8d2f49035a3d7a1 Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Tue, 10 Sep 2024 17:34:23 -0700 Subject: [PATCH 2/8] Adds Base64Url byte extension, adjusts TelemetryEventHandler logic, adjusts and adds tests. --- .../Extensions/BytesExtensions.cs | 43 +++++++++++++++++++ .../Telemetry/TelemetryEventHandler.cs | 28 +++++++++--- .../FeatureManagementTest.cs | 42 ++++++++++++++++++ tests/Tests.FeatureManagement/Features.cs | 1 + .../Tests.FeatureManagement/appsettings.json | 16 +++++++ 5 files changed, 125 insertions(+), 5 deletions(-) create mode 100644 src/Microsoft.FeatureManagement/Extensions/BytesExtensions.cs diff --git a/src/Microsoft.FeatureManagement/Extensions/BytesExtensions.cs b/src/Microsoft.FeatureManagement/Extensions/BytesExtensions.cs new file mode 100644 index 00000000..44f697d9 --- /dev/null +++ b/src/Microsoft.FeatureManagement/Extensions/BytesExtensions.cs @@ -0,0 +1,43 @@ +using System; +using System.Text; + +namespace Microsoft.FeatureManagement.Extensions +{ + internal static class BytesExtensions + { + /// + /// Converts a byte array to Base64URL string with optional padding ('=') characters removed. + /// Base64 description: https://datatracker.ietf.org/doc/html/rfc4648.html#section-4 + /// + public static string ToBase64Url(this byte[] bytes) + { + string bytesBase64 = Convert.ToBase64String(bytes); + + int indexOfEquals = bytesBase64.IndexOf("="); + + // Skip the optional padding of '=' characters based on the Base64Url spec if any are present from the Base64 conversion + int stringBuilderCapacity = indexOfEquals != -1 ? indexOfEquals : bytesBase64.Length; + + StringBuilder stringBuilder = new StringBuilder(stringBuilderCapacity); + + // Construct Base64URL string by replacing characters in Base64 conversion that are not URL safe + for (int i = 0; i < stringBuilderCapacity; i++) + { + if (bytesBase64[i] == '+') + { + stringBuilder.Append('-'); + } + else if (bytesBase64[i] == '/') + { + stringBuilder.Append('_'); + } + else + { + stringBuilder.Append(bytesBase64[i]); + } + } + + return stringBuilder.ToString(); + } + } +} diff --git a/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs b/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs index b969629e..db7b769c 100644 --- a/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs +++ b/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs @@ -1,4 +1,5 @@ using Microsoft.Extensions.Logging; +using Microsoft.FeatureManagement.Extensions; using System; using System.Collections.Generic; using System.Diagnostics; @@ -57,11 +58,28 @@ public static void HandleEvaluationEvent(EvaluationEvent evaluationEvent, ILogge } // VariantAllocationPercentage - if (evaluationEvent.FeatureDefinition.Allocation?.Percentile != null) + if (evaluationEvent.VariantAssignmentReason == VariantAssignmentReason.DefaultWhenEnabled) { - tags["VariantAssignmentPercentage"] = evaluationEvent.FeatureDefinition.Allocation.Percentile - .Where(p => p.Variant == evaluationEvent.Variant.Name) - .Sum(p => p.To - p.From); + // If the variant was assigned due to DefaultWhenEnabled, the percentage is 100% - all allocated percentiles + double allocatedPercentage = 0; + + if (evaluationEvent.FeatureDefinition.Allocation?.Percentile != null) + { + allocatedPercentage += evaluationEvent.FeatureDefinition.Allocation.Percentile + .Sum(p => p.To - p.From); + } + + tags["VariantAssignmentPercentage"] = 100 - allocatedPercentage; + } + else if (evaluationEvent.VariantAssignmentReason == VariantAssignmentReason.Percentile) + { + // If the variant was assigned due to Percentile, the percentage is the sum of the allocated percentiles for the given variant + if (evaluationEvent.FeatureDefinition.Allocation?.Percentile != null) + { + tags["VariantAssignmentPercentage"] = evaluationEvent.FeatureDefinition.Allocation.Percentile + .Where(p => p.Variant == evaluationEvent.Variant?.Name) + .Sum(p => p.To - p.From); + } } // DefaultWhenEnabled @@ -147,7 +165,7 @@ private static string GenerateAllocationId(FeatureDefinition featureDefinition) byte[] hash = sha256.ComputeHash(Encoding.UTF8.GetBytes(input)); byte[] truncatedHash = new byte[15]; Array.Copy(hash, truncatedHash, 15); - return Convert.ToBase64String(truncatedHash); + return truncatedHash.ToBase64Url(); } } } diff --git a/tests/Tests.FeatureManagement/FeatureManagementTest.cs b/tests/Tests.FeatureManagement/FeatureManagementTest.cs index d8adf251..87cac41c 100644 --- a/tests/Tests.FeatureManagement/FeatureManagementTest.cs +++ b/tests/Tests.FeatureManagement/FeatureManagementTest.cs @@ -1736,6 +1736,10 @@ public async Task TelemetryPublishing() string label = evaluationEvent.Tags.FirstOrDefault(kvp => kvp.Key == "Label").Value?.ToString(); string firstTag = evaluationEvent.Tags.FirstOrDefault(kvp => kvp.Key == "Tags.Tag1").Value?.ToString(); + string variantAssignmentPercentage = evaluationEvent.Tags.FirstOrDefault(kvp => kvp.Key == "VariantAssignmentPercentage").Value?.ToString(); + string defaultWhenEnabled = evaluationEvent.Tags.FirstOrDefault(kvp => kvp.Key == "DefaultWhenEnabled").Value?.ToString(); + string allocationId = evaluationEvent.Tags.FirstOrDefault(kvp => kvp.Key == "AllocationId").Value?.ToString(); + // Test telemetry cases switch (featureName) { @@ -1763,6 +1767,9 @@ public async Task TelemetryPublishing() Assert.Equal("True", enabled); Assert.Equal("Medium", variantName); Assert.Equal(VariantAssignmentReason.DefaultWhenEnabled.ToString(), variantAssignmentReason); + Assert.Equal("100", variantAssignmentPercentage); + Assert.Equal("Medium", defaultWhenEnabled); + Assert.Equal("kLGyMxiMp7fFb5N3cT_I", allocationId); break; case Features.VariantFeatureDefaultDisabled: @@ -1771,6 +1778,9 @@ public async Task TelemetryPublishing() Assert.Equal("False", enabled); Assert.Equal("Small", variantName); Assert.Equal(VariantAssignmentReason.DefaultWhenDisabled.ToString(), variantAssignmentReason); + Assert.Null(variantAssignmentPercentage); + Assert.Null(defaultWhenEnabled); + Assert.Null(allocationId); break; case Features.VariantFeaturePercentileOn: @@ -1793,6 +1803,9 @@ public async Task TelemetryPublishing() currentTest = 0; Assert.Null(variantName); Assert.Equal(VariantAssignmentReason.DefaultWhenDisabled.ToString(), variantAssignmentReason); + Assert.Null(variantAssignmentPercentage); + Assert.Null(defaultWhenEnabled); + Assert.Equal("JiCG2_6VXr16dczqxGFl", allocationId); break; case Features.VariantFeatureUser: @@ -1800,6 +1813,9 @@ public async Task TelemetryPublishing() currentTest = 0; Assert.Equal("Small", variantName); Assert.Equal(VariantAssignmentReason.User.ToString(), variantAssignmentReason); + Assert.Null(variantAssignmentPercentage); + Assert.Null(defaultWhenEnabled); + Assert.Null(allocationId); break; case Features.VariantFeatureGroup: @@ -1807,6 +1823,9 @@ public async Task TelemetryPublishing() currentTest = 0; Assert.Equal("Small", variantName); Assert.Equal(VariantAssignmentReason.Group.ToString(), variantAssignmentReason); + Assert.Null(variantAssignmentPercentage); + Assert.Null(defaultWhenEnabled); + Assert.Null(allocationId); break; case Features.VariantFeatureNoVariants: @@ -1814,6 +1833,9 @@ public async Task TelemetryPublishing() currentTest = 0; Assert.Null(variantName); Assert.Equal(VariantAssignmentReason.None.ToString(), variantAssignmentReason); + Assert.Null(variantAssignmentPercentage); + Assert.Null(defaultWhenEnabled); + Assert.Null(allocationId); break; case Features.VariantFeatureNoAllocation: @@ -1821,6 +1843,9 @@ public async Task TelemetryPublishing() currentTest = 0; Assert.Null(variantName); Assert.Equal(VariantAssignmentReason.DefaultWhenEnabled.ToString(), variantAssignmentReason); + Assert.Equal("100", variantAssignmentPercentage); + Assert.Null(defaultWhenEnabled); + Assert.Null(allocationId); break; case Features.VariantFeatureAlwaysOffNoAllocation: @@ -1828,6 +1853,19 @@ public async Task TelemetryPublishing() currentTest = 0; Assert.Null(variantName); Assert.Equal(VariantAssignmentReason.DefaultWhenDisabled.ToString(), variantAssignmentReason); + Assert.Null(variantAssignmentPercentage); + Assert.Null(defaultWhenEnabled); + Assert.Null(allocationId); + break; + + case Features.VariantFeatureIncorrectDefaultWhenEnabled: + Assert.Equal(13, currentTest); + currentTest = 0; + Assert.Null(variantName); + Assert.Equal(VariantAssignmentReason.DefaultWhenEnabled.ToString(), variantAssignmentReason); + Assert.Equal("100", variantAssignmentPercentage); + Assert.Equal("Foo", defaultWhenEnabled); + Assert.Equal("jGJG4WioOB6hH6rh7jOx", allocationId); break; default: @@ -1892,6 +1930,10 @@ public async Task TelemetryPublishing() await featureManager.GetVariantAsync(Features.VariantFeatureAlwaysOffNoAllocation, cancellationToken); Assert.Equal(0, currentTest); + currentTest = 13; + await featureManager.GetVariantAsync(Features.VariantFeatureIncorrectDefaultWhenEnabled, cancellationToken); + Assert.Equal(0, currentTest); + // Test a feature with telemetry disabled- should throw if the listener hits it bool result = await featureManager.IsEnabledAsync(Features.OnTestFeature, cancellationToken); diff --git a/tests/Tests.FeatureManagement/Features.cs b/tests/Tests.FeatureManagement/Features.cs index 22c22201..3f219766 100644 --- a/tests/Tests.FeatureManagement/Features.cs +++ b/tests/Tests.FeatureManagement/Features.cs @@ -23,6 +23,7 @@ static class Features public const string VariantFeatureGroup = "VariantFeatureGroup"; public const string VariantFeatureNoVariants = "VariantFeatureNoVariants"; public const string VariantFeatureNoAllocation = "VariantFeatureNoAllocation"; + public const string VariantFeatureIncorrectDefaultWhenEnabled = "VariantFeatureIncorrectDefaultWhenEnabled"; public const string VariantFeatureAlwaysOffNoAllocation = "VariantFeatureAlwaysOffNoAllocation"; public const string VariantFeatureInvalidStatusOverride = "VariantFeatureInvalidStatusOverride"; public const string VariantFeatureInvalidFromTo = "VariantFeatureInvalidFromTo"; diff --git a/tests/Tests.FeatureManagement/appsettings.json b/tests/Tests.FeatureManagement/appsettings.json index a99a325d..e9357782 100644 --- a/tests/Tests.FeatureManagement/appsettings.json +++ b/tests/Tests.FeatureManagement/appsettings.json @@ -383,6 +383,22 @@ "enabled": true } }, + { + "id": "VariantFeatureIncorrectDefaultWhenEnabled", + "enabled": true, + "variants": [ + { + "name": "Small", + "configuration_value": "300px" + } + ], + "allocation": { + "default_when_enabled": "Foo" + }, + "telemetry": { + "enabled": true + } + }, { "id": "VariantFeatureAlwaysOffNoAllocation", "enabled": false, From ba1676c24940aa6c5f7cf6cdfd0dab45a30700ce Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Tue, 10 Sep 2024 17:46:15 -0700 Subject: [PATCH 3/8] Update comments --- .../Telemetry/TelemetryEventHandler.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs b/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs index db7b769c..6e8a62fc 100644 --- a/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs +++ b/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs @@ -57,10 +57,10 @@ public static void HandleEvaluationEvent(EvaluationEvent evaluationEvent, ILogge } } - // VariantAllocationPercentage + // VariantAssignmentPercentage if (evaluationEvent.VariantAssignmentReason == VariantAssignmentReason.DefaultWhenEnabled) { - // If the variant was assigned due to DefaultWhenEnabled, the percentage is 100% - all allocated percentiles + // If the variant was assigned due to DefaultWhenEnabled, the percentage reflects the unallocated percentiles double allocatedPercentage = 0; if (evaluationEvent.FeatureDefinition.Allocation?.Percentile != null) From ada8823116e7602eb237a1df06ffbf9dffe9b8b0 Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Fri, 13 Sep 2024 11:50:40 -0700 Subject: [PATCH 4/8] Removed allocation id and fixed some tests --- .../Telemetry/TelemetryEventHandler.cs | 80 ------------------- .../FeatureManagementTest.cs | 10 --- .../Tests.FeatureManagement/appsettings.json | 6 +- 3 files changed, 3 insertions(+), 93 deletions(-) diff --git a/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs b/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs index 6e8a62fc..fc514428 100644 --- a/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs +++ b/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs @@ -1,12 +1,8 @@ using Microsoft.Extensions.Logging; -using Microsoft.FeatureManagement.Extensions; using System; using System.Collections.Generic; using System.Diagnostics; using System.Linq; -using System.Security.Cryptography; -using System.Text; -using System.Web; namespace Microsoft.FeatureManagement.Telemetry { @@ -88,85 +84,9 @@ public static void HandleEvaluationEvent(EvaluationEvent evaluationEvent, ILogge tags["DefaultWhenEnabled"] = evaluationEvent.FeatureDefinition.Allocation.DefaultWhenEnabled; } - // AllocationId - string allocationId = GenerateAllocationId(evaluationEvent.FeatureDefinition); - - if (allocationId != null) - { - tags["AllocationId"] = allocationId; - } - var activityEvent = new ActivityEvent("FeatureFlag", DateTimeOffset.UtcNow, tags); Activity.Current.AddEvent(activityEvent); } - - private static string GenerateAllocationId(FeatureDefinition featureDefinition) - { - StringBuilder inputBuilder = new StringBuilder(); - - // Seed - inputBuilder.Append($"seed={featureDefinition.Allocation?.Seed ?? ""}"); - - var allocatedVariants = new HashSet(); - - // DefaultWhenEnabled - if (featureDefinition.Allocation?.DefaultWhenEnabled != null) - { - allocatedVariants.Add(featureDefinition.Allocation.DefaultWhenEnabled); - } - - inputBuilder.Append("\n"); - inputBuilder.Append($"default_when_enabled={featureDefinition.Allocation?.DefaultWhenEnabled ?? ""}"); - - // Percentiles - inputBuilder.Append("\n"); - inputBuilder.Append("percentiles="); - - if (featureDefinition.Allocation?.Percentile != null && featureDefinition.Allocation.Percentile.Any()) - { - var sortedPercentiles = featureDefinition.Allocation.Percentile - .Where(p => p.From != p.To) - .OrderBy(p => p.From) - .ToList(); - - allocatedVariants.UnionWith(sortedPercentiles.Select(p => p.Variant)); - - inputBuilder.Append(string.Join(";", sortedPercentiles.Select(p => $"{p.From},{p.Variant},{p.To}"))); - } - - // Variants - inputBuilder.Append("\n"); - inputBuilder.Append("variants="); - - if (allocatedVariants.Any() && featureDefinition.Variants != null && featureDefinition.Variants.Any()) - { - var sortedVariants = featureDefinition.Variants - .Where(variant => allocatedVariants.Contains(variant.Name)) - .OrderBy(variant => variant.Name) - .ToList(); - - inputBuilder.Append(string.Join(";", sortedVariants.Select(v => $"{v.Name},{v.ConfigurationValue?.Value}"))); - } - - // If there's not a special seed and no variants allocated, return null - if (featureDefinition.Allocation?.Seed == null && - !allocatedVariants.Any()) - { - return null; - } - - // Example input string - // input == "seed=123abc\ndefault_when_enabled=Control\npercentiles=0,Control,20;20,Test,100\nvariants=Control,standard;Test,special" - string input = inputBuilder.ToString(); - - using (SHA256 sha256 = SHA256.Create()) - { - byte[] hash = sha256.ComputeHash(Encoding.UTF8.GetBytes(input)); - byte[] truncatedHash = new byte[15]; - Array.Copy(hash, truncatedHash, 15); - return truncatedHash.ToBase64Url(); - } - } } } diff --git a/tests/Tests.FeatureManagement/FeatureManagementTest.cs b/tests/Tests.FeatureManagement/FeatureManagementTest.cs index 87cac41c..4e783db5 100644 --- a/tests/Tests.FeatureManagement/FeatureManagementTest.cs +++ b/tests/Tests.FeatureManagement/FeatureManagementTest.cs @@ -1738,7 +1738,6 @@ public async Task TelemetryPublishing() string variantAssignmentPercentage = evaluationEvent.Tags.FirstOrDefault(kvp => kvp.Key == "VariantAssignmentPercentage").Value?.ToString(); string defaultWhenEnabled = evaluationEvent.Tags.FirstOrDefault(kvp => kvp.Key == "DefaultWhenEnabled").Value?.ToString(); - string allocationId = evaluationEvent.Tags.FirstOrDefault(kvp => kvp.Key == "AllocationId").Value?.ToString(); // Test telemetry cases switch (featureName) @@ -1769,7 +1768,6 @@ public async Task TelemetryPublishing() Assert.Equal(VariantAssignmentReason.DefaultWhenEnabled.ToString(), variantAssignmentReason); Assert.Equal("100", variantAssignmentPercentage); Assert.Equal("Medium", defaultWhenEnabled); - Assert.Equal("kLGyMxiMp7fFb5N3cT_I", allocationId); break; case Features.VariantFeatureDefaultDisabled: @@ -1780,7 +1778,6 @@ public async Task TelemetryPublishing() Assert.Equal(VariantAssignmentReason.DefaultWhenDisabled.ToString(), variantAssignmentReason); Assert.Null(variantAssignmentPercentage); Assert.Null(defaultWhenEnabled); - Assert.Null(allocationId); break; case Features.VariantFeaturePercentileOn: @@ -1805,7 +1802,6 @@ public async Task TelemetryPublishing() Assert.Equal(VariantAssignmentReason.DefaultWhenDisabled.ToString(), variantAssignmentReason); Assert.Null(variantAssignmentPercentage); Assert.Null(defaultWhenEnabled); - Assert.Equal("JiCG2_6VXr16dczqxGFl", allocationId); break; case Features.VariantFeatureUser: @@ -1815,7 +1811,6 @@ public async Task TelemetryPublishing() Assert.Equal(VariantAssignmentReason.User.ToString(), variantAssignmentReason); Assert.Null(variantAssignmentPercentage); Assert.Null(defaultWhenEnabled); - Assert.Null(allocationId); break; case Features.VariantFeatureGroup: @@ -1825,7 +1820,6 @@ public async Task TelemetryPublishing() Assert.Equal(VariantAssignmentReason.Group.ToString(), variantAssignmentReason); Assert.Null(variantAssignmentPercentage); Assert.Null(defaultWhenEnabled); - Assert.Null(allocationId); break; case Features.VariantFeatureNoVariants: @@ -1835,7 +1829,6 @@ public async Task TelemetryPublishing() Assert.Equal(VariantAssignmentReason.None.ToString(), variantAssignmentReason); Assert.Null(variantAssignmentPercentage); Assert.Null(defaultWhenEnabled); - Assert.Null(allocationId); break; case Features.VariantFeatureNoAllocation: @@ -1845,7 +1838,6 @@ public async Task TelemetryPublishing() Assert.Equal(VariantAssignmentReason.DefaultWhenEnabled.ToString(), variantAssignmentReason); Assert.Equal("100", variantAssignmentPercentage); Assert.Null(defaultWhenEnabled); - Assert.Null(allocationId); break; case Features.VariantFeatureAlwaysOffNoAllocation: @@ -1855,7 +1847,6 @@ public async Task TelemetryPublishing() Assert.Equal(VariantAssignmentReason.DefaultWhenDisabled.ToString(), variantAssignmentReason); Assert.Null(variantAssignmentPercentage); Assert.Null(defaultWhenEnabled); - Assert.Null(allocationId); break; case Features.VariantFeatureIncorrectDefaultWhenEnabled: @@ -1865,7 +1856,6 @@ public async Task TelemetryPublishing() Assert.Equal(VariantAssignmentReason.DefaultWhenEnabled.ToString(), variantAssignmentReason); Assert.Equal("100", variantAssignmentPercentage); Assert.Equal("Foo", defaultWhenEnabled); - Assert.Equal("jGJG4WioOB6hH6rh7jOx", allocationId); break; default: diff --git a/tests/Tests.FeatureManagement/appsettings.json b/tests/Tests.FeatureManagement/appsettings.json index e9357782..75a75d21 100644 --- a/tests/Tests.FeatureManagement/appsettings.json +++ b/tests/Tests.FeatureManagement/appsettings.json @@ -209,7 +209,7 @@ "to": 50 } ], - "seed": 1234 + "seed": "1234" }, "telemetry": { "enabled": true @@ -231,7 +231,7 @@ "to": 50 } ], - "seed": 12345 + "seed": "12345" }, "telemetry": { "enabled": true @@ -253,7 +253,7 @@ "to": 100 } ], - "seed": 12345 + "seed": "12345" }, "telemetry": { "enabled": true From 85cdd0aad3776c69de4c1a27ffcdff8dbbef3620 Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Mon, 16 Sep 2024 11:45:44 -0700 Subject: [PATCH 5/8] Removed bytes extension --- .../Extensions/BytesExtensions.cs | 43 ------------------- 1 file changed, 43 deletions(-) delete mode 100644 src/Microsoft.FeatureManagement/Extensions/BytesExtensions.cs diff --git a/src/Microsoft.FeatureManagement/Extensions/BytesExtensions.cs b/src/Microsoft.FeatureManagement/Extensions/BytesExtensions.cs deleted file mode 100644 index 44f697d9..00000000 --- a/src/Microsoft.FeatureManagement/Extensions/BytesExtensions.cs +++ /dev/null @@ -1,43 +0,0 @@ -using System; -using System.Text; - -namespace Microsoft.FeatureManagement.Extensions -{ - internal static class BytesExtensions - { - /// - /// Converts a byte array to Base64URL string with optional padding ('=') characters removed. - /// Base64 description: https://datatracker.ietf.org/doc/html/rfc4648.html#section-4 - /// - public static string ToBase64Url(this byte[] bytes) - { - string bytesBase64 = Convert.ToBase64String(bytes); - - int indexOfEquals = bytesBase64.IndexOf("="); - - // Skip the optional padding of '=' characters based on the Base64Url spec if any are present from the Base64 conversion - int stringBuilderCapacity = indexOfEquals != -1 ? indexOfEquals : bytesBase64.Length; - - StringBuilder stringBuilder = new StringBuilder(stringBuilderCapacity); - - // Construct Base64URL string by replacing characters in Base64 conversion that are not URL safe - for (int i = 0; i < stringBuilderCapacity; i++) - { - if (bytesBase64[i] == '+') - { - stringBuilder.Append('-'); - } - else if (bytesBase64[i] == '/') - { - stringBuilder.Append('_'); - } - else - { - stringBuilder.Append(bytesBase64[i]); - } - } - - return stringBuilder.ToString(); - } - } -} From 94069992773cbc80c666d92e6aa95c57f9e87a32 Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Wed, 23 Oct 2024 11:35:34 -0700 Subject: [PATCH 6/8] Update src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs Co-authored-by: Sami Sadfa <71456174+samsadsam@users.noreply.github.com> --- .../Telemetry/TelemetryEventHandler.cs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs b/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs index fc514428..bc10a2ce 100644 --- a/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs +++ b/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs @@ -57,13 +57,7 @@ public static void HandleEvaluationEvent(EvaluationEvent evaluationEvent, ILogge if (evaluationEvent.VariantAssignmentReason == VariantAssignmentReason.DefaultWhenEnabled) { // If the variant was assigned due to DefaultWhenEnabled, the percentage reflects the unallocated percentiles - double allocatedPercentage = 0; - - if (evaluationEvent.FeatureDefinition.Allocation?.Percentile != null) - { - allocatedPercentage += evaluationEvent.FeatureDefinition.Allocation.Percentile - .Sum(p => p.To - p.From); - } +double allocatedPercentage = evaluationEvent.FeatureDefinition.Allocation?.Percentile?.Sum(p => p.To - p.From) ?? 0; tags["VariantAssignmentPercentage"] = 100 - allocatedPercentage; } From e53aeb000bfebc800b12865aa18a6872e1c0c669 Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Wed, 23 Oct 2024 11:35:53 -0700 Subject: [PATCH 7/8] Resolving comments --- .../FeatureManager.cs | 2 +- ...ndler.cs => FeatureEvaluationTelemetry.cs} | 20 +++++++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) rename src/Microsoft.FeatureManagement/Telemetry/{TelemetryEventHandler.cs => FeatureEvaluationTelemetry.cs} (85%) diff --git a/src/Microsoft.FeatureManagement/FeatureManager.cs b/src/Microsoft.FeatureManagement/FeatureManager.cs index eb8d54d5..d1037452 100644 --- a/src/Microsoft.FeatureManagement/FeatureManager.cs +++ b/src/Microsoft.FeatureManagement/FeatureManager.cs @@ -373,7 +373,7 @@ private async ValueTask EvaluateFeature(string featur Activity.Current != null && Activity.Current.IsAllDataRequested) { - TelemetryEventHandler.HandleEvaluationEvent(evaluationEvent, Logger); + FeatureEvaluationTelemetry.Publish(evaluationEvent, Logger); } } diff --git a/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs b/src/Microsoft.FeatureManagement/Telemetry/FeatureEvaluationTelemetry.cs similarity index 85% rename from src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs rename to src/Microsoft.FeatureManagement/Telemetry/FeatureEvaluationTelemetry.cs index fc514428..aaf92649 100644 --- a/src/Microsoft.FeatureManagement/Telemetry/TelemetryEventHandler.cs +++ b/src/Microsoft.FeatureManagement/Telemetry/FeatureEvaluationTelemetry.cs @@ -6,7 +6,7 @@ namespace Microsoft.FeatureManagement.Telemetry { - internal static class TelemetryEventHandler + internal static class FeatureEvaluationTelemetry { private static readonly string EvaluationEventVersion = "1.0.0"; @@ -15,10 +15,22 @@ internal static class TelemetryEventHandler /// /// The to publish as an /// Optional logger to log warnings to - public static void HandleEvaluationEvent(EvaluationEvent evaluationEvent, ILogger logger) + public static void Publish(EvaluationEvent evaluationEvent, ILogger logger) { - Debug.Assert(evaluationEvent != null); - Debug.Assert(evaluationEvent.FeatureDefinition != null); + if (Activity.Current == null) + { + throw new InvalidOperationException("An Activity must be created before calling this method."); + } + + if (evaluationEvent == null) + { + throw new ArgumentNullException(nameof(evaluationEvent)); + } + + if (evaluationEvent.FeatureDefinition == null) + { + throw new ArgumentNullException(nameof(evaluationEvent.FeatureDefinition)); + } var tags = new ActivityTagsCollection() { From 4cb6a17598b4b212998467b37b0abcc8fc0f9bc0 Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Wed, 23 Oct 2024 11:54:31 -0700 Subject: [PATCH 8/8] Fix formatting --- .../Telemetry/FeatureEvaluationTelemetry.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.FeatureManagement/Telemetry/FeatureEvaluationTelemetry.cs b/src/Microsoft.FeatureManagement/Telemetry/FeatureEvaluationTelemetry.cs index 8b8add67..26952ca5 100644 --- a/src/Microsoft.FeatureManagement/Telemetry/FeatureEvaluationTelemetry.cs +++ b/src/Microsoft.FeatureManagement/Telemetry/FeatureEvaluationTelemetry.cs @@ -69,7 +69,7 @@ public static void Publish(EvaluationEvent evaluationEvent, ILogger logger) if (evaluationEvent.VariantAssignmentReason == VariantAssignmentReason.DefaultWhenEnabled) { // If the variant was assigned due to DefaultWhenEnabled, the percentage reflects the unallocated percentiles -double allocatedPercentage = evaluationEvent.FeatureDefinition.Allocation?.Percentile?.Sum(p => p.To - p.From) ?? 0; + double allocatedPercentage = evaluationEvent.FeatureDefinition.Allocation?.Percentile?.Sum(p => p.To - p.From) ?? 0; tags["VariantAssignmentPercentage"] = 100 - allocatedPercentage; }