Merge pull request #55239 from CyrusNajmabadi/treatmentVariables

CyrusNajmabadi · web-flow · commit 3a06c1248078 · 2021-07-29T15:49:58.000-07:00
Add support for the new 'treatment variable' approach to A/B testing.
diff --git a/src/VisualStudio/Core/Def/Experimentation/VisualStudioExperimentationService.cs b/src/VisualStudio/Core/Def/Experimentation/VisualStudioExperimentationService.cs
@@ -2,8 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-#nullable disable
-
 using System;
 using System.Collections.Generic;
 using System.Composition;
@@ -13,16 +11,17 @@
 using Microsoft.CodeAnalysis.Host.Mef;
 using Microsoft.Internal.VisualStudio.Shell.Interop;
 using Microsoft.VisualStudio.Shell;
+using VSExperimentation = Microsoft.VisualStudio.Experimentation;
 
 namespace Microsoft.VisualStudio.LanguageServices.Experimentation
 {
     [Export(typeof(VisualStudioExperimentationService))]
     [ExportWorkspaceService(typeof(IExperimentationService), ServiceLayer.Host), Shared]
     internal class VisualStudioExperimentationService : ForegroundThreadAffinitizedObject, IExperimentationService
     {
-        private readonly object _experimentationServiceOpt;
-        private readonly MethodInfo _isCachedFlightEnabledInfo;
-        private readonly IVsFeatureFlags _featureFlags;
+        private readonly object? _experimentationServiceOpt;
+        private readonly MethodInfo? _isCachedFlightEnabledInfo;
+        private readonly IVsFeatureFlags? _featureFlags;
 
         /// <summary>
         /// Cache of values we've queried from the underlying VS service.  These values are expected to last for the
@@ -36,15 +35,15 @@ internal class VisualStudioExperimentationService : ForegroundThreadAffinitizedO
         public VisualStudioExperimentationService(IThreadingContext threadingContext, SVsServiceProvider serviceProvider)
             : base(threadingContext)
         {
-            object experimentationServiceOpt = null;
-            MethodInfo isCachedFlightEnabledInfo = null;
-            IVsFeatureFlags featureFlags = null;
+            object? experimentationServiceOpt = null;
+            MethodInfo? isCachedFlightEnabledInfo = null;
+            IVsFeatureFlags? featureFlags = null;
 
             threadingContext.JoinableTaskFactory.Run(async () =>
             {
                 try
                 {
-                    featureFlags = (IVsFeatureFlags)await ((IAsyncServiceProvider)serviceProvider).GetServiceAsync(typeof(SVsFeatureFlags)).ConfigureAwait(false);
+                    featureFlags = (IVsFeatureFlags?)await ((IAsyncServiceProvider)serviceProvider).GetServiceAsync(typeof(SVsFeatureFlags)).ConfigureAwait(false);
                     experimentationServiceOpt = await ((IAsyncServiceProvider)serviceProvider).GetServiceAsync(typeof(SVsExperimentationService)).ConfigureAwait(false);
                     if (experimentationServiceOpt != null)
                     {
@@ -71,8 +70,8 @@ public void EnableExperiment(string experimentName, bool value)
                     _experimentEnabledMap.Remove(experimentName);
             }
 
-            var featureFlags2 = (IVsFeatureFlags2)_featureFlags;
-            featureFlags2.EnableFeatureFlag(experimentName, value);
+            var featureFlags2 = (IVsFeatureFlags2?)_featureFlags;
+            featureFlags2?.EnableFeatureFlag(experimentName, value);
         }
 
         public bool IsExperimentEnabled(string experimentName)
@@ -101,36 +100,49 @@ public bool IsExperimentEnabled(string experimentName)
         private bool IsExperimentEnabledWorker(string experimentName)
         {
             ThisCanBeCalledOnAnyThread();
-            if (_isCachedFlightEnabledInfo != null)
+
+            // First check feature flags.
+            try
             {
-                try
+                // check whether "." exist in the experimentName since it is requirement for featureflag service.
+                // we do this since RPS complains about resource file being loaded for invalid name exception
+                // we are not testing all rules but just simple "." check
+                if (experimentName.IndexOf(".") > 0 && _featureFlags != null && _featureFlags.IsFeatureEnabled(experimentName, defaultValue: false))
+                    return true;
+            }
+            catch
+            {
+                // featureFlags can throw if given name is in incorrect format which can happen for us
+                // since we use this for experimentation service as well
+            }
+
+            // Then the legacy 'targetted notification system'.
+            try
+            {
+                if (_isCachedFlightEnabledInfo != null && (bool)_isCachedFlightEnabledInfo.Invoke(_experimentationServiceOpt, new object[] { experimentName }))
+                    return true;
+            }
+            catch
+            {
+            }
+
+            // Finally, the modern 'treatment variable' system.
+            try
+            {
+                if (VSExperimentation.ExperimentationService.Default is VSExperimentation.IExperimentationService3 experimentationService)
                 {
-                    // check whether "." exist in the experimentName since it is requirement for featureflag service.
-                    // we do this since RPS complains about resource file being loaded for invalid name exception
-                    // we are not testing all rules but just simple "." check
-                    if (experimentName.IndexOf(".") > 0)
+                    // Get from the well known 'VisualStudio' set of treatment variables.
+                    var treatmentVariables = experimentationService.GetCachedTreatmentVariables("VisualStudio");
+                    if (treatmentVariables != null &&
+                        treatmentVariables.TryGetValue(experimentName, out var value) &&
+                        value is true)
                     {
-                        var enabled = _featureFlags.IsFeatureEnabled(experimentName, defaultValue: false);
-                        if (enabled)
-                        {
-                            return enabled;
-                        }
+                        return true;
                     }
                 }
-                catch
-                {
-                    // featureFlags can throw if given name is in incorrect format which can happen for us
-                    // since we use this for experimentation service as well
-                }
-
-                try
-                {
-                    return (bool)_isCachedFlightEnabledInfo.Invoke(_experimentationServiceOpt, new object[] { experimentName });
-                }
-                catch
-                {
-
-                }
+            }
+            catch
+            {
             }
 
             return false;
