From 87b34df75fcb5ec1fb86ae28403da6d9a0ec9bce Mon Sep 17 00:00:00 2001 From: Ross Grambo Date: Wed, 25 Oct 2023 19:38:01 -0700 Subject: [PATCH 01/15] Adds Reason field and adjusts evaluation logic --- .../FeatureManager.cs | 227 +++++++++--------- .../Telemetry/EvaluationEvent.cs | 5 + .../FeatureManagement.cs | 13 + 3 files changed, 130 insertions(+), 115 deletions(-) diff --git a/src/Microsoft.FeatureManagement/FeatureManager.cs b/src/Microsoft.FeatureManagement/FeatureManager.cs index c9ddb786..5512ce0b 100644 --- a/src/Microsoft.FeatureManagement/FeatureManager.cs +++ b/src/Microsoft.FeatureManagement/FeatureManager.cs @@ -71,49 +71,106 @@ public FeatureManager( public Task IsEnabledAsync(string feature) { - return IsEnabledWithVariantsAsync(feature, appContext: null, useAppContext: false, CancellationToken.None).AsTask(); + return IsEnabledEvaluation(feature, appContext: null, useAppContext: false, CancellationToken.None).AsTask(); } public Task IsEnabledAsync(string feature, TContext appContext) { - return IsEnabledWithVariantsAsync(feature, appContext, useAppContext: true, CancellationToken.None).AsTask(); + return IsEnabledEvaluation(feature, appContext, useAppContext: true, CancellationToken.None).AsTask(); } public ValueTask IsEnabledAsync(string feature, CancellationToken cancellationToken) { - return IsEnabledWithVariantsAsync(feature, appContext: null, useAppContext: false, cancellationToken); + return IsEnabledEvaluation(feature, appContext: null, useAppContext: false, cancellationToken); } public ValueTask IsEnabledAsync(string feature, TContext appContext, CancellationToken cancellationToken) { - return IsEnabledWithVariantsAsync(feature, appContext, useAppContext: true, cancellationToken); + return IsEnabledEvaluation(feature, appContext, useAppContext: true, cancellationToken); } - private async ValueTask IsEnabledWithVariantsAsync(string feature, TContext appContext, bool useAppContext, CancellationToken cancellationToken) + private async ValueTask IsEnabledEvaluation(string feature, TContext appContext, bool useAppContext, CancellationToken cancellationToken) { - bool isFeatureEnabled = false; + EvaluationEvent evaluationEvent = new EvaluationEvent + { + FeatureDefinition = await GetFeatureDefinition(feature).ConfigureAwait(false) + }; - FeatureDefinition featureDefinition = await GetFeatureDefinition(feature).ConfigureAwait(false); + await EvaluateFeature(evaluationEvent, appContext, useAppContext, cancellationToken); - VariantDefinition variantDefinition = null; + return evaluationEvent.IsEnabled; + } - if (featureDefinition != null) + public ValueTask GetVariantAsync(string feature, CancellationToken cancellationToken) + { + if (string.IsNullOrEmpty(feature)) { - isFeatureEnabled = await IsEnabledAsync(featureDefinition, appContext, useAppContext, cancellationToken).ConfigureAwait(false); + throw new ArgumentNullException(nameof(feature)); + } + + return GetVariantEvaluation(feature, context: null, useContext: false, cancellationToken); + } + + public ValueTask GetVariantAsync(string feature, TargetingContext context, CancellationToken cancellationToken) + { + if (string.IsNullOrEmpty(feature)) + { + throw new ArgumentNullException(nameof(feature)); + } + + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + return GetVariantEvaluation(feature, context, useContext: true, cancellationToken); + } - if (featureDefinition.Variants != null && featureDefinition.Variants.Any() && featureDefinition.Allocation != null) + private async ValueTask GetVariantEvaluation(string feature, TargetingContext context, bool useContext, CancellationToken cancellationToken) + { + EvaluationEvent evaluationEvent = new EvaluationEvent + { + FeatureDefinition = await GetFeatureDefinition(feature).ConfigureAwait(false) + }; + + await EvaluateFeature(evaluationEvent, context, useContext, cancellationToken); + + return evaluationEvent.Variant; + } + + private async Task EvaluateFeature(EvaluationEvent evaluationEvent, TContext context, bool useContext, CancellationToken cancellationToken) + { + if (evaluationEvent.FeatureDefinition != null) + { + // + // Determine IsEnabled + evaluationEvent.IsEnabled = await IsEnabledAsync(evaluationEvent.FeatureDefinition, context, useContext, cancellationToken).ConfigureAwait(false); + + // + // Determine Variant + VariantDefinition variantDefinition; + + if (evaluationEvent.FeatureDefinition.Allocation == null || (!evaluationEvent.FeatureDefinition.Variants?.Any() ?? false)) + { + variantDefinition = null; + + evaluationEvent.VariantReason = "No Allocation or Variants"; + } + else { - if (!isFeatureEnabled) + if (!evaluationEvent.IsEnabled) { - variantDefinition = featureDefinition.Variants.FirstOrDefault((variant) => variant.Name == featureDefinition.Allocation.DefaultWhenDisabled); + variantDefinition = evaluationEvent.FeatureDefinition.Variants.FirstOrDefault((variant) => variant.Name == evaluationEvent.FeatureDefinition.Allocation.DefaultWhenDisabled); + + evaluationEvent.VariantReason = "Disabled Default"; } else { TargetingContext targetingContext; - if (useAppContext) + if (useContext) { - targetingContext = appContext as TargetingContext; + targetingContext = context as TargetingContext; } else { @@ -121,21 +178,25 @@ private async ValueTask IsEnabledWithVariantsAsync(string featur } variantDefinition = await GetAssignedVariantAsync( - featureDefinition, + evaluationEvent, targetingContext, cancellationToken) .ConfigureAwait(false); } - if (variantDefinition != null && featureDefinition.Status != FeatureStatus.Disabled) + evaluationEvent.Variant = variantDefinition != null ? GetVariantFromVariantDefinition(variantDefinition) : null; + + // + // Override IsEnabled if variant has an override + if (variantDefinition != null && evaluationEvent.FeatureDefinition.Status != FeatureStatus.Disabled) { if (variantDefinition.StatusOverride == StatusOverride.Enabled) { - isFeatureEnabled = true; + evaluationEvent.IsEnabled = true; } else if (variantDefinition.StatusOverride == StatusOverride.Disabled) { - isFeatureEnabled = false; + evaluationEvent.IsEnabled = false; } } } @@ -143,20 +204,15 @@ private async ValueTask IsEnabledWithVariantsAsync(string featur foreach (ISessionManager sessionManager in _sessionManagers) { - await sessionManager.SetAsync(feature, isFeatureEnabled).ConfigureAwait(false); + await sessionManager.SetAsync(evaluationEvent.FeatureDefinition.Name, evaluationEvent.IsEnabled).ConfigureAwait(false); } - if (featureDefinition.TelemetryEnabled) + if (evaluationEvent.FeatureDefinition.TelemetryEnabled) { - PublishTelemetry(new EvaluationEvent - { - FeatureDefinition = featureDefinition, - IsEnabled = isFeatureEnabled, - Variant = variantDefinition != null ? GetVariantFromVariantDefinition(variantDefinition) : null - }, cancellationToken); + PublishTelemetry(evaluationEvent, cancellationToken); } - return isFeatureEnabled; + return evaluationEvent; } public IAsyncEnumerable GetFeatureNamesAsync() @@ -304,73 +360,6 @@ await contextualFilter.EvaluateAsync(context, appContext).ConfigureAwait(false) return enabled; } - public ValueTask GetVariantAsync(string feature, CancellationToken cancellationToken) - { - if (string.IsNullOrEmpty(feature)) - { - throw new ArgumentNullException(nameof(feature)); - } - - return GetVariantAsync(feature, context: null, useContext: false, cancellationToken); - } - - public ValueTask GetVariantAsync(string feature, TargetingContext context, CancellationToken cancellationToken) - { - if (string.IsNullOrEmpty(feature)) - { - throw new ArgumentNullException(nameof(feature)); - } - - if (context == null) - { - throw new ArgumentNullException(nameof(context)); - } - - return GetVariantAsync(feature, context, useContext: true, cancellationToken); - } - - private async ValueTask GetVariantAsync(string feature, TargetingContext context, bool useContext, CancellationToken cancellationToken) - { - FeatureDefinition featureDefinition = await GetFeatureDefinition(feature).ConfigureAwait(false); - - if (featureDefinition == null || featureDefinition.Allocation == null || (!featureDefinition.Variants?.Any() ?? false)) - { - return null; - } - - VariantDefinition variantDefinition = null; - - bool isFeatureEnabled = await IsEnabledAsync(featureDefinition, context, useContext, cancellationToken).ConfigureAwait(false); - - if (!isFeatureEnabled) - { - variantDefinition = featureDefinition.Variants.FirstOrDefault((variant) => variant.Name == featureDefinition.Allocation.DefaultWhenDisabled); - } - else - { - if (!useContext) - { - context = await ResolveTargetingContextAsync(cancellationToken).ConfigureAwait(false); - } - - variantDefinition = await GetAssignedVariantAsync(featureDefinition, context, cancellationToken).ConfigureAwait(false); - } - - Variant variant = variantDefinition != null ? GetVariantFromVariantDefinition(variantDefinition) : null; - - if (featureDefinition.TelemetryEnabled) - { - PublishTelemetry(new EvaluationEvent - { - FeatureDefinition = featureDefinition, - IsEnabled = isFeatureEnabled, - Variant = variant - }, cancellationToken); - } - - return variant; - } - private async ValueTask GetFeatureDefinition(string feature) { FeatureDefinition featureDefinition = await _featureDefinitionProvider @@ -415,95 +404,103 @@ private async ValueTask ResolveTargetingContextAsync(Cancellat return context; } - private async ValueTask GetAssignedVariantAsync(FeatureDefinition featureDefinition, TargetingContext context, CancellationToken cancellationToken) + private async ValueTask GetAssignedVariantAsync(EvaluationEvent evaluationEvent, TargetingContext context, CancellationToken cancellationToken) { VariantDefinition variantDefinition = null; if (context != null) { - variantDefinition = await AssignVariantAsync(featureDefinition, context, cancellationToken).ConfigureAwait(false); + variantDefinition = await AssignVariantAsync(evaluationEvent, context, cancellationToken).ConfigureAwait(false); } if (variantDefinition == null) { - variantDefinition = featureDefinition.Variants.FirstOrDefault((variant) => variant.Name == featureDefinition.Allocation.DefaultWhenEnabled); + variantDefinition = evaluationEvent.FeatureDefinition.Variants.FirstOrDefault((variant) => variant.Name == evaluationEvent.FeatureDefinition.Allocation.DefaultWhenEnabled); + + evaluationEvent.VariantReason = "Enabled Default"; } return variantDefinition; } - private ValueTask AssignVariantAsync(FeatureDefinition featureDefinition, TargetingContext targetingContext, CancellationToken cancellationToken) + private ValueTask AssignVariantAsync(EvaluationEvent evaluationEvent, TargetingContext targetingContext, CancellationToken cancellationToken) { VariantDefinition variant = null; - if (featureDefinition.Allocation.User != null) + if (evaluationEvent.FeatureDefinition.Allocation.User != null) { - foreach (UserAllocation user in featureDefinition.Allocation.User) + foreach (UserAllocation user in evaluationEvent.FeatureDefinition.Allocation.User) { if (TargetingEvaluator.IsTargeted(targetingContext.UserId, user.Users, _assignerOptions.IgnoreCase)) { if (string.IsNullOrEmpty(user.Variant)) { - _logger.LogWarning($"Missing variant name for user allocation in feature {featureDefinition.Name}"); + _logger.LogWarning($"Missing variant name for user allocation in feature {evaluationEvent.FeatureDefinition.Name}"); return new ValueTask((VariantDefinition)null); } - Debug.Assert(featureDefinition.Variants != null); + Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); + + evaluationEvent.VariantReason = "User Allocated"; return new ValueTask( - featureDefinition + evaluationEvent.FeatureDefinition .Variants .FirstOrDefault((variant) => variant.Name == user.Variant)); } } } - if (featureDefinition.Allocation.Group != null) + if (evaluationEvent.FeatureDefinition.Allocation.Group != null) { - foreach (GroupAllocation group in featureDefinition.Allocation.Group) + foreach (GroupAllocation group in evaluationEvent.FeatureDefinition.Allocation.Group) { if (TargetingEvaluator.IsTargeted(targetingContext.Groups, group.Groups, _assignerOptions.IgnoreCase)) { if (string.IsNullOrEmpty(group.Variant)) { - _logger.LogWarning($"Missing variant name for group allocation in feature {featureDefinition.Name}"); + _logger.LogWarning($"Missing variant name for group allocation in feature {evaluationEvent.FeatureDefinition.Name}"); return new ValueTask((VariantDefinition)null); } - Debug.Assert(featureDefinition.Variants != null); + Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); + + evaluationEvent.VariantReason = "Group Allocated"; return new ValueTask( - featureDefinition + evaluationEvent.FeatureDefinition .Variants .FirstOrDefault((variant) => variant.Name == group.Variant)); } } } - if (featureDefinition.Allocation.Percentile != null) + if (evaluationEvent.FeatureDefinition.Allocation.Percentile != null) { - foreach (PercentileAllocation percentile in featureDefinition.Allocation.Percentile) + foreach (PercentileAllocation percentile in evaluationEvent.FeatureDefinition.Allocation.Percentile) { if (TargetingEvaluator.IsTargeted( targetingContext, percentile.From, percentile.To, _assignerOptions.IgnoreCase, - featureDefinition.Allocation.Seed ?? $"allocation\n{featureDefinition.Name}")) + evaluationEvent.FeatureDefinition.Allocation.Seed ?? $"allocation\n{evaluationEvent.FeatureDefinition.Name}")) { if (string.IsNullOrEmpty(percentile.Variant)) { - _logger.LogWarning($"Missing variant name for percentile allocation in feature {featureDefinition.Name}"); + _logger.LogWarning($"Missing variant name for percentile allocation in feature {evaluationEvent.FeatureDefinition.Name}"); return new ValueTask((VariantDefinition)null); } - Debug.Assert(featureDefinition.Variants != null); + Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); + + evaluationEvent.VariantReason = "Percentile Allocated"; return new ValueTask( - featureDefinition + evaluationEvent.FeatureDefinition .Variants .FirstOrDefault((variant) => variant.Name == percentile.Variant)); } diff --git a/src/Microsoft.FeatureManagement/Telemetry/EvaluationEvent.cs b/src/Microsoft.FeatureManagement/Telemetry/EvaluationEvent.cs index a425c290..0fa0a6ae 100644 --- a/src/Microsoft.FeatureManagement/Telemetry/EvaluationEvent.cs +++ b/src/Microsoft.FeatureManagement/Telemetry/EvaluationEvent.cs @@ -24,5 +24,10 @@ public class EvaluationEvent /// The variant given after evaluation. /// public Variant Variant { get; set; } + + /// + /// The reason the variant was given. + /// + public string VariantReason { get; set; } } } diff --git a/tests/Tests.FeatureManagement/FeatureManagement.cs b/tests/Tests.FeatureManagement/FeatureManagement.cs index 935bf9ae..d28c18b7 100644 --- a/tests/Tests.FeatureManagement/FeatureManagement.cs +++ b/tests/Tests.FeatureManagement/FeatureManagement.cs @@ -859,6 +859,7 @@ public async Task TelemetryPublishing() Assert.Equal("EtagValue", testPublisher.evaluationEventCache.FeatureDefinition.TelemetryMetadata["Etag"]); Assert.Equal("LabelValue", testPublisher.evaluationEventCache.FeatureDefinition.TelemetryMetadata["Label"]); Assert.Equal("Tag1Value", testPublisher.evaluationEventCache.FeatureDefinition.TelemetryMetadata["Tags.Tag1"]); + Assert.Equal("No Allocation or Variants", testPublisher.evaluationEventCache.VariantReason); string offFeature = "OffTimeTestFeature"; @@ -867,6 +868,7 @@ public async Task TelemetryPublishing() Assert.False(result); Assert.Equal(offFeature, testPublisher.evaluationEventCache.FeatureDefinition.Name); Assert.Equal(result, testPublisher.evaluationEventCache.IsEnabled); + Assert.Equal("No Allocation or Variants", testPublisher.evaluationEventCache.VariantReason); // Test variant cases string variantDefaultEnabledFeature = "VariantFeatureDefaultEnabled"; @@ -892,12 +894,23 @@ public async Task TelemetryPublishing() Assert.Equal(variantFeatureStatusDisabled, testPublisher.evaluationEventCache.FeatureDefinition.Name); Assert.Equal(result, testPublisher.evaluationEventCache.IsEnabled); Assert.Equal("Small", testPublisher.evaluationEventCache.Variant.Name); + Assert.Equal("Disabled Default", testPublisher.evaluationEventCache.VariantReason); variantResult = await featureManager.GetVariantAsync(variantFeatureStatusDisabled, CancellationToken.None); Assert.False(testPublisher.evaluationEventCache.IsEnabled); Assert.Equal(variantFeatureStatusDisabled, testPublisher.evaluationEventCache.FeatureDefinition.Name); Assert.Equal(variantResult.Name, testPublisher.evaluationEventCache.Variant.Name); + Assert.Equal("Disabled Default", testPublisher.evaluationEventCache.VariantReason); + + string variantFeatureDefaultEnabled = "VariantFeatureDefaultEnabled"; + + variantResult = await featureManager.GetVariantAsync(variantFeatureDefaultEnabled, CancellationToken.None); + + Assert.True(testPublisher.evaluationEventCache.IsEnabled); + Assert.Equal(variantFeatureDefaultEnabled, testPublisher.evaluationEventCache.FeatureDefinition.Name); + Assert.Equal(variantResult.Name, testPublisher.evaluationEventCache.Variant.Name); + Assert.Equal("Enabled Default", testPublisher.evaluationEventCache.VariantReason); } [Fact] From 6a24277876b3190f32263ac1dde366f306a7c950 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Tue, 12 Dec 2023 11:19:31 +0800 Subject: [PATCH 02/15] use enum AssignmentReason --- .../ApplicationInsightsTelemetryPublisher.cs | 2 + .../FeatureManager.cs | 12 ++-- .../Telemetry/AssignmentReason.cs | 41 ++++++++++++ .../Telemetry/EvaluationEvent.cs | 6 +- .../FeatureManagement.cs | 64 +++++++------------ tests/Tests.FeatureManagement/Features.cs | 4 ++ 6 files changed, 78 insertions(+), 51 deletions(-) create mode 100644 src/Microsoft.FeatureManagement/Telemetry/AssignmentReason.cs diff --git a/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs b/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs index 02aed91a..cea919c4 100644 --- a/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs +++ b/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs @@ -41,6 +41,8 @@ public ValueTask PublishEvent(EvaluationEvent evaluationEvent, CancellationToken properties["Variant"] = evaluationEvent.Variant.Name; } + properties["AssignmentReason"] = evaluationEvent.AssignmentReason.ToString(); + if (featureDefinition.TelemetryMetadata != null) { foreach (KeyValuePair kvp in featureDefinition.TelemetryMetadata) diff --git a/src/Microsoft.FeatureManagement/FeatureManager.cs b/src/Microsoft.FeatureManagement/FeatureManager.cs index cdb3f2bc..e725a3fb 100644 --- a/src/Microsoft.FeatureManagement/FeatureManager.cs +++ b/src/Microsoft.FeatureManagement/FeatureManager.cs @@ -284,7 +284,7 @@ private async Task EvaluateFeature(EvaluationEvent ev { variantDefinition = null; - evaluationEvent.VariantReason = "No Allocation or Variants"; + evaluationEvent.AssignmentReason = AssignmentReason.None; } else { @@ -292,7 +292,7 @@ private async Task EvaluateFeature(EvaluationEvent ev { variantDefinition = evaluationEvent.FeatureDefinition.Variants.FirstOrDefault((variant) => variant.Name == evaluationEvent.FeatureDefinition.Allocation.DefaultWhenDisabled); - evaluationEvent.VariantReason = "Disabled Default"; + evaluationEvent.AssignmentReason = AssignmentReason.DisabledDefault; } else { @@ -543,7 +543,7 @@ private async ValueTask GetAssignedVariantAsync(EvaluationEve { variantDefinition = evaluationEvent.FeatureDefinition.Variants.FirstOrDefault((variant) => variant.Name == evaluationEvent.FeatureDefinition.Allocation.DefaultWhenEnabled); - evaluationEvent.VariantReason = "Enabled Default"; + evaluationEvent.AssignmentReason = AssignmentReason.EnabledDefault; } return variantDefinition; @@ -568,7 +568,7 @@ private ValueTask AssignVariantAsync(EvaluationEvent evaluati Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); - evaluationEvent.VariantReason = "User Allocated"; + evaluationEvent.AssignmentReason = AssignmentReason.User; return new ValueTask( evaluationEvent.FeatureDefinition @@ -593,7 +593,7 @@ private ValueTask AssignVariantAsync(EvaluationEvent evaluati Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); - evaluationEvent.VariantReason = "Group Allocated"; + evaluationEvent.AssignmentReason = AssignmentReason.Group; return new ValueTask( evaluationEvent.FeatureDefinition @@ -623,7 +623,7 @@ private ValueTask AssignVariantAsync(EvaluationEvent evaluati Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); - evaluationEvent.VariantReason = "Percentile Allocated"; + evaluationEvent.AssignmentReason = AssignmentReason.Percentile; return new ValueTask( evaluationEvent.FeatureDefinition diff --git a/src/Microsoft.FeatureManagement/Telemetry/AssignmentReason.cs b/src/Microsoft.FeatureManagement/Telemetry/AssignmentReason.cs new file mode 100644 index 00000000..b3a2c51b --- /dev/null +++ b/src/Microsoft.FeatureManagement/Telemetry/AssignmentReason.cs @@ -0,0 +1,41 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. +// +namespace Microsoft.FeatureManagement.Telemetry +{ + /// + /// The reason the variant was assigned during the evaluation of a feature. + /// + public enum AssignmentReason + { + /// + /// The reason the variant was assigned during the evaluation of a feature. + /// + None, + + /// + /// The reason the variant was assigned during the evaluation of a feature. + /// + DisabledDefault, + + /// + /// The reason the variant was assigned during the evaluation of a feature. + /// + EnabledDefault, + + /// + /// The reason the variant was assigned during the evaluation of a feature. + /// + User, + + /// + /// The reason the variant was assigned during the evaluation of a feature. + /// + Group, + + /// + /// The reason the variant was assigned during the evaluation of a feature. + /// + Percentile + } +} diff --git a/src/Microsoft.FeatureManagement/Telemetry/EvaluationEvent.cs b/src/Microsoft.FeatureManagement/Telemetry/EvaluationEvent.cs index 0fa0a6ae..2ca73903 100644 --- a/src/Microsoft.FeatureManagement/Telemetry/EvaluationEvent.cs +++ b/src/Microsoft.FeatureManagement/Telemetry/EvaluationEvent.cs @@ -1,8 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. // -using Microsoft.FeatureManagement.FeatureFilters; - namespace Microsoft.FeatureManagement.Telemetry { /// @@ -26,8 +24,8 @@ public class EvaluationEvent public Variant Variant { get; set; } /// - /// The reason the variant was given. + /// The reason the variant was assigned. /// - public string VariantReason { get; set; } + public AssignmentReason AssignmentReason { get; set; } } } diff --git a/tests/Tests.FeatureManagement/FeatureManagement.cs b/tests/Tests.FeatureManagement/FeatureManagement.cs index 59cb59c5..a0684ff5 100644 --- a/tests/Tests.FeatureManagement/FeatureManagement.cs +++ b/tests/Tests.FeatureManagement/FeatureManagement.cs @@ -5,6 +5,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.FeatureManagement; using Microsoft.FeatureManagement.FeatureFilters; +using Microsoft.FeatureManagement.Telemetry; using Microsoft.FeatureManagement.Tests; using System; using System.Collections.Generic; @@ -1010,11 +1011,11 @@ public async Task TelemetryPublishing() var services = new ServiceCollection(); - services + var targetingContextAccessor = new OnDemandTargetingContextAccessor(); + services.AddSingleton(targetingContextAccessor) .AddSingleton(config) .AddFeatureManagement() - .AddTelemetryPublisher() - .AddFeatureFilter(); + .AddTelemetryPublisher(); ServiceProvider serviceProvider = services.BuildServiceProvider(); @@ -1028,68 +1029,52 @@ public async Task TelemetryPublishing() Assert.Null(testPublisher.evaluationEventCache); // Test telemetry cases - const string onFeature = "AlwaysOnTestFeature"; - - result = await featureManager.IsEnabledAsync(onFeature, CancellationToken.None); + result = await featureManager.IsEnabledAsync(Features.AlwaysOnTestFeature, CancellationToken.None); Assert.True(result); - Assert.Equal(onFeature, testPublisher.evaluationEventCache.FeatureDefinition.Name); + Assert.Equal(Features.AlwaysOnTestFeature, testPublisher.evaluationEventCache.FeatureDefinition.Name); Assert.Equal(result, testPublisher.evaluationEventCache.IsEnabled); Assert.Equal("EtagValue", testPublisher.evaluationEventCache.FeatureDefinition.TelemetryMetadata["Etag"]); Assert.Equal("LabelValue", testPublisher.evaluationEventCache.FeatureDefinition.TelemetryMetadata["Label"]); Assert.Equal("Tag1Value", testPublisher.evaluationEventCache.FeatureDefinition.TelemetryMetadata["Tags.Tag1"]); - Assert.Equal("No Allocation or Variants", testPublisher.evaluationEventCache.VariantReason); + Assert.Equal(AssignmentReason.None, testPublisher.evaluationEventCache.AssignmentReason); - const string offFeature = "OffTimeTestFeature"; - - result = await featureManager.IsEnabledAsync(offFeature, CancellationToken.None); + result = await featureManager.IsEnabledAsync(Features.OffTimeTestFeature, CancellationToken.None); Assert.False(result); - Assert.Equal(offFeature, testPublisher.evaluationEventCache.FeatureDefinition.Name); + Assert.Equal(Features.OffTimeTestFeature, testPublisher.evaluationEventCache.FeatureDefinition.Name); Assert.Equal(result, testPublisher.evaluationEventCache.IsEnabled); - Assert.Equal("No Allocation or Variants", testPublisher.evaluationEventCache.VariantReason); + Assert.Equal(AssignmentReason.None, testPublisher.evaluationEventCache.AssignmentReason); // Test variant cases - const string variantDefaultEnabledFeature = "VariantFeatureDefaultEnabled"; - - result = await featureManager.IsEnabledAsync(variantDefaultEnabledFeature, CancellationToken.None); + result = await featureManager.IsEnabledAsync(Features.VariantFeatureDefaultEnabled, CancellationToken.None); Assert.True(result); - Assert.Equal(variantDefaultEnabledFeature, testPublisher.evaluationEventCache.FeatureDefinition.Name); + Assert.Equal(Features.VariantFeatureDefaultEnabled, testPublisher.evaluationEventCache.FeatureDefinition.Name); Assert.Equal(result, testPublisher.evaluationEventCache.IsEnabled); Assert.Equal("Medium", testPublisher.evaluationEventCache.Variant.Name); - Variant variantResult = await featureManager.GetVariantAsync(variantDefaultEnabledFeature, CancellationToken.None); + Variant variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureDefaultEnabled, CancellationToken.None); Assert.True(testPublisher.evaluationEventCache.IsEnabled); - Assert.Equal(variantDefaultEnabledFeature, testPublisher.evaluationEventCache.FeatureDefinition.Name); + Assert.Equal(Features.VariantFeatureDefaultEnabled, testPublisher.evaluationEventCache.FeatureDefinition.Name); Assert.Equal(variantResult.Name, testPublisher.evaluationEventCache.Variant.Name); + Assert.Equal(AssignmentReason.EnabledDefault, testPublisher.evaluationEventCache.AssignmentReason); - string variantFeatureStatusDisabled = "VariantFeatureStatusDisabled"; - - result = await featureManager.IsEnabledAsync(variantFeatureStatusDisabled, CancellationToken.None); + result = await featureManager.IsEnabledAsync(Features.VariantFeatureStatusDisabled, CancellationToken.None); Assert.False(result); - Assert.Equal(variantFeatureStatusDisabled, testPublisher.evaluationEventCache.FeatureDefinition.Name); + Assert.Equal(Features.VariantFeatureStatusDisabled, testPublisher.evaluationEventCache.FeatureDefinition.Name); Assert.Equal(result, testPublisher.evaluationEventCache.IsEnabled); Assert.Equal("Small", testPublisher.evaluationEventCache.Variant.Name); - Assert.Equal("Disabled Default", testPublisher.evaluationEventCache.VariantReason); + Assert.Equal(AssignmentReason.DisabledDefault, testPublisher.evaluationEventCache.AssignmentReason); - variantResult = await featureManager.GetVariantAsync(variantFeatureStatusDisabled, CancellationToken.None); + variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureStatusDisabled, CancellationToken.None); Assert.False(testPublisher.evaluationEventCache.IsEnabled); - Assert.Equal(variantFeatureStatusDisabled, testPublisher.evaluationEventCache.FeatureDefinition.Name); + Assert.Equal(Features.VariantFeatureStatusDisabled, testPublisher.evaluationEventCache.FeatureDefinition.Name); Assert.Equal(variantResult.Name, testPublisher.evaluationEventCache.Variant.Name); - Assert.Equal("Disabled Default", testPublisher.evaluationEventCache.VariantReason); - - string variantFeatureDefaultEnabled = "VariantFeatureDefaultEnabled"; - - variantResult = await featureManager.GetVariantAsync(variantFeatureDefaultEnabled, CancellationToken.None); - - Assert.True(testPublisher.evaluationEventCache.IsEnabled); - Assert.Equal(variantFeatureDefaultEnabled, testPublisher.evaluationEventCache.FeatureDefinition.Name); - Assert.Equal(variantResult.Name, testPublisher.evaluationEventCache.Variant.Name); - Assert.Equal("Enabled Default", testPublisher.evaluationEventCache.VariantReason); + Assert.Equal(AssignmentReason.DisabledDefault, testPublisher.evaluationEventCache.AssignmentReason); } [Fact] @@ -1101,17 +1086,14 @@ public async Task TelemetryPublishingNullPublisher() services .AddSingleton(config) - .AddFeatureManagement() - .AddFeatureFilter(); + .AddFeatureManagement(); ServiceProvider serviceProvider = services.BuildServiceProvider(); FeatureManager featureManager = (FeatureManager)serviceProvider.GetRequiredService(); // Test telemetry enabled feature with no telemetry publisher - string onFeature = "AlwaysOnTestFeature"; - - bool result = await featureManager.IsEnabledAsync(onFeature, CancellationToken.None); + bool result = await featureManager.IsEnabledAsync(Features.AlwaysOnTestFeature, CancellationToken.None); Assert.True(result); } diff --git a/tests/Tests.FeatureManagement/Features.cs b/tests/Tests.FeatureManagement/Features.cs index b52ec008..eac6505a 100644 --- a/tests/Tests.FeatureManagement/Features.cs +++ b/tests/Tests.FeatureManagement/Features.cs @@ -9,11 +9,15 @@ static class Features public const string TargetingTestFeatureWithExclusion = "TargetingTestFeatureWithExclusion"; public const string OnTestFeature = "OnTestFeature"; public const string OffTestFeature = "OffTestFeature"; + public const string AlwaysOnTestFeature = "AlwaysOnTestFeature"; + public const string OffTimeTestFeature = "OffTimeTestFeature"; public const string ConditionalFeature = "ConditionalFeature"; public const string ConditionalFeature2 = "ConditionalFeature2"; public const string ContextualFeature = "ContextualFeature"; public const string AnyFilterFeature = "AnyFilterFeature"; public const string AllFilterFeature = "AllFilterFeature"; public const string FeatureUsesFiltersWithDuplicatedAlias = "FeatureUsesFiltersWithDuplicatedAlias"; + public const string VariantFeatureDefaultEnabled = "VariantFeatureDefaultEnabled"; + public const string VariantFeatureStatusDisabled = "VariantFeatureStatusDisabled"; } } From 602f32f423cfb82274cc6354ac4ccab8b0dce592 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Tue, 12 Dec 2023 14:45:10 +0800 Subject: [PATCH 03/15] add more testcases & remove some interal methods --- .../ApplicationInsightsTelemetryPublisher.cs | 7 +- .../FeatureManager.cs | 215 +++++++++--------- .../FeatureManagement.cs | 64 ++++-- tests/Tests.FeatureManagement/Features.cs | 9 + .../Tests.FeatureManagement/appsettings.json | 4 + 5 files changed, 167 insertions(+), 132 deletions(-) diff --git a/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs b/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs index cea919c4..cc7327c8 100644 --- a/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs +++ b/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs @@ -38,10 +38,13 @@ public ValueTask PublishEvent(EvaluationEvent evaluationEvent, CancellationToken if (evaluationEvent.Variant != null) { - properties["Variant"] = evaluationEvent.Variant.Name; + properties["Variant"] = evaluationEvent.Variant?.Name ?? String.Empty; } - properties["AssignmentReason"] = evaluationEvent.AssignmentReason.ToString(); + if (evaluationEvent.AssignmentReason != AssignmentReason.None) + { + properties["AssignmentReason"] = evaluationEvent.AssignmentReason.ToString(); + } if (featureDefinition.TelemetryMetadata != null) { diff --git a/src/Microsoft.FeatureManagement/FeatureManager.cs b/src/Microsoft.FeatureManagement/FeatureManager.cs index e725a3fb..a149356c 100644 --- a/src/Microsoft.FeatureManagement/FeatureManager.cs +++ b/src/Microsoft.FeatureManagement/FeatureManager.cs @@ -144,9 +144,11 @@ public TargetingEvaluationOptions AssignerOptions /// /// The name of the feature to check. /// True if the feature is enabled, otherwise false. - public Task IsEnabledAsync(string feature) + public async Task IsEnabledAsync(string feature) { - return IsEnabledEvaluation(feature, appContext: null, useAppContext: false, CancellationToken.None).AsTask(); + EvaluationEvent evaluationEvent = await EvaluateFeature(feature, context: null, useContext: false, CancellationToken.None); + + return evaluationEvent.IsEnabled; } /// @@ -155,9 +157,11 @@ public Task IsEnabledAsync(string feature) /// The name of the feature to check. /// A context providing information that can be used to evaluate whether a feature should be on or off. /// True if the feature is enabled, otherwise false. - public Task IsEnabledAsync(string feature, TContext appContext) + public async Task IsEnabledAsync(string feature, TContext appContext) { - return IsEnabledEvaluation(feature, appContext, useAppContext: true, CancellationToken.None).AsTask(); + EvaluationEvent evaluationEvent = await EvaluateFeature(feature, context: appContext, useContext: true, CancellationToken.None); + + return evaluationEvent.IsEnabled; } /// @@ -166,9 +170,11 @@ public Task IsEnabledAsync(string feature, TContext appContext) /// The name of the feature to check. /// The cancellation token to cancel the operation. /// True if the feature is enabled, otherwise false. - public ValueTask IsEnabledAsync(string feature, CancellationToken cancellationToken) + public async ValueTask IsEnabledAsync(string feature, CancellationToken cancellationToken) { - return IsEnabledEvaluation(feature, appContext: null, useAppContext: false, cancellationToken); + EvaluationEvent evaluationEvent = await EvaluateFeature(feature, context: null, useContext: false, cancellationToken); + + return evaluationEvent.IsEnabled; } /// @@ -178,9 +184,11 @@ public ValueTask IsEnabledAsync(string feature, CancellationToken cancella /// A context providing information that can be used to evaluate whether a feature should be on or off. /// The cancellation token to cancel the operation. /// True if the feature is enabled, otherwise false. - public ValueTask IsEnabledAsync(string feature, TContext appContext, CancellationToken cancellationToken) + public async ValueTask IsEnabledAsync(string feature, TContext appContext, CancellationToken cancellationToken) { - return IsEnabledEvaluation(feature, appContext, useAppContext: true, cancellationToken); + EvaluationEvent evaluationEvent = await EvaluateFeature(feature, context: appContext, useContext: true, cancellationToken); + + return evaluationEvent.IsEnabled; } /// @@ -212,14 +220,16 @@ public async IAsyncEnumerable GetFeatureNamesAsync([EnumeratorCancellati /// The name of the feature to evaluate. /// The cancellation token to cancel the operation. /// A variant assigned to the user based on the feature's configured allocation. - public ValueTask GetVariantAsync(string feature, CancellationToken cancellationToken) + public async ValueTask GetVariantAsync(string feature, CancellationToken cancellationToken) { if (string.IsNullOrEmpty(feature)) { throw new ArgumentNullException(nameof(feature)); } - return GetVariantEvaluation(feature, context: null, useContext: false, cancellationToken); + EvaluationEvent evaluationEvent = await EvaluateFeature(feature, context: null, useContext: false, cancellationToken); + + return evaluationEvent.Variant; } /// @@ -229,7 +239,7 @@ public ValueTask GetVariantAsync(string feature, CancellationToken canc /// An instance of used to evaluate which variant the user will be assigned. /// The cancellation token to cancel the operation. /// A variant assigned to the user based on the feature's configured allocation. - public ValueTask GetVariantAsync(string feature, TargetingContext context, CancellationToken cancellationToken) + public async ValueTask GetVariantAsync(string feature, TargetingContext context, CancellationToken cancellationToken) { if (string.IsNullOrEmpty(feature)) { @@ -241,35 +251,18 @@ public ValueTask GetVariantAsync(string feature, TargetingContext conte throw new ArgumentNullException(nameof(context)); } - return GetVariantEvaluation(feature, context, useContext: true, cancellationToken); - } - - private async ValueTask GetVariantEvaluation(string feature, TargetingContext context, bool useContext, CancellationToken cancellationToken) - { - EvaluationEvent evaluationEvent = new EvaluationEvent - { - FeatureDefinition = await GetFeatureDefinition(feature).ConfigureAwait(false) - }; - - await EvaluateFeature(evaluationEvent, context, useContext, cancellationToken); + EvaluationEvent evaluationEvent = await EvaluateFeature(feature, context, useContext: true, cancellationToken); return evaluationEvent.Variant; } - private async ValueTask IsEnabledEvaluation(string feature, TContext appContext, bool useAppContext, CancellationToken cancellationToken) + private async Task EvaluateFeature(string feature, TContext context, bool useContext, CancellationToken cancellationToken) { - EvaluationEvent evaluationEvent = new EvaluationEvent + var evaluationEvent = new EvaluationEvent { FeatureDefinition = await GetFeatureDefinition(feature).ConfigureAwait(false) }; - await EvaluateFeature(evaluationEvent, appContext, useAppContext, cancellationToken); - - return evaluationEvent.IsEnabled; - } - - private async Task EvaluateFeature(EvaluationEvent evaluationEvent, TContext context, bool useContext, CancellationToken cancellationToken) - { if (evaluationEvent.FeatureDefinition != null) { // @@ -292,7 +285,10 @@ private async Task EvaluateFeature(EvaluationEvent ev { variantDefinition = evaluationEvent.FeatureDefinition.Variants.FirstOrDefault((variant) => variant.Name == evaluationEvent.FeatureDefinition.Allocation.DefaultWhenDisabled); - evaluationEvent.AssignmentReason = AssignmentReason.DisabledDefault; + if (variantDefinition != null) + { + evaluationEvent.AssignmentReason = AssignmentReason.DisabledDefault; + } } else { @@ -307,11 +303,17 @@ private async Task EvaluateFeature(EvaluationEvent ev targetingContext = await ResolveTargetingContextAsync(cancellationToken).ConfigureAwait(false); } - variantDefinition = await GetAssignedVariantAsync( - evaluationEvent, - targetingContext, - cancellationToken) - .ConfigureAwait(false); + variantDefinition = await AssignVariantAsync(evaluationEvent, targetingContext, cancellationToken).ConfigureAwait(false); + + if (variantDefinition == null) + { + variantDefinition = evaluationEvent.FeatureDefinition.Variants.FirstOrDefault((variant) => variant.Name == evaluationEvent.FeatureDefinition.Allocation.DefaultWhenEnabled); + + if (variantDefinition != null) + { + evaluationEvent.AssignmentReason = AssignmentReason.EnabledDefault; + } + } } evaluationEvent.Variant = variantDefinition != null ? GetVariantFromVariantDefinition(variantDefinition) : null; @@ -330,16 +332,16 @@ private async Task EvaluateFeature(EvaluationEvent ev } } } - } - foreach (ISessionManager sessionManager in _sessionManagers) - { - await sessionManager.SetAsync(evaluationEvent.FeatureDefinition.Name, evaluationEvent.IsEnabled).ConfigureAwait(false); - } + foreach (ISessionManager sessionManager in _sessionManagers) + { + await sessionManager.SetAsync(evaluationEvent.FeatureDefinition.Name, evaluationEvent.IsEnabled).ConfigureAwait(false); + } - if (evaluationEvent.FeatureDefinition.TelemetryEnabled) - { - PublishTelemetry(evaluationEvent, cancellationToken); + if (evaluationEvent.FeatureDefinition.TelemetryEnabled) + { + PublishTelemetry(evaluationEvent, cancellationToken); + } } return evaluationEvent; @@ -530,109 +532,98 @@ private async ValueTask ResolveTargetingContextAsync(Cancellat return context; } - private async ValueTask GetAssignedVariantAsync(EvaluationEvent evaluationEvent, TargetingContext context, CancellationToken cancellationToken) - { - VariantDefinition variantDefinition = null; - - if (context != null) - { - variantDefinition = await AssignVariantAsync(evaluationEvent, context, cancellationToken).ConfigureAwait(false); - } - - if (variantDefinition == null) - { - variantDefinition = evaluationEvent.FeatureDefinition.Variants.FirstOrDefault((variant) => variant.Name == evaluationEvent.FeatureDefinition.Allocation.DefaultWhenEnabled); - - evaluationEvent.AssignmentReason = AssignmentReason.EnabledDefault; - } - - return variantDefinition; - } - private ValueTask AssignVariantAsync(EvaluationEvent evaluationEvent, TargetingContext targetingContext, CancellationToken cancellationToken) { VariantDefinition variant = null; - if (evaluationEvent.FeatureDefinition.Allocation.User != null) + if (targetingContext != null) { - foreach (UserAllocation user in evaluationEvent.FeatureDefinition.Allocation.User) + if (evaluationEvent.FeatureDefinition.Allocation.User != null) { - if (TargetingEvaluator.IsTargeted(targetingContext.UserId, user.Users, _assignerOptions.IgnoreCase)) + foreach (UserAllocation user in evaluationEvent.FeatureDefinition.Allocation.User) { - if (string.IsNullOrEmpty(user.Variant)) + if (TargetingEvaluator.IsTargeted(targetingContext.UserId, user.Users, _assignerOptions.IgnoreCase)) { - Logger?.LogWarning($"Missing variant name for user allocation in feature {evaluationEvent.FeatureDefinition.Name}"); + if (string.IsNullOrEmpty(user.Variant)) + { + Logger?.LogWarning($"Missing variant name for user allocation in feature {evaluationEvent.FeatureDefinition.Name}"); - return new ValueTask((VariantDefinition)null); - } + return new ValueTask((VariantDefinition)null); + } - Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); + Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); - evaluationEvent.AssignmentReason = AssignmentReason.User; + evaluationEvent.AssignmentReason = AssignmentReason.User; - return new ValueTask( - evaluationEvent.FeatureDefinition - .Variants - .FirstOrDefault((variant) => variant.Name == user.Variant)); + return new ValueTask( + evaluationEvent.FeatureDefinition + .Variants + .FirstOrDefault((variant) => variant.Name == user.Variant)); + } } } - } - if (evaluationEvent.FeatureDefinition.Allocation.Group != null) - { - foreach (GroupAllocation group in evaluationEvent.FeatureDefinition.Allocation.Group) + if (evaluationEvent.FeatureDefinition.Allocation.Group != null) { - if (TargetingEvaluator.IsTargeted(targetingContext.Groups, group.Groups, _assignerOptions.IgnoreCase)) + foreach (GroupAllocation group in evaluationEvent.FeatureDefinition.Allocation.Group) { - if (string.IsNullOrEmpty(group.Variant)) + if (TargetingEvaluator.IsTargeted(targetingContext.Groups, group.Groups, _assignerOptions.IgnoreCase)) { - Logger?.LogWarning($"Missing variant name for group allocation in feature {evaluationEvent.FeatureDefinition.Name}"); + if (string.IsNullOrEmpty(group.Variant)) + { + Logger?.LogWarning($"Missing variant name for group allocation in feature {evaluationEvent.FeatureDefinition.Name}"); - return new ValueTask((VariantDefinition)null); - } + return new ValueTask((VariantDefinition)null); + } - Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); + Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); - evaluationEvent.AssignmentReason = AssignmentReason.Group; + evaluationEvent.AssignmentReason = AssignmentReason.Group; - return new ValueTask( - evaluationEvent.FeatureDefinition - .Variants - .FirstOrDefault((variant) => variant.Name == group.Variant)); + return new ValueTask( + evaluationEvent.FeatureDefinition + .Variants + .FirstOrDefault((variant) => variant.Name == group.Variant)); + } } } - } - if (evaluationEvent.FeatureDefinition.Allocation.Percentile != null) - { - foreach (PercentileAllocation percentile in evaluationEvent.FeatureDefinition.Allocation.Percentile) + if (evaluationEvent.FeatureDefinition.Allocation.Percentile != null) { - if (TargetingEvaluator.IsTargeted( - targetingContext, - percentile.From, - percentile.To, - _assignerOptions.IgnoreCase, - evaluationEvent.FeatureDefinition.Allocation.Seed ?? $"allocation\n{evaluationEvent.FeatureDefinition.Name}")) + foreach (PercentileAllocation percentile in evaluationEvent.FeatureDefinition.Allocation.Percentile) { - if (string.IsNullOrEmpty(percentile.Variant)) + if (TargetingEvaluator.IsTargeted( + targetingContext, + percentile.From, + percentile.To, + _assignerOptions.IgnoreCase, + evaluationEvent.FeatureDefinition.Allocation.Seed ?? $"allocation\n{evaluationEvent.FeatureDefinition.Name}")) { - Logger?.LogWarning($"Missing variant name for percentile allocation in feature {evaluationEvent.FeatureDefinition.Name}"); + if (string.IsNullOrEmpty(percentile.Variant)) + { + Logger?.LogWarning($"Missing variant name for percentile allocation in feature {evaluationEvent.FeatureDefinition.Name}"); - return new ValueTask((VariantDefinition)null); - } + return new ValueTask((VariantDefinition)null); + } - Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); + Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); - evaluationEvent.AssignmentReason = AssignmentReason.Percentile; + evaluationEvent.AssignmentReason = AssignmentReason.Percentile; - return new ValueTask( - evaluationEvent.FeatureDefinition - .Variants - .FirstOrDefault((variant) => variant.Name == percentile.Variant)); + return new ValueTask( + evaluationEvent.FeatureDefinition + .Variants + .FirstOrDefault((variant) => variant.Name == percentile.Variant)); + } } } } + if (variant == null) + { + evaluationEvent.AssignmentReason = AssignmentReason.None; + } + return new ValueTask(variant); } diff --git a/tests/Tests.FeatureManagement/FeatureManagement.cs b/tests/Tests.FeatureManagement/FeatureManagement.cs index a0684ff5..6f4a5d89 100644 --- a/tests/Tests.FeatureManagement/FeatureManagement.cs +++ b/tests/Tests.FeatureManagement/FeatureManagement.cs @@ -356,7 +356,7 @@ public async Task Percentage() } } - Assert.True(enabledCount >= 0 && enabledCount < 10); + Assert.True(enabledCount >= 0 && enabledCount <= 10); } [Fact] @@ -1037,6 +1037,7 @@ public async Task TelemetryPublishing() Assert.Equal("EtagValue", testPublisher.evaluationEventCache.FeatureDefinition.TelemetryMetadata["Etag"]); Assert.Equal("LabelValue", testPublisher.evaluationEventCache.FeatureDefinition.TelemetryMetadata["Label"]); Assert.Equal("Tag1Value", testPublisher.evaluationEventCache.FeatureDefinition.TelemetryMetadata["Tags.Tag1"]); + Assert.Null(testPublisher.evaluationEventCache.Variant); Assert.Equal(AssignmentReason.None, testPublisher.evaluationEventCache.AssignmentReason); result = await featureManager.IsEnabledAsync(Features.OffTimeTestFeature, CancellationToken.None); @@ -1075,6 +1076,33 @@ public async Task TelemetryPublishing() Assert.Equal(Features.VariantFeatureStatusDisabled, testPublisher.evaluationEventCache.FeatureDefinition.Name); Assert.Equal(variantResult.Name, testPublisher.evaluationEventCache.Variant.Name); Assert.Equal(AssignmentReason.DisabledDefault, testPublisher.evaluationEventCache.AssignmentReason); + + targetingContextAccessor.Current = new TargetingContext + { + UserId = "Marsha", + Groups = new List { "Group1" } + }; + + variantResult = await featureManager.GetVariantAsync(Features.VariantFeaturePercentileOn, CancellationToken.None); + Assert.Equal("Big", variantResult.Name); + Assert.Equal("Big", testPublisher.evaluationEventCache.Variant.Name); + Assert.Equal(AssignmentReason.Percentile, testPublisher.evaluationEventCache.AssignmentReason); + + variantResult = await featureManager.GetVariantAsync(Features.VariantFeaturePercentileOff, CancellationToken.None); + Assert.Null(variantResult); + Assert.Null(testPublisher.evaluationEventCache.Variant); + Assert.Equal(AssignmentReason.None, testPublisher.evaluationEventCache.AssignmentReason); + + variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureUser, CancellationToken.None); + Assert.Equal("Small", variantResult.Name); + Assert.Equal("Small", testPublisher.evaluationEventCache.Variant.Name); + Assert.Equal(AssignmentReason.User, testPublisher.evaluationEventCache.AssignmentReason); + + variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureGroup, CancellationToken.None); + Assert.Equal("Small", variantResult.Name); + Assert.Equal("Small", testPublisher.evaluationEventCache.Variant.Name); + Assert.Equal(AssignmentReason.Group, testPublisher.evaluationEventCache.AssignmentReason); + } [Fact] @@ -1122,44 +1150,44 @@ public async Task UsesVariants() }; // Test StatusOverride and Percentile with Seed - Variant variant = await featureManager.GetVariantAsync("VariantFeaturePercentileOn", cancellationToken); + Variant variant = await featureManager.GetVariantAsync(Features.VariantFeaturePercentileOn, cancellationToken); Assert.Equal("Big", variant.Name); Assert.Equal("green", variant.Configuration["Color"]); - Assert.False(await featureManager.IsEnabledAsync("VariantFeaturePercentileOn", cancellationToken)); + Assert.False(await featureManager.IsEnabledAsync(Features.VariantFeaturePercentileOn, cancellationToken)); - variant = await featureManager.GetVariantAsync("VariantFeaturePercentileOff", cancellationToken); + variant = await featureManager.GetVariantAsync(Features.VariantFeaturePercentileOff, cancellationToken); Assert.Null(variant); - Assert.True(await featureManager.IsEnabledAsync("VariantFeaturePercentileOff", cancellationToken)); + Assert.True(await featureManager.IsEnabledAsync(Features.VariantFeaturePercentileOff, cancellationToken)); // Test Status = Disabled - variant = await featureManager.GetVariantAsync("VariantFeatureStatusDisabled", cancellationToken); + variant = await featureManager.GetVariantAsync(Features.VariantFeatureStatusDisabled, cancellationToken); Assert.Equal("Small", variant.Name); Assert.Equal("300px", variant.Configuration.Value); - Assert.False(await featureManager.IsEnabledAsync("VariantFeatureStatusDisabled", cancellationToken)); + Assert.False(await featureManager.IsEnabledAsync(Features.VariantFeatureStatusDisabled, cancellationToken)); // Test DefaultWhenEnabled and ConfigurationValue with inline IConfigurationSection - variant = await featureManager.GetVariantAsync("VariantFeatureDefaultEnabled", cancellationToken); + variant = await featureManager.GetVariantAsync(Features.VariantFeatureDefaultEnabled, cancellationToken); Assert.Equal("Medium", variant.Name); Assert.Equal("450px", variant.Configuration["Size"]); - Assert.True(await featureManager.IsEnabledAsync("VariantFeatureDefaultEnabled", cancellationToken)); + Assert.True(await featureManager.IsEnabledAsync(Features.VariantFeatureDefaultEnabled, cancellationToken)); // Test User allocation - variant = await featureManager.GetVariantAsync("VariantFeatureUser", cancellationToken); + variant = await featureManager.GetVariantAsync(Features.VariantFeatureUser, cancellationToken); Assert.Equal("Small", variant.Name); Assert.Equal("300px", variant.Configuration.Value); - Assert.True(await featureManager.IsEnabledAsync("VariantFeatureUser", cancellationToken)); + Assert.True(await featureManager.IsEnabledAsync(Features.VariantFeatureUser, cancellationToken)); // Test Group allocation - variant = await featureManager.GetVariantAsync("VariantFeatureGroup", cancellationToken); + variant = await featureManager.GetVariantAsync(Features.VariantFeatureGroup, cancellationToken); Assert.Equal("Small", variant.Name); Assert.Equal("300px", variant.Configuration.Value); - Assert.True(await featureManager.IsEnabledAsync("VariantFeatureGroup", cancellationToken)); + Assert.True(await featureManager.IsEnabledAsync(Features.VariantFeatureGroup, cancellationToken)); } [Fact] @@ -1185,24 +1213,24 @@ public async Task VariantsInvalidScenarios() CancellationToken cancellationToken = CancellationToken.None; // Verify null variant returned if no variants are specified - Variant variant = await featureManager.GetVariantAsync("VariantFeatureNoVariants", cancellationToken); + Variant variant = await featureManager.GetVariantAsync(Features.VariantFeatureNoVariants, cancellationToken); Assert.Null(variant); // Verify null variant returned if no allocation is specified - variant = await featureManager.GetVariantAsync("VariantFeatureNoAllocation", cancellationToken); + variant = await featureManager.GetVariantAsync(Features.VariantFeatureNoAllocation, cancellationToken); Assert.Null(variant); // Verify that ConfigurationValue has priority over ConfigurationReference - variant = await featureManager.GetVariantAsync("VariantFeatureBothConfigurations", cancellationToken); + variant = await featureManager.GetVariantAsync(Features.VariantFeatureBothConfigurations, cancellationToken); Assert.Equal("600px", variant.Configuration.Value); // Verify that an exception is thrown for invalid StatusOverride value FeatureManagementException e = await Assert.ThrowsAsync(async () => { - variant = await featureManager.GetVariantAsync("VariantFeatureInvalidStatusOverride", cancellationToken); + variant = await featureManager.GetVariantAsync(Features.VariantFeatureInvalidStatusOverride, cancellationToken); }); Assert.Equal(FeatureManagementError.InvalidConfigurationSetting, e.Error); @@ -1211,7 +1239,7 @@ public async Task VariantsInvalidScenarios() // Verify that an exception is thrown for invalid doubles From and To in the Percentile section e = await Assert.ThrowsAsync(async () => { - variant = await featureManager.GetVariantAsync("VariantFeatureInvalidFromTo", cancellationToken); + variant = await featureManager.GetVariantAsync(Features.VariantFeatureInvalidFromTo, cancellationToken); }); Assert.Equal(FeatureManagementError.InvalidConfigurationSetting, e.Error); diff --git a/tests/Tests.FeatureManagement/Features.cs b/tests/Tests.FeatureManagement/Features.cs index eac6505a..29fe8bd7 100644 --- a/tests/Tests.FeatureManagement/Features.cs +++ b/tests/Tests.FeatureManagement/Features.cs @@ -19,5 +19,14 @@ static class Features public const string FeatureUsesFiltersWithDuplicatedAlias = "FeatureUsesFiltersWithDuplicatedAlias"; public const string VariantFeatureDefaultEnabled = "VariantFeatureDefaultEnabled"; public const string VariantFeatureStatusDisabled = "VariantFeatureStatusDisabled"; + public const string VariantFeaturePercentileOn = "VariantFeaturePercentileOn"; + public const string VariantFeaturePercentileOff = "VariantFeaturePercentileOff"; + public const string VariantFeatureUser = "VariantFeatureUser"; + public const string VariantFeatureGroup = "VariantFeatureGroup"; + public const string VariantFeatureNoVariants = "VariantFeatureNoVariants"; + public const string VariantFeatureNoAllocation = "VariantFeatureNoAllocation"; + public const string VariantFeatureBothConfigurations = "VariantFeatureBothConfigurations"; + 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 dff27db3..ed998cf6 100644 --- a/tests/Tests.FeatureManagement/appsettings.json +++ b/tests/Tests.FeatureManagement/appsettings.json @@ -199,6 +199,7 @@ }, "VariantFeaturePercentileOn": { + "TelemetryEnabled": true, "Allocation": { "Percentile": [ { @@ -223,6 +224,7 @@ ] }, "VariantFeaturePercentileOff": { + "TelemetryEnabled": true, "Allocation": { "Percentile": [ { @@ -297,6 +299,7 @@ ] }, "VariantFeatureUser": { + "TelemetryEnabled": true, "Allocation": { "User": [ { @@ -320,6 +323,7 @@ ] }, "VariantFeatureGroup": { + "TelemetryEnabled": true, "Allocation": { "User": [ { From 8e6c14d948653ee3602d75eeed66c42ef889784c Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Tue, 12 Dec 2023 14:47:21 +0800 Subject: [PATCH 04/15] fix typo --- .../ApplicationInsightsTelemetryPublisher.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs b/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs index cc7327c8..a3aa5791 100644 --- a/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs +++ b/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs @@ -38,7 +38,7 @@ public ValueTask PublishEvent(EvaluationEvent evaluationEvent, CancellationToken if (evaluationEvent.Variant != null) { - properties["Variant"] = evaluationEvent.Variant?.Name ?? String.Empty; + properties["Variant"] = evaluationEvent.Variant.Name; } if (evaluationEvent.AssignmentReason != AssignmentReason.None) From c1094df50da4d20ac919f7624accd24f9b2d6153 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Tue, 12 Dec 2023 14:56:08 +0800 Subject: [PATCH 05/15] update comments --- .../Telemetry/AssignmentReason.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.FeatureManagement/Telemetry/AssignmentReason.cs b/src/Microsoft.FeatureManagement/Telemetry/AssignmentReason.cs index b3a2c51b..cf6f75b9 100644 --- a/src/Microsoft.FeatureManagement/Telemetry/AssignmentReason.cs +++ b/src/Microsoft.FeatureManagement/Telemetry/AssignmentReason.cs @@ -9,32 +9,32 @@ namespace Microsoft.FeatureManagement.Telemetry public enum AssignmentReason { /// - /// The reason the variant was assigned during the evaluation of a feature. + /// No variant is assigned. /// None, /// - /// The reason the variant was assigned during the evaluation of a feature. + /// Variant is assigned by default after processing the user/group/percentile allocation, when the feature flag is disabled. /// DisabledDefault, /// - /// The reason the variant was assigned during the evaluation of a feature. + /// Variant is assigned by default after processing the user/group/percentile allocation, when the feature flag is enabled. /// EnabledDefault, /// - /// The reason the variant was assigned during the evaluation of a feature. + /// Variant is assigned because of the user allocation. /// User, /// - /// The reason the variant was assigned during the evaluation of a feature. + /// Variant is assigned because of the group allocation. /// Group, /// - /// The reason the variant was assigned during the evaluation of a feature. + /// Variant is assigned because of the percentile allocation. /// Percentile } From 391fef0625dbe052478781ed199e397f972a1a50 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Wed, 13 Dec 2023 15:20:36 +0800 Subject: [PATCH 06/15] resolve comments --- .../ApplicationInsightsTelemetryPublisher.cs | 23 ++- .../FeatureManager.cs | 159 +++++++++--------- 2 files changed, 105 insertions(+), 77 deletions(-) diff --git a/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs b/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs index a3aa5791..f33cddf8 100644 --- a/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs +++ b/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs @@ -43,7 +43,7 @@ public ValueTask PublishEvent(EvaluationEvent evaluationEvent, CancellationToken if (evaluationEvent.AssignmentReason != AssignmentReason.None) { - properties["AssignmentReason"] = evaluationEvent.AssignmentReason.ToString(); + properties["AssignmentReason"] = ToString(evaluationEvent.AssignmentReason); } if (featureDefinition.TelemetryMetadata != null) @@ -71,5 +71,26 @@ private void ValidateEvent(EvaluationEvent evaluationEvent) throw new ArgumentNullException(nameof(evaluationEvent.FeatureDefinition)); } } + + private static string ToString(AssignmentReason reason) + { + const string None = "None"; + const string DisabledDefault = "DisabledDefault"; + const string EnabledDefault = "EnabledDefault"; + const string User = "User"; + const string Group = "Group"; + const string Percentile = "Percentile"; + + return reason switch + { + AssignmentReason.None => None, + AssignmentReason.DisabledDefault => DisabledDefault, + AssignmentReason.EnabledDefault => EnabledDefault, + AssignmentReason.User => User, + AssignmentReason.Group => Group, + AssignmentReason.Percentile => Percentile, + _ => throw new ArgumentException("Invalid assignment reason.", nameof(reason)) + }; + } } } \ No newline at end of file diff --git a/src/Microsoft.FeatureManagement/FeatureManager.cs b/src/Microsoft.FeatureManagement/FeatureManager.cs index a149356c..7a1dbb19 100644 --- a/src/Microsoft.FeatureManagement/FeatureManager.cs +++ b/src/Microsoft.FeatureManagement/FeatureManager.cs @@ -271,26 +271,11 @@ private async Task EvaluateFeature(string feature, TC // // Determine Variant - VariantDefinition variantDefinition; + VariantDefinition variantDefinition = null; - if (evaluationEvent.FeatureDefinition.Allocation == null || (!evaluationEvent.FeatureDefinition.Variants?.Any() ?? false)) + if (evaluationEvent.FeatureDefinition.Allocation != null && (evaluationEvent.FeatureDefinition.Variants?.Any() ?? false)) { - variantDefinition = null; - - evaluationEvent.AssignmentReason = AssignmentReason.None; - } - else - { - if (!evaluationEvent.IsEnabled) - { - variantDefinition = evaluationEvent.FeatureDefinition.Variants.FirstOrDefault((variant) => variant.Name == evaluationEvent.FeatureDefinition.Allocation.DefaultWhenDisabled); - - if (variantDefinition != null) - { - evaluationEvent.AssignmentReason = AssignmentReason.DisabledDefault; - } - } - else + if (evaluationEvent.IsEnabled) { TargetingContext targetingContext; @@ -303,11 +288,17 @@ private async Task EvaluateFeature(string feature, TC targetingContext = await ResolveTargetingContextAsync(cancellationToken).ConfigureAwait(false); } - variantDefinition = await AssignVariantAsync(evaluationEvent, targetingContext, cancellationToken).ConfigureAwait(false); + if (targetingContext != null) + { + variantDefinition = await AssignVariantAsync(evaluationEvent, targetingContext, cancellationToken).ConfigureAwait(false); + } if (variantDefinition == null) { - variantDefinition = evaluationEvent.FeatureDefinition.Variants.FirstOrDefault((variant) => variant.Name == evaluationEvent.FeatureDefinition.Allocation.DefaultWhenEnabled); + variantDefinition = evaluationEvent.FeatureDefinition + .Variants + .FirstOrDefault(variant => + variant.Name == evaluationEvent.FeatureDefinition.Allocation.DefaultWhenEnabled); if (variantDefinition != null) { @@ -315,6 +306,18 @@ private async Task EvaluateFeature(string feature, TC } } } + else + { + variantDefinition = evaluationEvent.FeatureDefinition + .Variants + .FirstOrDefault(variant => + variant.Name == evaluationEvent.FeatureDefinition.Allocation.DefaultWhenDisabled); + + if (variantDefinition != null) + { + evaluationEvent.AssignmentReason = AssignmentReason.DisabledDefault; + } + } evaluationEvent.Variant = variantDefinition != null ? GetVariantFromVariantDefinition(variantDefinition) : null; @@ -333,6 +336,11 @@ private async Task EvaluateFeature(string feature, TC } } + if (variantDefinition == null) + { + evaluationEvent.AssignmentReason = AssignmentReason.None; + } + foreach (ISessionManager sessionManager in _sessionManagers) { await sessionManager.SetAsync(evaluationEvent.FeatureDefinition.Name, evaluationEvent.IsEnabled).ConfigureAwait(false); @@ -534,96 +542,95 @@ private async ValueTask ResolveTargetingContextAsync(Cancellat private ValueTask AssignVariantAsync(EvaluationEvent evaluationEvent, TargetingContext targetingContext, CancellationToken cancellationToken) { + Debug.Assert(evaluationEvent != null); + + Debug.Assert(targetingContext != null); + VariantDefinition variant = null; - if (targetingContext != null) + if (evaluationEvent.FeatureDefinition.Allocation.User != null) { - if (evaluationEvent.FeatureDefinition.Allocation.User != null) + foreach (UserAllocation user in evaluationEvent.FeatureDefinition.Allocation.User) { - foreach (UserAllocation user in evaluationEvent.FeatureDefinition.Allocation.User) + if (TargetingEvaluator.IsTargeted(targetingContext.UserId, user.Users, _assignerOptions.IgnoreCase)) { - if (TargetingEvaluator.IsTargeted(targetingContext.UserId, user.Users, _assignerOptions.IgnoreCase)) + if (string.IsNullOrEmpty(user.Variant)) { - if (string.IsNullOrEmpty(user.Variant)) - { - Logger?.LogWarning($"Missing variant name for user allocation in feature {evaluationEvent.FeatureDefinition.Name}"); + Logger?.LogWarning($"Missing variant name for user allocation in feature {evaluationEvent.FeatureDefinition.Name}"); - return new ValueTask((VariantDefinition)null); - } + return new ValueTask((VariantDefinition)null); + } - Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); + Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); - evaluationEvent.AssignmentReason = AssignmentReason.User; + evaluationEvent.AssignmentReason = AssignmentReason.User; - return new ValueTask( - evaluationEvent.FeatureDefinition - .Variants - .FirstOrDefault((variant) => variant.Name == user.Variant)); - } + return new ValueTask( + evaluationEvent.FeatureDefinition + .Variants + .FirstOrDefault(variant => + variant.Name == user.Variant)); } } + } - if (evaluationEvent.FeatureDefinition.Allocation.Group != null) + if (evaluationEvent.FeatureDefinition.Allocation.Group != null) + { + foreach (GroupAllocation group in evaluationEvent.FeatureDefinition.Allocation.Group) { - foreach (GroupAllocation group in evaluationEvent.FeatureDefinition.Allocation.Group) + if (TargetingEvaluator.IsTargeted(targetingContext.Groups, group.Groups, _assignerOptions.IgnoreCase)) { - if (TargetingEvaluator.IsTargeted(targetingContext.Groups, group.Groups, _assignerOptions.IgnoreCase)) + if (string.IsNullOrEmpty(group.Variant)) { - if (string.IsNullOrEmpty(group.Variant)) - { - Logger?.LogWarning($"Missing variant name for group allocation in feature {evaluationEvent.FeatureDefinition.Name}"); + Logger?.LogWarning($"Missing variant name for group allocation in feature {evaluationEvent.FeatureDefinition.Name}"); - return new ValueTask((VariantDefinition)null); - } + return new ValueTask((VariantDefinition)null); + } - Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); + Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); - evaluationEvent.AssignmentReason = AssignmentReason.Group; + evaluationEvent.AssignmentReason = AssignmentReason.Group; - return new ValueTask( - evaluationEvent.FeatureDefinition - .Variants - .FirstOrDefault((variant) => variant.Name == group.Variant)); - } + return new ValueTask( + evaluationEvent.FeatureDefinition + .Variants + .FirstOrDefault(variant => + variant.Name == group.Variant)); } } + } - if (evaluationEvent.FeatureDefinition.Allocation.Percentile != null) + if (evaluationEvent.FeatureDefinition.Allocation.Percentile != null) + { + foreach (PercentileAllocation percentile in evaluationEvent.FeatureDefinition.Allocation.Percentile) { - foreach (PercentileAllocation percentile in evaluationEvent.FeatureDefinition.Allocation.Percentile) + if (TargetingEvaluator.IsTargeted( + targetingContext, + percentile.From, + percentile.To, + _assignerOptions.IgnoreCase, + evaluationEvent.FeatureDefinition.Allocation.Seed ?? $"allocation\n{evaluationEvent.FeatureDefinition.Name}")) { - if (TargetingEvaluator.IsTargeted( - targetingContext, - percentile.From, - percentile.To, - _assignerOptions.IgnoreCase, - evaluationEvent.FeatureDefinition.Allocation.Seed ?? $"allocation\n{evaluationEvent.FeatureDefinition.Name}")) + if (string.IsNullOrEmpty(percentile.Variant)) { - if (string.IsNullOrEmpty(percentile.Variant)) - { - Logger?.LogWarning($"Missing variant name for percentile allocation in feature {evaluationEvent.FeatureDefinition.Name}"); + Logger?.LogWarning($"Missing variant name for percentile allocation in feature {evaluationEvent.FeatureDefinition.Name}"); - return new ValueTask((VariantDefinition)null); - } + return new ValueTask((VariantDefinition)null); + } - Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); + Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); - evaluationEvent.AssignmentReason = AssignmentReason.Percentile; + evaluationEvent.AssignmentReason = AssignmentReason.Percentile; - return new ValueTask( - evaluationEvent.FeatureDefinition - .Variants - .FirstOrDefault((variant) => variant.Name == percentile.Variant)); - } + return new ValueTask( + evaluationEvent.FeatureDefinition + .Variants + .FirstOrDefault(variant => + variant.Name == percentile.Variant)); } } } - if (variant == null) - { - evaluationEvent.AssignmentReason = AssignmentReason.None; - } - return new ValueTask(variant); } From 263e284717a6c29701a912439ecc22f4f9c2a53e Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Thu, 14 Dec 2023 14:28:10 +0800 Subject: [PATCH 07/15] update comment for DisabledDefault --- src/Microsoft.FeatureManagement/Telemetry/AssignmentReason.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.FeatureManagement/Telemetry/AssignmentReason.cs b/src/Microsoft.FeatureManagement/Telemetry/AssignmentReason.cs index cf6f75b9..d77b152b 100644 --- a/src/Microsoft.FeatureManagement/Telemetry/AssignmentReason.cs +++ b/src/Microsoft.FeatureManagement/Telemetry/AssignmentReason.cs @@ -14,7 +14,7 @@ public enum AssignmentReason None, /// - /// Variant is assigned by default after processing the user/group/percentile allocation, when the feature flag is disabled. + /// Variant is assigned by default when the feature flag is disabled. /// DisabledDefault, From 6ff2a4dc826be1b21739f56bf3e78353ca05294d Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Thu, 14 Dec 2023 16:50:13 +0800 Subject: [PATCH 08/15] fix typo --- .../ApplicationInsightsTelemetryPublisher.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs b/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs index f33cddf8..2f0948d5 100644 --- a/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs +++ b/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs @@ -30,7 +30,7 @@ public ValueTask PublishEvent(EvaluationEvent evaluationEvent, CancellationToken FeatureDefinition featureDefinition = evaluationEvent.FeatureDefinition; - Dictionary properties = new Dictionary() + var properties = new Dictionary() { { "FeatureName", featureDefinition.Name }, { "IsEnabled", evaluationEvent.IsEnabled.ToString() } From ff0b24092fe51c5ceac94d3c67fc7a8e3f1452c0 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Mon, 18 Dec 2023 11:37:41 +0800 Subject: [PATCH 09/15] update --- .../ApplicationInsightsTelemetryPublisher.cs | 24 ++++++------ .../FeatureManager.cs | 39 +++++++------------ .../Telemetry/EvaluationEvent.cs | 4 +- ...ntReason.cs => VariantAssignmentReason.cs} | 8 ++-- .../FeatureManagement.cs | 35 ++++++++++------- tests/Tests.FeatureManagement/Features.cs | 1 + .../Tests.FeatureManagement/appsettings.json | 23 ++++++++++- 7 files changed, 74 insertions(+), 60 deletions(-) rename src/Microsoft.FeatureManagement/Telemetry/{AssignmentReason.cs => VariantAssignmentReason.cs} (85%) diff --git a/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs b/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs index 2f0948d5..90ab2dd5 100644 --- a/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs +++ b/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs @@ -33,7 +33,7 @@ public ValueTask PublishEvent(EvaluationEvent evaluationEvent, CancellationToken var properties = new Dictionary() { { "FeatureName", featureDefinition.Name }, - { "IsEnabled", evaluationEvent.IsEnabled.ToString() } + { "Enabled", evaluationEvent.Enabled.ToString() } }; if (evaluationEvent.Variant != null) @@ -41,9 +41,9 @@ public ValueTask PublishEvent(EvaluationEvent evaluationEvent, CancellationToken properties["Variant"] = evaluationEvent.Variant.Name; } - if (evaluationEvent.AssignmentReason != AssignmentReason.None) + if (evaluationEvent.VariantAssignmentReason != VariantAssignmentReason.None) { - properties["AssignmentReason"] = ToString(evaluationEvent.AssignmentReason); + properties["VariantAssignmentReason"] = ToString(evaluationEvent.VariantAssignmentReason); } if (featureDefinition.TelemetryMetadata != null) @@ -72,23 +72,21 @@ private void ValidateEvent(EvaluationEvent evaluationEvent) } } - private static string ToString(AssignmentReason reason) + private static string ToString(VariantAssignmentReason reason) { - const string None = "None"; - const string DisabledDefault = "DisabledDefault"; - const string EnabledDefault = "EnabledDefault"; + const string DefaultWhenDisabled = "DefaultWhenDisabled"; + const string DefaultWhenEnabled = "DefaultWhenEnabled"; const string User = "User"; const string Group = "Group"; const string Percentile = "Percentile"; return reason switch { - AssignmentReason.None => None, - AssignmentReason.DisabledDefault => DisabledDefault, - AssignmentReason.EnabledDefault => EnabledDefault, - AssignmentReason.User => User, - AssignmentReason.Group => Group, - AssignmentReason.Percentile => Percentile, + VariantAssignmentReason.DefaultWhenDisabled => DefaultWhenDisabled, + VariantAssignmentReason.DefaultWhenEnabled => DefaultWhenEnabled, + VariantAssignmentReason.User => User, + VariantAssignmentReason.Group => Group, + VariantAssignmentReason.Percentile => Percentile, _ => throw new ArgumentException("Invalid assignment reason.", nameof(reason)) }; } diff --git a/src/Microsoft.FeatureManagement/FeatureManager.cs b/src/Microsoft.FeatureManagement/FeatureManager.cs index 7a1dbb19..38fb2df3 100644 --- a/src/Microsoft.FeatureManagement/FeatureManager.cs +++ b/src/Microsoft.FeatureManagement/FeatureManager.cs @@ -148,7 +148,7 @@ public async Task IsEnabledAsync(string feature) { EvaluationEvent evaluationEvent = await EvaluateFeature(feature, context: null, useContext: false, CancellationToken.None); - return evaluationEvent.IsEnabled; + return evaluationEvent.Enabled; } /// @@ -161,7 +161,7 @@ public async Task IsEnabledAsync(string feature, TContext appCon { EvaluationEvent evaluationEvent = await EvaluateFeature(feature, context: appContext, useContext: true, CancellationToken.None); - return evaluationEvent.IsEnabled; + return evaluationEvent.Enabled; } /// @@ -174,7 +174,7 @@ public async ValueTask IsEnabledAsync(string feature, CancellationToken ca { EvaluationEvent evaluationEvent = await EvaluateFeature(feature, context: null, useContext: false, cancellationToken); - return evaluationEvent.IsEnabled; + return evaluationEvent.Enabled; } /// @@ -188,7 +188,7 @@ public async ValueTask IsEnabledAsync(string feature, TContext a { EvaluationEvent evaluationEvent = await EvaluateFeature(feature, context: appContext, useContext: true, cancellationToken); - return evaluationEvent.IsEnabled; + return evaluationEvent.Enabled; } /// @@ -267,7 +267,7 @@ private async Task EvaluateFeature(string feature, TC { // // Determine IsEnabled - evaluationEvent.IsEnabled = await IsEnabledAsync(evaluationEvent.FeatureDefinition, context, useContext, cancellationToken).ConfigureAwait(false); + evaluationEvent.Enabled = await IsEnabledAsync(evaluationEvent.FeatureDefinition, context, useContext, cancellationToken).ConfigureAwait(false); // // Determine Variant @@ -275,7 +275,7 @@ private async Task EvaluateFeature(string feature, TC if (evaluationEvent.FeatureDefinition.Allocation != null && (evaluationEvent.FeatureDefinition.Variants?.Any() ?? false)) { - if (evaluationEvent.IsEnabled) + if (evaluationEvent.Enabled) { TargetingContext targetingContext; @@ -300,10 +300,7 @@ private async Task EvaluateFeature(string feature, TC .FirstOrDefault(variant => variant.Name == evaluationEvent.FeatureDefinition.Allocation.DefaultWhenEnabled); - if (variantDefinition != null) - { - evaluationEvent.AssignmentReason = AssignmentReason.EnabledDefault; - } + evaluationEvent.VariantAssignmentReason = VariantAssignmentReason.DefaultWhenEnabled; } } else @@ -313,10 +310,7 @@ private async Task EvaluateFeature(string feature, TC .FirstOrDefault(variant => variant.Name == evaluationEvent.FeatureDefinition.Allocation.DefaultWhenDisabled); - if (variantDefinition != null) - { - evaluationEvent.AssignmentReason = AssignmentReason.DisabledDefault; - } + evaluationEvent.VariantAssignmentReason = VariantAssignmentReason.DefaultWhenDisabled; } evaluationEvent.Variant = variantDefinition != null ? GetVariantFromVariantDefinition(variantDefinition) : null; @@ -327,23 +321,20 @@ private async Task EvaluateFeature(string feature, TC { if (variantDefinition.StatusOverride == StatusOverride.Enabled) { - evaluationEvent.IsEnabled = true; + evaluationEvent.Enabled = true; } else if (variantDefinition.StatusOverride == StatusOverride.Disabled) { - evaluationEvent.IsEnabled = false; + evaluationEvent.Enabled = false; } } } - if (variantDefinition == null) - { - evaluationEvent.AssignmentReason = AssignmentReason.None; - } + Debug.Assert(evaluationEvent.Variant != null ? evaluationEvent.VariantAssignmentReason != VariantAssignmentReason.None : true); foreach (ISessionManager sessionManager in _sessionManagers) { - await sessionManager.SetAsync(evaluationEvent.FeatureDefinition.Name, evaluationEvent.IsEnabled).ConfigureAwait(false); + await sessionManager.SetAsync(evaluationEvent.FeatureDefinition.Name, evaluationEvent.Enabled).ConfigureAwait(false); } if (evaluationEvent.FeatureDefinition.TelemetryEnabled) @@ -563,7 +554,7 @@ private ValueTask AssignVariantAsync(EvaluationEvent evaluati Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); - evaluationEvent.AssignmentReason = AssignmentReason.User; + evaluationEvent.VariantAssignmentReason = VariantAssignmentReason.User; return new ValueTask( evaluationEvent.FeatureDefinition @@ -589,7 +580,7 @@ private ValueTask AssignVariantAsync(EvaluationEvent evaluati Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); - evaluationEvent.AssignmentReason = AssignmentReason.Group; + evaluationEvent.VariantAssignmentReason = VariantAssignmentReason.Group; return new ValueTask( evaluationEvent.FeatureDefinition @@ -620,7 +611,7 @@ private ValueTask AssignVariantAsync(EvaluationEvent evaluati Debug.Assert(evaluationEvent.FeatureDefinition.Variants != null); - evaluationEvent.AssignmentReason = AssignmentReason.Percentile; + evaluationEvent.VariantAssignmentReason = VariantAssignmentReason.Percentile; return new ValueTask( evaluationEvent.FeatureDefinition diff --git a/src/Microsoft.FeatureManagement/Telemetry/EvaluationEvent.cs b/src/Microsoft.FeatureManagement/Telemetry/EvaluationEvent.cs index 2ca73903..fef2e579 100644 --- a/src/Microsoft.FeatureManagement/Telemetry/EvaluationEvent.cs +++ b/src/Microsoft.FeatureManagement/Telemetry/EvaluationEvent.cs @@ -16,7 +16,7 @@ public class EvaluationEvent /// /// The enabled state of the feature after evaluation. /// - public bool IsEnabled { get; set; } + public bool Enabled { get; set; } /// /// The variant given after evaluation. @@ -26,6 +26,6 @@ public class EvaluationEvent /// /// The reason the variant was assigned. /// - public AssignmentReason AssignmentReason { get; set; } + public VariantAssignmentReason VariantAssignmentReason { get; set; } } } diff --git a/src/Microsoft.FeatureManagement/Telemetry/AssignmentReason.cs b/src/Microsoft.FeatureManagement/Telemetry/VariantAssignmentReason.cs similarity index 85% rename from src/Microsoft.FeatureManagement/Telemetry/AssignmentReason.cs rename to src/Microsoft.FeatureManagement/Telemetry/VariantAssignmentReason.cs index d77b152b..06e41bf7 100644 --- a/src/Microsoft.FeatureManagement/Telemetry/AssignmentReason.cs +++ b/src/Microsoft.FeatureManagement/Telemetry/VariantAssignmentReason.cs @@ -6,22 +6,22 @@ namespace Microsoft.FeatureManagement.Telemetry /// /// The reason the variant was assigned during the evaluation of a feature. /// - public enum AssignmentReason + public enum VariantAssignmentReason { /// - /// No variant is assigned. + /// Variant allocation did not happend. No variant is assigned. /// None, /// /// Variant is assigned by default when the feature flag is disabled. /// - DisabledDefault, + DefaultWhenDisabled, /// /// Variant is assigned by default after processing the user/group/percentile allocation, when the feature flag is enabled. /// - EnabledDefault, + DefaultWhenEnabled, /// /// Variant is assigned because of the user allocation. diff --git a/tests/Tests.FeatureManagement/FeatureManagement.cs b/tests/Tests.FeatureManagement/FeatureManagement.cs index 6f4a5d89..f03a626c 100644 --- a/tests/Tests.FeatureManagement/FeatureManagement.cs +++ b/tests/Tests.FeatureManagement/FeatureManagement.cs @@ -1033,49 +1033,49 @@ public async Task TelemetryPublishing() Assert.True(result); Assert.Equal(Features.AlwaysOnTestFeature, testPublisher.evaluationEventCache.FeatureDefinition.Name); - Assert.Equal(result, testPublisher.evaluationEventCache.IsEnabled); + Assert.Equal(result, testPublisher.evaluationEventCache.Enabled); Assert.Equal("EtagValue", testPublisher.evaluationEventCache.FeatureDefinition.TelemetryMetadata["Etag"]); Assert.Equal("LabelValue", testPublisher.evaluationEventCache.FeatureDefinition.TelemetryMetadata["Label"]); Assert.Equal("Tag1Value", testPublisher.evaluationEventCache.FeatureDefinition.TelemetryMetadata["Tags.Tag1"]); Assert.Null(testPublisher.evaluationEventCache.Variant); - Assert.Equal(AssignmentReason.None, testPublisher.evaluationEventCache.AssignmentReason); + Assert.Equal(VariantAssignmentReason.None, testPublisher.evaluationEventCache.VariantAssignmentReason); result = await featureManager.IsEnabledAsync(Features.OffTimeTestFeature, CancellationToken.None); Assert.False(result); Assert.Equal(Features.OffTimeTestFeature, testPublisher.evaluationEventCache.FeatureDefinition.Name); - Assert.Equal(result, testPublisher.evaluationEventCache.IsEnabled); - Assert.Equal(AssignmentReason.None, testPublisher.evaluationEventCache.AssignmentReason); + Assert.Equal(result, testPublisher.evaluationEventCache.Enabled); + Assert.Equal(VariantAssignmentReason.None, testPublisher.evaluationEventCache.VariantAssignmentReason); // Test variant cases result = await featureManager.IsEnabledAsync(Features.VariantFeatureDefaultEnabled, CancellationToken.None); Assert.True(result); Assert.Equal(Features.VariantFeatureDefaultEnabled, testPublisher.evaluationEventCache.FeatureDefinition.Name); - Assert.Equal(result, testPublisher.evaluationEventCache.IsEnabled); + Assert.Equal(result, testPublisher.evaluationEventCache.Enabled); Assert.Equal("Medium", testPublisher.evaluationEventCache.Variant.Name); Variant variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureDefaultEnabled, CancellationToken.None); - Assert.True(testPublisher.evaluationEventCache.IsEnabled); + Assert.True(testPublisher.evaluationEventCache.Enabled); Assert.Equal(Features.VariantFeatureDefaultEnabled, testPublisher.evaluationEventCache.FeatureDefinition.Name); Assert.Equal(variantResult.Name, testPublisher.evaluationEventCache.Variant.Name); - Assert.Equal(AssignmentReason.EnabledDefault, testPublisher.evaluationEventCache.AssignmentReason); + Assert.Equal(VariantAssignmentReason.DefaultWhenEnabled, testPublisher.evaluationEventCache.VariantAssignmentReason); result = await featureManager.IsEnabledAsync(Features.VariantFeatureStatusDisabled, CancellationToken.None); Assert.False(result); Assert.Equal(Features.VariantFeatureStatusDisabled, testPublisher.evaluationEventCache.FeatureDefinition.Name); - Assert.Equal(result, testPublisher.evaluationEventCache.IsEnabled); + Assert.Equal(result, testPublisher.evaluationEventCache.Enabled); Assert.Equal("Small", testPublisher.evaluationEventCache.Variant.Name); - Assert.Equal(AssignmentReason.DisabledDefault, testPublisher.evaluationEventCache.AssignmentReason); + Assert.Equal(VariantAssignmentReason.DefaultWhenDisabled, testPublisher.evaluationEventCache.VariantAssignmentReason); variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureStatusDisabled, CancellationToken.None); - Assert.False(testPublisher.evaluationEventCache.IsEnabled); + Assert.False(testPublisher.evaluationEventCache.Enabled); Assert.Equal(Features.VariantFeatureStatusDisabled, testPublisher.evaluationEventCache.FeatureDefinition.Name); Assert.Equal(variantResult.Name, testPublisher.evaluationEventCache.Variant.Name); - Assert.Equal(AssignmentReason.DisabledDefault, testPublisher.evaluationEventCache.AssignmentReason); + Assert.Equal(VariantAssignmentReason.DefaultWhenDisabled, testPublisher.evaluationEventCache.VariantAssignmentReason); targetingContextAccessor.Current = new TargetingContext { @@ -1086,22 +1086,27 @@ public async Task TelemetryPublishing() variantResult = await featureManager.GetVariantAsync(Features.VariantFeaturePercentileOn, CancellationToken.None); Assert.Equal("Big", variantResult.Name); Assert.Equal("Big", testPublisher.evaluationEventCache.Variant.Name); - Assert.Equal(AssignmentReason.Percentile, testPublisher.evaluationEventCache.AssignmentReason); + Assert.Equal(VariantAssignmentReason.Percentile, testPublisher.evaluationEventCache.VariantAssignmentReason); variantResult = await featureManager.GetVariantAsync(Features.VariantFeaturePercentileOff, CancellationToken.None); Assert.Null(variantResult); Assert.Null(testPublisher.evaluationEventCache.Variant); - Assert.Equal(AssignmentReason.None, testPublisher.evaluationEventCache.AssignmentReason); + Assert.Equal(VariantAssignmentReason.DefaultWhenEnabled, testPublisher.evaluationEventCache.VariantAssignmentReason); + + variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureAlwaysOff, CancellationToken.None); + Assert.Null(variantResult); + Assert.Null(testPublisher.evaluationEventCache.Variant); + Assert.Equal(VariantAssignmentReason.DefaultWhenDisabled, testPublisher.evaluationEventCache.VariantAssignmentReason); variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureUser, CancellationToken.None); Assert.Equal("Small", variantResult.Name); Assert.Equal("Small", testPublisher.evaluationEventCache.Variant.Name); - Assert.Equal(AssignmentReason.User, testPublisher.evaluationEventCache.AssignmentReason); + Assert.Equal(VariantAssignmentReason.User, testPublisher.evaluationEventCache.VariantAssignmentReason); variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureGroup, CancellationToken.None); Assert.Equal("Small", variantResult.Name); Assert.Equal("Small", testPublisher.evaluationEventCache.Variant.Name); - Assert.Equal(AssignmentReason.Group, testPublisher.evaluationEventCache.AssignmentReason); + Assert.Equal(VariantAssignmentReason.Group, testPublisher.evaluationEventCache.VariantAssignmentReason); } diff --git a/tests/Tests.FeatureManagement/Features.cs b/tests/Tests.FeatureManagement/Features.cs index 29fe8bd7..4add8779 100644 --- a/tests/Tests.FeatureManagement/Features.cs +++ b/tests/Tests.FeatureManagement/Features.cs @@ -21,6 +21,7 @@ static class Features public const string VariantFeatureStatusDisabled = "VariantFeatureStatusDisabled"; public const string VariantFeaturePercentileOn = "VariantFeaturePercentileOn"; public const string VariantFeaturePercentileOff = "VariantFeaturePercentileOff"; + public const string VariantFeatureAlwaysOff = "VariantFeatureAlwaysOff"; public const string VariantFeatureUser = "VariantFeatureUser"; public const string VariantFeatureGroup = "VariantFeatureGroup"; public const string VariantFeatureNoVariants = "VariantFeatureNoVariants"; diff --git a/tests/Tests.FeatureManagement/appsettings.json b/tests/Tests.FeatureManagement/appsettings.json index ed998cf6..fb63b02a 100644 --- a/tests/Tests.FeatureManagement/appsettings.json +++ b/tests/Tests.FeatureManagement/appsettings.json @@ -238,8 +238,7 @@ "Variants": [ { "Name": "Big", - "ConfigurationReference": "ShoppingCart:Big", - "StatusOverride": "Disabled" + "ConfigurationReference": "ShoppingCart:Big" } ], "EnabledFor": [ @@ -248,6 +247,26 @@ } ] }, + "VariantFeatureAlwaysOff": { + "TelemetryEnabled": true, + "Allocation": { + "Percentile": [ + { + "Variant": "Big", + "From": 0, + "To": 100 + } + ], + "Seed": 12345 + }, + "Variants": [ + { + "Name": "Big", + "ConfigurationReference": "ShoppingCart:Big" + } + ], + "EnabledFor": [] + }, "VariantFeatureStatusDisabled": { "Status": "Disabled", "TelemetryEnabled": true, From 6660d095fa203b71fdf34e241b09fbbe0d472975 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Tue, 19 Dec 2023 11:19:37 +0800 Subject: [PATCH 10/15] update telemetry publisher --- .../ApplicationInsightsTelemetryPublisher.cs | 10 +++++----- src/Microsoft.FeatureManagement/FeatureManager.cs | 2 -- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs b/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs index 90ab2dd5..75935b10 100644 --- a/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs +++ b/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. // using Microsoft.ApplicationInsights; +using System.Diagnostics; namespace Microsoft.FeatureManagement.Telemetry.ApplicationInsights { @@ -36,13 +37,10 @@ public ValueTask PublishEvent(EvaluationEvent evaluationEvent, CancellationToken { "Enabled", evaluationEvent.Enabled.ToString() } }; - if (evaluationEvent.Variant != null) - { - properties["Variant"] = evaluationEvent.Variant.Name; - } - if (evaluationEvent.VariantAssignmentReason != VariantAssignmentReason.None) { + properties["Variant"] = evaluationEvent.Variant?.Name ?? string.Empty; + properties["VariantAssignmentReason"] = ToString(evaluationEvent.VariantAssignmentReason); } @@ -74,6 +72,8 @@ private void ValidateEvent(EvaluationEvent evaluationEvent) private static string ToString(VariantAssignmentReason reason) { + Debug.Assert(reason != VariantAssignmentReason.None); + const string DefaultWhenDisabled = "DefaultWhenDisabled"; const string DefaultWhenEnabled = "DefaultWhenEnabled"; const string User = "User"; diff --git a/src/Microsoft.FeatureManagement/FeatureManager.cs b/src/Microsoft.FeatureManagement/FeatureManager.cs index 38fb2df3..e0685ac3 100644 --- a/src/Microsoft.FeatureManagement/FeatureManager.cs +++ b/src/Microsoft.FeatureManagement/FeatureManager.cs @@ -330,8 +330,6 @@ private async Task EvaluateFeature(string feature, TC } } - Debug.Assert(evaluationEvent.Variant != null ? evaluationEvent.VariantAssignmentReason != VariantAssignmentReason.None : true); - foreach (ISessionManager sessionManager in _sessionManagers) { await sessionManager.SetAsync(evaluationEvent.FeatureDefinition.Name, evaluationEvent.Enabled).ConfigureAwait(false); From cd1dc64b8d661658ff27a669af7824288609756f Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Tue, 19 Dec 2023 23:13:53 +0800 Subject: [PATCH 11/15] add reason when no allocation section --- .../FeatureManager.cs | 30 +- .../FeatureManagement.cs | 35 +- tests/Tests.FeatureManagement/Features.cs | 1 + .../Tests.FeatureManagement/appsettings.json | 860 +++++++++--------- 4 files changed, 478 insertions(+), 448 deletions(-) diff --git a/src/Microsoft.FeatureManagement/FeatureManager.cs b/src/Microsoft.FeatureManagement/FeatureManager.cs index e0685ac3..b30b578e 100644 --- a/src/Microsoft.FeatureManagement/FeatureManager.cs +++ b/src/Microsoft.FeatureManagement/FeatureManager.cs @@ -273,7 +273,7 @@ private async Task EvaluateFeature(string feature, TC // Determine Variant VariantDefinition variantDefinition = null; - if (evaluationEvent.FeatureDefinition.Allocation != null && (evaluationEvent.FeatureDefinition.Variants?.Any() ?? false)) + if (evaluationEvent.FeatureDefinition.Variants?.Any() ?? false) { if (evaluationEvent.Enabled) { @@ -295,20 +295,26 @@ private async Task EvaluateFeature(string feature, TC if (variantDefinition == null) { - variantDefinition = evaluationEvent.FeatureDefinition - .Variants - .FirstOrDefault(variant => - variant.Name == evaluationEvent.FeatureDefinition.Allocation.DefaultWhenEnabled); + if (evaluationEvent.FeatureDefinition.Allocation?.DefaultWhenEnabled != null) + { + variantDefinition = evaluationEvent.FeatureDefinition + .Variants + .FirstOrDefault(variant => + variant.Name == evaluationEvent.FeatureDefinition.Allocation.DefaultWhenEnabled); + } evaluationEvent.VariantAssignmentReason = VariantAssignmentReason.DefaultWhenEnabled; } } else { - variantDefinition = evaluationEvent.FeatureDefinition - .Variants - .FirstOrDefault(variant => - variant.Name == evaluationEvent.FeatureDefinition.Allocation.DefaultWhenDisabled); + if (evaluationEvent.FeatureDefinition.Allocation?.DefaultWhenDisabled != null) + { + variantDefinition = evaluationEvent.FeatureDefinition + .Variants + .FirstOrDefault(variant => + variant.Name == evaluationEvent.FeatureDefinition.Allocation.DefaultWhenDisabled); + } evaluationEvent.VariantAssignmentReason = VariantAssignmentReason.DefaultWhenDisabled; } @@ -537,7 +543,7 @@ private ValueTask AssignVariantAsync(EvaluationEvent evaluati VariantDefinition variant = null; - if (evaluationEvent.FeatureDefinition.Allocation.User != null) + if (evaluationEvent.FeatureDefinition.Allocation?.User != null) { foreach (UserAllocation user in evaluationEvent.FeatureDefinition.Allocation.User) { @@ -563,7 +569,7 @@ private ValueTask AssignVariantAsync(EvaluationEvent evaluati } } - if (evaluationEvent.FeatureDefinition.Allocation.Group != null) + if (evaluationEvent.FeatureDefinition.Allocation?.Group != null) { foreach (GroupAllocation group in evaluationEvent.FeatureDefinition.Allocation.Group) { @@ -589,7 +595,7 @@ private ValueTask AssignVariantAsync(EvaluationEvent evaluati } } - if (evaluationEvent.FeatureDefinition.Allocation.Percentile != null) + if (evaluationEvent.FeatureDefinition.Allocation?.Percentile != null) { foreach (PercentileAllocation percentile in evaluationEvent.FeatureDefinition.Allocation.Percentile) { diff --git a/tests/Tests.FeatureManagement/FeatureManagement.cs b/tests/Tests.FeatureManagement/FeatureManagement.cs index f03a626c..0a7aa357 100644 --- a/tests/Tests.FeatureManagement/FeatureManagement.cs +++ b/tests/Tests.FeatureManagement/FeatureManagement.cs @@ -1021,15 +1021,16 @@ public async Task TelemetryPublishing() FeatureManager featureManager = (FeatureManager) serviceProvider.GetRequiredService(); TestTelemetryPublisher testPublisher = (TestTelemetryPublisher) featureManager.TelemetryPublishers.First(); + CancellationToken cancellationToken = CancellationToken.None; // Test a feature with telemetry disabled - bool result = await featureManager.IsEnabledAsync(Features.OnTestFeature, CancellationToken.None); + bool result = await featureManager.IsEnabledAsync(Features.OnTestFeature, cancellationToken); Assert.True(result); Assert.Null(testPublisher.evaluationEventCache); // Test telemetry cases - result = await featureManager.IsEnabledAsync(Features.AlwaysOnTestFeature, CancellationToken.None); + result = await featureManager.IsEnabledAsync(Features.AlwaysOnTestFeature, cancellationToken); Assert.True(result); Assert.Equal(Features.AlwaysOnTestFeature, testPublisher.evaluationEventCache.FeatureDefinition.Name); @@ -1040,7 +1041,7 @@ public async Task TelemetryPublishing() Assert.Null(testPublisher.evaluationEventCache.Variant); Assert.Equal(VariantAssignmentReason.None, testPublisher.evaluationEventCache.VariantAssignmentReason); - result = await featureManager.IsEnabledAsync(Features.OffTimeTestFeature, CancellationToken.None); + result = await featureManager.IsEnabledAsync(Features.OffTimeTestFeature, cancellationToken); Assert.False(result); Assert.Equal(Features.OffTimeTestFeature, testPublisher.evaluationEventCache.FeatureDefinition.Name); @@ -1048,21 +1049,21 @@ public async Task TelemetryPublishing() Assert.Equal(VariantAssignmentReason.None, testPublisher.evaluationEventCache.VariantAssignmentReason); // Test variant cases - result = await featureManager.IsEnabledAsync(Features.VariantFeatureDefaultEnabled, CancellationToken.None); + result = await featureManager.IsEnabledAsync(Features.VariantFeatureDefaultEnabled, cancellationToken); Assert.True(result); Assert.Equal(Features.VariantFeatureDefaultEnabled, testPublisher.evaluationEventCache.FeatureDefinition.Name); Assert.Equal(result, testPublisher.evaluationEventCache.Enabled); Assert.Equal("Medium", testPublisher.evaluationEventCache.Variant.Name); - Variant variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureDefaultEnabled, CancellationToken.None); + Variant variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureDefaultEnabled, cancellationToken); Assert.True(testPublisher.evaluationEventCache.Enabled); Assert.Equal(Features.VariantFeatureDefaultEnabled, testPublisher.evaluationEventCache.FeatureDefinition.Name); Assert.Equal(variantResult.Name, testPublisher.evaluationEventCache.Variant.Name); Assert.Equal(VariantAssignmentReason.DefaultWhenEnabled, testPublisher.evaluationEventCache.VariantAssignmentReason); - result = await featureManager.IsEnabledAsync(Features.VariantFeatureStatusDisabled, CancellationToken.None); + result = await featureManager.IsEnabledAsync(Features.VariantFeatureStatusDisabled, cancellationToken); Assert.False(result); Assert.Equal(Features.VariantFeatureStatusDisabled, testPublisher.evaluationEventCache.FeatureDefinition.Name); @@ -1070,7 +1071,7 @@ public async Task TelemetryPublishing() Assert.Equal("Small", testPublisher.evaluationEventCache.Variant.Name); Assert.Equal(VariantAssignmentReason.DefaultWhenDisabled, testPublisher.evaluationEventCache.VariantAssignmentReason); - variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureStatusDisabled, CancellationToken.None); + variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureStatusDisabled, cancellationToken); Assert.False(testPublisher.evaluationEventCache.Enabled); Assert.Equal(Features.VariantFeatureStatusDisabled, testPublisher.evaluationEventCache.FeatureDefinition.Name); @@ -1083,31 +1084,41 @@ public async Task TelemetryPublishing() Groups = new List { "Group1" } }; - variantResult = await featureManager.GetVariantAsync(Features.VariantFeaturePercentileOn, CancellationToken.None); + variantResult = await featureManager.GetVariantAsync(Features.VariantFeaturePercentileOn, cancellationToken); Assert.Equal("Big", variantResult.Name); Assert.Equal("Big", testPublisher.evaluationEventCache.Variant.Name); Assert.Equal(VariantAssignmentReason.Percentile, testPublisher.evaluationEventCache.VariantAssignmentReason); - variantResult = await featureManager.GetVariantAsync(Features.VariantFeaturePercentileOff, CancellationToken.None); + variantResult = await featureManager.GetVariantAsync(Features.VariantFeaturePercentileOff, cancellationToken); Assert.Null(variantResult); Assert.Null(testPublisher.evaluationEventCache.Variant); Assert.Equal(VariantAssignmentReason.DefaultWhenEnabled, testPublisher.evaluationEventCache.VariantAssignmentReason); - variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureAlwaysOff, CancellationToken.None); + variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureAlwaysOff, cancellationToken); Assert.Null(variantResult); Assert.Null(testPublisher.evaluationEventCache.Variant); Assert.Equal(VariantAssignmentReason.DefaultWhenDisabled, testPublisher.evaluationEventCache.VariantAssignmentReason); - variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureUser, CancellationToken.None); + variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureUser, cancellationToken); Assert.Equal("Small", variantResult.Name); Assert.Equal("Small", testPublisher.evaluationEventCache.Variant.Name); Assert.Equal(VariantAssignmentReason.User, testPublisher.evaluationEventCache.VariantAssignmentReason); - variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureGroup, CancellationToken.None); + variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureGroup, cancellationToken); Assert.Equal("Small", variantResult.Name); Assert.Equal("Small", testPublisher.evaluationEventCache.Variant.Name); Assert.Equal(VariantAssignmentReason.Group, testPublisher.evaluationEventCache.VariantAssignmentReason); + variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureNoAllocation, cancellationToken); + Assert.Null(variantResult); + Assert.Null(testPublisher.evaluationEventCache.Variant); + Assert.Equal(VariantAssignmentReason.DefaultWhenEnabled, testPublisher.evaluationEventCache.VariantAssignmentReason); + + variantResult = await featureManager.GetVariantAsync(Features.VariantFeatureAlwaysOffNoAllocation, cancellationToken); + Assert.Null(variantResult); + Assert.Null(testPublisher.evaluationEventCache.Variant); + Assert.Equal(VariantAssignmentReason.DefaultWhenDisabled, testPublisher.evaluationEventCache.VariantAssignmentReason); + } [Fact] diff --git a/tests/Tests.FeatureManagement/Features.cs b/tests/Tests.FeatureManagement/Features.cs index 4add8779..d3d2b81f 100644 --- a/tests/Tests.FeatureManagement/Features.cs +++ b/tests/Tests.FeatureManagement/Features.cs @@ -26,6 +26,7 @@ static class Features public const string VariantFeatureGroup = "VariantFeatureGroup"; public const string VariantFeatureNoVariants = "VariantFeatureNoVariants"; public const string VariantFeatureNoAllocation = "VariantFeatureNoAllocation"; + public const string VariantFeatureAlwaysOffNoAllocation = "VariantFeatureAlwaysOffNoAllocation"; public const string VariantFeatureBothConfigurations = "VariantFeatureBothConfigurations"; 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 fb63b02a..c7eed721 100644 --- a/tests/Tests.FeatureManagement/appsettings.json +++ b/tests/Tests.FeatureManagement/appsettings.json @@ -17,448 +17,460 @@ } }, - "FeatureManagement": { - "OnTestFeature": true, - "OffTestFeature": false, - "AlwaysOnTestFeature": { - "TelemetryEnabled": true, - "EnabledFor": [ - { - "Name": "AlwaysOn" - } - ], - "TelemetryMetadata": { - "Tags.Tag1": "Tag1Value", - "Tags.Tag2": "Tag2Value", - "Etag": "EtagValue", - "Label": "LabelValue" - } - }, - "OffTimeTestFeature": { - "TelemetryEnabled": true, - "EnabledFor": [ - { - "Name": "TimeWindow", - "Parameters": { - "End": "1970-01-01T00:00:00Z" - } - } - ] - }, - "FeatureUsesFiltersWithDuplicatedAlias": { - "RequirementType": "all", - "EnabledFor": [ - { - "Name": "DuplicatedFilterName" + "FeatureManagement": { + "OnTestFeature": true, + "OffTestFeature": false, + "AlwaysOnTestFeature": { + "TelemetryEnabled": true, + "EnabledFor": [ + { + "Name": "AlwaysOn" + } + ], + "TelemetryMetadata": { + "Tags.Tag1": "Tag1Value", + "Tags.Tag2": "Tag2Value", + "Etag": "EtagValue", + "Label": "LabelValue" + } }, - { - "Name": "Percentage", - "Parameters": { - "Value": 100 - } - } - ] - }, - "TargetingTestFeature": { - "EnabledFor": [ - { - "Name": "Targeting", - "Parameters": { - "Audience": { - "Users": [ - "Jeff", - "Alicia" - ], - "Groups": [ - { - "Name": "Ring0", - "RolloutPercentage": 100 + "OffTimeTestFeature": { + "TelemetryEnabled": true, + "EnabledFor": [ + { + "Name": "TimeWindow", + "Parameters": { + "End": "1970-01-01T00:00:00Z" + } + } + ] + }, + "FeatureUsesFiltersWithDuplicatedAlias": { + "RequirementType": "all", + "EnabledFor": [ + { + "Name": "DuplicatedFilterName" }, { - "Name": "Ring1", - "RolloutPercentage": 50 + "Name": "Percentage", + "Parameters": { + "Value": 100 + } } - ], - "DefaultRolloutPercentage": 20 - } - } - } - ] - }, - "TargetingTestFeatureWithExclusion": { - "EnabledFor": [ - { - "Name": "Targeting", - "Parameters": { - "Audience": { - "Users": [ - "Jeff", - "Alicia" - ], - "Groups": [ - { - "Name": "Ring0", - "RolloutPercentage": 100 + ] + }, + "TargetingTestFeature": { + "EnabledFor": [ + { + "Name": "Targeting", + "Parameters": { + "Audience": { + "Users": [ + "Jeff", + "Alicia" + ], + "Groups": [ + { + "Name": "Ring0", + "RolloutPercentage": 100 + }, + { + "Name": "Ring1", + "RolloutPercentage": 50 + } + ], + "DefaultRolloutPercentage": 20 + } + } + } + ] + }, + "TargetingTestFeatureWithExclusion": { + "EnabledFor": [ + { + "Name": "Targeting", + "Parameters": { + "Audience": { + "Users": [ + "Jeff", + "Alicia" + ], + "Groups": [ + { + "Name": "Ring0", + "RolloutPercentage": 100 + }, + { + "Name": "Ring1", + "RolloutPercentage": 50 + } + ], + "DefaultRolloutPercentage": 20, + "Exclusion": { + "Users": [ + "Jeff" + ], + "Groups": [ + "Ring0", + "Ring2" + ] + } + } + } + } + ] + }, + "CustomFilterFeature": { + "EnabledFor": [ + { + "Name": "CustomTargetingFilter", + "Parameters": { + "Audience": { + "Users": [ + "Jeff" + ] + } + } + } + ] + }, + "ConditionalFeature": { + "EnabledFor": [ + { + "Name": "Test", + "Parameters": { + "P1": "V1" + } + } + ] + }, + "ConditionalFeature2": { + "EnabledFor": [ + { + "Name": "Test" + } + ] + }, + "ContextualFeature": { + "EnabledFor": [ + { + "Name": "ContextualTest", + "Parameters": { + "AllowedAccounts": [ + "abc" + ] + } + } + ] + }, + "AnyFilterFeature": { + "RequirementType": "Any", + "EnabledFor": [ + { + "Name": "Test", + "Parameters": { + "Id": "1" + } }, { - "Name": "Ring1", - "RolloutPercentage": 50 + "Name": "Test", + "Parameters": { + "Id": "2" + } + } + ] + }, + "AllFilterFeature": { + "RequirementType": "all", + "EnabledFor": [ + { + "Name": "Test", + "Parameters": { + "Id": "1" + } + }, + { + "Name": "Test", + "Parameters": { + "Id": "2" + } + } + ] + + }, + "VariantFeaturePercentileOn": { + "TelemetryEnabled": true, + "Allocation": { + "Percentile": [ + { + "Variant": "Big", + "From": 0, + "To": 50 + } + ], + "Seed": 1234 + }, + "Variants": [ + { + "Name": "Big", + "ConfigurationReference": "ShoppingCart:Big", + "StatusOverride": "Disabled" + } + ], + "EnabledFor": [ + { + "Name": "On" } - ], - "DefaultRolloutPercentage": 20, - "Exclusion": { - "Users": [ - "Jeff" + ] + }, + "VariantFeaturePercentileOff": { + "TelemetryEnabled": true, + "Allocation": { + "Percentile": [ + { + "Variant": "Big", + "From": 0, + "To": 50 + } ], - "Groups": [ - "Ring0", - "Ring2" + "Seed": 12345 + }, + "Variants": [ + { + "Name": "Big", + "ConfigurationReference": "ShoppingCart:Big" + } + ], + "EnabledFor": [ + { + "Name": "On" + } + ] + }, + "VariantFeatureAlwaysOff": { + "TelemetryEnabled": true, + "Allocation": { + "Percentile": [ + { + "Variant": "Big", + "From": 0, + "To": 100 + } + ], + "Seed": 12345 + }, + "Variants": [ + { + "Name": "Big", + "ConfigurationReference": "ShoppingCart:Big" + } + ], + "EnabledFor": [] + }, + "VariantFeatureStatusDisabled": { + "Status": "Disabled", + "TelemetryEnabled": true, + "Allocation": { + "DefaultWhenDisabled": "Small" + }, + "Variants": [ + { + "Name": "Small", + "ConfigurationValue": "300px" + } + ], + "EnabledFor": [ + { + "Name": "On" + } + ] + }, + "VariantFeatureDefaultEnabled": { + "TelemetryEnabled": true, + "Allocation": { + "DefaultWhenEnabled": "Medium", + "User": [ + { + "Variant": "Small", + "Users": [ + "Jeff" + ] + } ] - } - } - } - } - ] - }, - "CustomFilterFeature": { - "EnabledFor": [ - { - "Name": "CustomTargetingFilter", - "Parameters": { - "Audience": { - "Users": [ - "Jeff" - ] - } - } - } - ] - }, - "ConditionalFeature": { - "EnabledFor": [ - { - "Name": "Test", - "Parameters": { - "P1": "V1" - } - } - ] - }, - "ConditionalFeature2": { - "EnabledFor": [ - { - "Name": "Test" - } - ] - }, - "ContextualFeature": { - "EnabledFor": [ - { - "Name": "ContextualTest", - "Parameters": { - "AllowedAccounts": [ - "abc" + }, + "Variants": [ + { + "Name": "Medium", + "ConfigurationValue": { + "Size": "450px", + "Color": "Purple" + } + }, + { + "Name": "Small", + "ConfigurationValue": "300px" + } + ], + "EnabledFor": [ + { + "Name": "On" + } ] - } - } - ] - }, - "AnyFilterFeature": { - "RequirementType": "Any", - "EnabledFor": [ - { - "Name": "Test", - "Parameters": { - "Id": "1" - } }, - { - "Name": "Test", - "Parameters": { - "Id": "2" - } - } - ] - }, - "AllFilterFeature": { - "RequirementType": "all", - "EnabledFor": [ - { - "Name": "Test", - "Parameters": { - "Id": "1" - } + "VariantFeatureUser": { + "TelemetryEnabled": true, + "Allocation": { + "User": [ + { + "Variant": "Small", + "Users": [ + "Marsha" + ] + } + ] + }, + "Variants": [ + { + "Name": "Small", + "ConfigurationValue": "300px" + } + ], + "EnabledFor": [ + { + "Name": "On" + } + ] }, - { - "Name": "Test", - "Parameters": { - "Id": "2" - } - } - ] - - }, - "VariantFeaturePercentileOn": { - "TelemetryEnabled": true, - "Allocation": { - "Percentile": [ - { - "Variant": "Big", - "From": 0, - "To": 50 - } - ], - "Seed": 1234 - }, - "Variants": [ - { - "Name": "Big", - "ConfigurationReference": "ShoppingCart:Big", - "StatusOverride": "Disabled" - } - ], - "EnabledFor": [ - { - "Name": "On" - } - ] - }, - "VariantFeaturePercentileOff": { - "TelemetryEnabled": true, - "Allocation": { - "Percentile": [ - { - "Variant": "Big", - "From": 0, - "To": 50 - } - ], - "Seed": 12345 - }, - "Variants": [ - { - "Name": "Big", - "ConfigurationReference": "ShoppingCart:Big" - } - ], - "EnabledFor": [ - { - "Name": "On" - } - ] - }, - "VariantFeatureAlwaysOff": { - "TelemetryEnabled": true, - "Allocation": { - "Percentile": [ - { - "Variant": "Big", - "From": 0, - "To": 100 - } - ], - "Seed": 12345 - }, - "Variants": [ - { - "Name": "Big", - "ConfigurationReference": "ShoppingCart:Big" - } - ], - "EnabledFor": [] - }, - "VariantFeatureStatusDisabled": { - "Status": "Disabled", - "TelemetryEnabled": true, - "Allocation": { - "DefaultWhenDisabled": "Small" - }, - "Variants": [ - { - "Name": "Small", - "ConfigurationValue": "300px" - } - ], - "EnabledFor": [ - { - "Name": "On" - } - ] - }, - "VariantFeatureDefaultEnabled": { - "TelemetryEnabled": true, - "Allocation": { - "DefaultWhenEnabled": "Medium", - "User": [ - { - "Variant": "Small", - "Users": [ - "Jeff" + "VariantFeatureGroup": { + "TelemetryEnabled": true, + "Allocation": { + "User": [ + { + "Variant": "Small", + "Users": [ + "Jeff" + ] + } + ], + "Group": [ + { + "Variant": "Small", + "Groups": [ + "Group1" + ] + } + ] + }, + "Variants": [ + { + "Name": "Small", + "ConfigurationValue": "300px" + } + ], + "EnabledFor": [ + { + "Name": "On" + } ] - } - ] - }, - "Variants": [ - { - "Name": "Medium", - "ConfigurationValue": { - "Size": "450px", - "Color": "Purple" - } }, - { - "Name": "Small", - "ConfigurationValue": "300px" - } - ], - "EnabledFor": [ - { - "Name": "On" - } - ] - }, - "VariantFeatureUser": { - "TelemetryEnabled": true, - "Allocation": { - "User": [ - { - "Variant": "Small", - "Users": [ - "Marsha" + "VariantFeatureNoVariants": { + "Allocation": { + "User": [ + { + "Variant": "Small", + "Users": [ + "Marsha" + ] + } + ] + }, + "Variants": [], + "EnabledFor": [ + { + "Name": "On" + } ] - } - ] - }, - "Variants": [ - { - "Name": "Small", - "ConfigurationValue": "300px" - } - ], - "EnabledFor": [ - { - "Name": "On" - } - ] - }, - "VariantFeatureGroup": { - "TelemetryEnabled": true, - "Allocation": { - "User": [ - { - "Variant": "Small", - "Users": [ - "Jeff" + }, + "VariantFeatureBothConfigurations": { + "Allocation": { + "DefaultWhenEnabled": "Small" + }, + "Variants": [ + { + "Name": "Small", + "ConfigurationValue": "600px", + "ConfigurationReference": "ShoppingCart:Small" + } + ], + "EnabledFor": [ + { + "Name": "On" + } ] - } - ], - "Group": [ - { - "Variant": "Small", - "Groups": [ - "Group1" + }, + "VariantFeatureNoAllocation": { + "TelemetryEnabled": true, + "Variants": [ + { + "Name": "Small", + "ConfigurationValue": "300px" + } + ], + "EnabledFor": [ + { + "Name": "On" + } ] - } - ] - }, - "Variants": [ - { - "Name": "Small", - "ConfigurationValue": "300px" - } - ], - "EnabledFor": [ - { - "Name": "On" - } - ] - }, - "VariantFeatureNoVariants": { - "Allocation": { - "User": [ - { - "Variant": "Small", - "Users": [ - "Marsha" + }, + "VariantFeatureAlwaysOffNoAllocation": { + "TelemetryEnabled": true, + "Variants": [ + { + "Name": "Small", + "ConfigurationValue": "300px" + } + ], + "EnabledFor": [ + ] + }, + "VariantFeatureInvalidStatusOverride": { + "Allocation": { + "DefaultWhenEnabled": "Small" + }, + "Variants": [ + { + "Name": "Small", + "ConfigurationValue": "300px", + "StatusOverride": "InvalidValue" + } + ], + "EnabledFor": [ + { + "Name": "On" + } + ] + }, + "VariantFeatureInvalidFromTo": { + "Allocation": { + "Percentile": [ + { + "Variant": "Small", + "From": "Invalid", + "To": "Invalid" + } + ] + }, + "Variants": [ + { + "Name": "Small", + "ConfigurationReference": "ShoppingCart:Small" + } + ], + "EnabledFor": [ + { + "Name": "On" + } ] - } - ] - }, - "Variants": [], - "EnabledFor": [ - { - "Name": "On" - } - ] - }, - "VariantFeatureBothConfigurations": { - "Allocation": { - "DefaultWhenEnabled": "Small" - }, - "Variants": [ - { - "Name": "Small", - "ConfigurationValue": "600px", - "ConfigurationReference": "ShoppingCart:Small" - } - ], - "EnabledFor": [ - { - "Name": "On" - } - ] - }, - "VariantFeatureNoAllocation": { - "Variants": [ - { - "Name": "Small", - "ConfigurationValue": "300px" - } - ], - "EnabledFor": [ - { - "Name": "On" - } - ] - }, - "VariantFeatureInvalidStatusOverride": { - "Allocation": { - "DefaultWhenEnabled": "Small" - }, - "Variants": [ - { - "Name": "Small", - "ConfigurationValue": "300px", - "StatusOverride": "InvalidValue" - } - ], - "EnabledFor": [ - { - "Name": "On" - } - ] - }, - "VariantFeatureInvalidFromTo": { - "Allocation": { - "Percentile": [ - { - "Variant": "Small", - "From": "Invalid", - "To": "Invalid" - } - ] - }, - "Variants": [ - { - "Name": "Small", - "ConfigurationReference": "ShoppingCart:Small" - } - ], - "EnabledFor": [ - { - "Name": "On" } - ] } - } } From b26628a05e7e73dc943251ac988de3f577bb0ae1 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Wed, 20 Dec 2023 00:18:44 +0800 Subject: [PATCH 12/15] update --- .../FeatureManager.cs | 49 +++++++++++-------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/src/Microsoft.FeatureManagement/FeatureManager.cs b/src/Microsoft.FeatureManagement/FeatureManager.cs index b30b578e..d8e7d243 100644 --- a/src/Microsoft.FeatureManagement/FeatureManager.cs +++ b/src/Microsoft.FeatureManagement/FeatureManager.cs @@ -273,9 +273,28 @@ private async Task EvaluateFeature(string feature, TC // Determine Variant VariantDefinition variantDefinition = null; - if (evaluationEvent.FeatureDefinition.Variants?.Any() ?? false) + if (evaluationEvent.FeatureDefinition.Variants != null && + evaluationEvent.FeatureDefinition.Variants.Any()) { - if (evaluationEvent.Enabled) + if (evaluationEvent.FeatureDefinition.Allocation == null) + { + evaluationEvent.VariantAssignmentReason = evaluationEvent.Enabled ? + VariantAssignmentReason.DefaultWhenEnabled : + VariantAssignmentReason.DefaultWhenDisabled; + } + else if (!evaluationEvent.Enabled) + { + if (evaluationEvent.FeatureDefinition.Allocation.DefaultWhenDisabled != null) + { + variantDefinition = evaluationEvent.FeatureDefinition + .Variants + .FirstOrDefault(variant => + variant.Name == evaluationEvent.FeatureDefinition.Allocation.DefaultWhenDisabled); + } + + evaluationEvent.VariantAssignmentReason = VariantAssignmentReason.DefaultWhenDisabled; + } + else { TargetingContext targetingContext; @@ -288,14 +307,14 @@ private async Task EvaluateFeature(string feature, TC targetingContext = await ResolveTargetingContextAsync(cancellationToken).ConfigureAwait(false); } - if (targetingContext != null) + if (targetingContext != null && evaluationEvent.FeatureDefinition.Allocation != null) { variantDefinition = await AssignVariantAsync(evaluationEvent, targetingContext, cancellationToken).ConfigureAwait(false); } if (variantDefinition == null) { - if (evaluationEvent.FeatureDefinition.Allocation?.DefaultWhenEnabled != null) + if (evaluationEvent.FeatureDefinition.Allocation.DefaultWhenEnabled != null) { variantDefinition = evaluationEvent.FeatureDefinition .Variants @@ -305,19 +324,7 @@ private async Task EvaluateFeature(string feature, TC evaluationEvent.VariantAssignmentReason = VariantAssignmentReason.DefaultWhenEnabled; } - } - else - { - if (evaluationEvent.FeatureDefinition.Allocation?.DefaultWhenDisabled != null) - { - variantDefinition = evaluationEvent.FeatureDefinition - .Variants - .FirstOrDefault(variant => - variant.Name == evaluationEvent.FeatureDefinition.Allocation.DefaultWhenDisabled); - } - - evaluationEvent.VariantAssignmentReason = VariantAssignmentReason.DefaultWhenDisabled; - } + } evaluationEvent.Variant = variantDefinition != null ? GetVariantFromVariantDefinition(variantDefinition) : null; @@ -541,9 +548,11 @@ private ValueTask AssignVariantAsync(EvaluationEvent evaluati Debug.Assert(targetingContext != null); + Debug.Assert(evaluationEvent.FeatureDefinition.Allocation != null); + VariantDefinition variant = null; - if (evaluationEvent.FeatureDefinition.Allocation?.User != null) + if (evaluationEvent.FeatureDefinition.Allocation.User != null) { foreach (UserAllocation user in evaluationEvent.FeatureDefinition.Allocation.User) { @@ -569,7 +578,7 @@ private ValueTask AssignVariantAsync(EvaluationEvent evaluati } } - if (evaluationEvent.FeatureDefinition.Allocation?.Group != null) + if (evaluationEvent.FeatureDefinition.Allocation.Group != null) { foreach (GroupAllocation group in evaluationEvent.FeatureDefinition.Allocation.Group) { @@ -595,7 +604,7 @@ private ValueTask AssignVariantAsync(EvaluationEvent evaluati } } - if (evaluationEvent.FeatureDefinition.Allocation?.Percentile != null) + if (evaluationEvent.FeatureDefinition.Allocation.Percentile != null) { foreach (PercentileAllocation percentile in evaluationEvent.FeatureDefinition.Allocation.Percentile) { From 4a3f7cf03963aea1dbbbd092ba84cb8e8db335d0 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Wed, 20 Dec 2023 00:22:28 +0800 Subject: [PATCH 13/15] remove nullable --- .../ApplicationInsightsTelemetryPublisher.cs | 2 +- ...osoft.FeatureManagement.Telemetry.ApplicationInsights.csproj | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs b/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs index 75935b10..77605462 100644 --- a/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs +++ b/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs @@ -39,7 +39,7 @@ public ValueTask PublishEvent(EvaluationEvent evaluationEvent, CancellationToken if (evaluationEvent.VariantAssignmentReason != VariantAssignmentReason.None) { - properties["Variant"] = evaluationEvent.Variant?.Name ?? string.Empty; + properties["Variant"] = evaluationEvent.Variant?.Name; properties["VariantAssignmentReason"] = ToString(evaluationEvent.VariantAssignmentReason); } diff --git a/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/Microsoft.FeatureManagement.Telemetry.ApplicationInsights.csproj b/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/Microsoft.FeatureManagement.Telemetry.ApplicationInsights.csproj index ea1077cb..43a107f9 100644 --- a/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/Microsoft.FeatureManagement.Telemetry.ApplicationInsights.csproj +++ b/src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/Microsoft.FeatureManagement.Telemetry.ApplicationInsights.csproj @@ -3,7 +3,6 @@ net6.0 enable - enable From 3007179b48fa3d1ac7496398ae813d8f0534e3af Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Wed, 20 Dec 2023 11:07:20 +0800 Subject: [PATCH 14/15] resolve comments --- src/Microsoft.FeatureManagement/FeatureManager.cs | 2 +- .../Telemetry/VariantAssignmentReason.cs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.FeatureManagement/FeatureManager.cs b/src/Microsoft.FeatureManagement/FeatureManager.cs index d8e7d243..ff0671f3 100644 --- a/src/Microsoft.FeatureManagement/FeatureManager.cs +++ b/src/Microsoft.FeatureManagement/FeatureManager.cs @@ -312,7 +312,7 @@ private async Task EvaluateFeature(string feature, TC variantDefinition = await AssignVariantAsync(evaluationEvent, targetingContext, cancellationToken).ConfigureAwait(false); } - if (variantDefinition == null) + if (evaluationEvent.VariantAssignmentReason == VariantAssignmentReason.None) { if (evaluationEvent.FeatureDefinition.Allocation.DefaultWhenEnabled != null) { diff --git a/src/Microsoft.FeatureManagement/Telemetry/VariantAssignmentReason.cs b/src/Microsoft.FeatureManagement/Telemetry/VariantAssignmentReason.cs index 06e41bf7..a4db27d4 100644 --- a/src/Microsoft.FeatureManagement/Telemetry/VariantAssignmentReason.cs +++ b/src/Microsoft.FeatureManagement/Telemetry/VariantAssignmentReason.cs @@ -14,27 +14,27 @@ public enum VariantAssignmentReason None, /// - /// Variant is assigned by default when the feature flag is disabled. + /// The default variant is assigned when a feature flag is disabled. /// DefaultWhenDisabled, /// - /// Variant is assigned by default after processing the user/group/percentile allocation, when the feature flag is enabled. + /// The default variant is assigned because of no applicable user/group/percentile allocation when a feature flag is enabled. /// DefaultWhenEnabled, /// - /// Variant is assigned because of the user allocation. + /// The variant is assigned because of the user allocation when a feature flag is enabled. /// User, /// - /// Variant is assigned because of the group allocation. + /// The variant is assigned because of the group allocation when a feature flag is enabled. /// Group, /// - /// Variant is assigned because of the percentile allocation. + /// The variant is assigned because of the percentile allocation when a feature flag is enabled. /// Percentile } From 3649fd090600faeaf68cee0b08d529e3c5b366a5 Mon Sep 17 00:00:00 2001 From: zhiyuanliang Date: Wed, 20 Dec 2023 11:37:38 +0800 Subject: [PATCH 15/15] update comments --- src/Microsoft.FeatureManagement/Telemetry/EvaluationEvent.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.FeatureManagement/Telemetry/EvaluationEvent.cs b/src/Microsoft.FeatureManagement/Telemetry/EvaluationEvent.cs index fef2e579..aca0cdfc 100644 --- a/src/Microsoft.FeatureManagement/Telemetry/EvaluationEvent.cs +++ b/src/Microsoft.FeatureManagement/Telemetry/EvaluationEvent.cs @@ -24,7 +24,7 @@ public class EvaluationEvent public Variant Variant { get; set; } /// - /// The reason the variant was assigned. + /// The reason why the variant was assigned. /// public VariantAssignmentReason VariantAssignmentReason { get; set; } }