fix(experiment): don't process if empty or tainted

src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/IExperimentConfiguration.cs

@@ -28,4 +28,15 @@ public interface IExperimentConfiguration
     /// <param name="componentDetector">The detector.</param>
     /// <returns><c>true</c> if the detector is in the experiment group; otherwise, <c>false</c>.</returns>
     bool IsInExperimentGroup(IComponentDetector componentDetector);
+
+    /// <summary>
+    /// Determines if the experiment should be recorded, given a list of all the detectors that ran and the number of
+    /// components that were detected. If any call to this method returns <c>false</c>, the experiment will not be
+    /// recorded.
+    /// </summary>
+    /// <example><see cref="NpmLockfile3Experiment.ShouldRecord"/>.</example>
+    /// <param name="componentDetector">The component detector.</param>
+    /// <param name="numComponents">The number of components found by the <paramref name="componentDetector"/>.</param>
+    /// <returns><c>true</c> if the experiment should be recorded; otherwise, <c>false</c>.</returns>
+    bool ShouldRecord(IComponentDetector componentDetector, int numComponents);
 }

src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/NewNugetExperiment.cs

@@ -18,4 +18,7 @@ public bool IsInControlGroup(IComponentDetector componentDetector) =>
     /// <inheritdoc />
     public bool IsInExperimentGroup(IComponentDetector componentDetector) =>
         componentDetector is NuGetProjectModelProjectCentricComponentDetector or NuGetPackagesConfigDetector;
+
+    /// <inheritdoc />
+    public bool ShouldRecord(IComponentDetector componentDetector, int numComponents) => true;
 }

src/Microsoft.ComponentDetection.Orchestrator/Experiments/Configs/NpmLockfile3Experiment.cs

@@ -16,4 +16,8 @@ public class NpmLockfile3Experiment : IExperimentConfiguration
 
     /// <inheritdoc />
     public bool IsInExperimentGroup(IComponentDetector componentDetector) => componentDetector is NpmLockfile3Detector;
+
+    /// <inheritdoc />
+    public bool ShouldRecord(IComponentDetector componentDetector, int numComponents) =>
+        componentDetector is not NpmComponentDetector || numComponents == 0;
 }

src/Microsoft.ComponentDetection.Orchestrator/Experiments/ExperimentService.cs

@@ -35,6 +35,8 @@ public ExperimentService(
     /// <inheritdoc />
     public void RecordDetectorRun(IComponentDetector detector, IEnumerable<DetectedComponent> components)
     {
+        this.FilterExperiments(detector, components.Count());
+
         foreach (var (config, experimentResults) in this.experiments)
         {
             if (config.IsInControlGroup(detector))
@@ -59,21 +61,42 @@ public void RecordDetectorRun(IComponentDetector detector, IEnumerable<DetectedC
         }
     }
 
+    private void FilterExperiments(IComponentDetector detector, int count) =>
+        this.experiments.RemoveAll(experiment =>
+        {
+            var shouldRemove = !experiment.Config.ShouldRecord(detector, count);
+
+            if (shouldRemove)
+            {
+                this.logger.LogDebug("Removing {Experiment} from active experiments", experiment.Config.Name);
+            }
+
+            return shouldRemove;
+        });
+
     /// <inheritdoc />
     public async Task FinishAsync()
     {
         foreach (var (config, experiment) in this.experiments)
         {
-            var oldComponents = experiment.ControlGroupComponents;
-            var newComponents = experiment.ExperimentGroupComponents;
+            var controlComponents = experiment.ControlGroupComponents;
+            var experimentComponents = experiment.ExperimentGroupComponents;
 
             this.logger.LogInformation(
-                "Experiment {Experiment} finished and has {Count} components in the control group and {Count} components in the experiment group.",
+                "Experiment {Experiment} finished with {ControlCount} components in the control group and {ExperimentCount} components in the experiment group",
                 config.Name,
-                oldComponents.Count,
-                newComponents.Count);
+                controlComponents.Count,
+                experimentComponents.Count);
+
+            // If there are no components recorded in the experiment, skip processing experiments. We still want to
+            // process empty diffs as this means the experiment was successful.
+            if (!experimentComponents.Any() && !controlComponents.Any())
+            {
+                this.logger.LogWarning("Experiment {Experiment} has no components in either group, skipping processing", config.Name);
+                continue;
+            }
 
-            var diff = new ExperimentDiff(experiment.ControlGroupComponents, experiment.ExperimentGroupComponents);
+            var diff = new ExperimentDiff(controlComponents, experimentComponents);
 
             foreach (var processor in this.experimentProcessors)
             {

test/Microsoft.ComponentDetection.Orchestrator.Tests/Experiments/ExperimentServiceTests.cs

@@ -28,16 +28,17 @@ public ExperimentServiceTests()
         this.experimentProcessorMock = new Mock<IExperimentProcessor>();
         this.loggerMock = new Mock<ILogger<ExperimentService>>();
         this.detectorMock = new Mock<IComponentDetector>();
+
+        this.experimentConfigMock.Setup(x => x.IsInControlGroup(this.detectorMock.Object)).Returns(true);
+        this.experimentConfigMock.Setup(x => x.IsInExperimentGroup(this.detectorMock.Object)).Returns(true);
+        this.experimentConfigMock.Setup(x => x.ShouldRecord(this.detectorMock.Object, It.IsAny<int>())).Returns(true);
     }
 
     [TestMethod]
     public void RecordDetectorRun_AddsComponentsToControlAndExperimentGroup()
     {
         var components = ExperimentTestUtils.CreateRandomComponents();
 
-        this.experimentConfigMock.Setup(x => x.IsInControlGroup(this.detectorMock.Object)).Returns(true);
-        this.experimentConfigMock.Setup(x => x.IsInExperimentGroup(this.detectorMock.Object)).Returns(true);
-
         var service = new ExperimentService(
             new[] { this.experimentConfigMock.Object },
             Enumerable.Empty<IExperimentProcessor>(),
@@ -50,12 +51,38 @@ public void RecordDetectorRun_AddsComponentsToControlAndExperimentGroup()
     }
 
     [TestMethod]
-    public async Task FinishAsync_ProcessesExperimentsAsync()
+    public async Task RecordDetectorRun_FiltersExperimentsAsync()
     {
+        var filterConfigMock = new Mock<IExperimentConfiguration>();
+        filterConfigMock
+            .Setup(x => x.ShouldRecord(It.IsAny<IComponentDetector>(), It.IsAny<int>()))
+            .Returns(false);
+        filterConfigMock.Setup(x => x.IsInControlGroup(this.detectorMock.Object)).Returns(true);
+        filterConfigMock.Setup(x => x.IsInExperimentGroup(this.detectorMock.Object)).Returns(true);
+
         var components = ExperimentTestUtils.CreateRandomComponents();
 
-        this.experimentConfigMock.Setup(x => x.IsInControlGroup(this.detectorMock.Object)).Returns(true);
-        this.experimentConfigMock.Setup(x => x.IsInExperimentGroup(this.detectorMock.Object)).Returns(true);
+        var service = new ExperimentService(
+            new[] { this.experimentConfigMock.Object, filterConfigMock.Object },
+            new[] { this.experimentProcessorMock.Object },
+            this.loggerMock.Object);
+
+        service.RecordDetectorRun(this.detectorMock.Object, components);
+        await service.FinishAsync();
+
+        filterConfigMock.Verify(x => x.ShouldRecord(this.detectorMock.Object, components.Count), Times.Once());
+        this.experimentProcessorMock.Verify(
+            x => x.ProcessExperimentAsync(filterConfigMock.Object, It.IsAny<ExperimentDiff>()),
+            Times.Never());
+        this.experimentProcessorMock.Verify(
+            x => x.ProcessExperimentAsync(this.experimentConfigMock.Object, It.IsAny<ExperimentDiff>()),
+            Times.Once());
+    }
+
+    [TestMethod]
+    public async Task FinishAsync_ProcessesExperimentsAsync()
+    {
+        var components = ExperimentTestUtils.CreateRandomComponents();
 
         var service = new ExperimentService(
             new[] { this.experimentConfigMock.Object },
@@ -80,9 +107,6 @@ public async Task FinishAsync_SwallowsExceptionsAsync()
 
         var components = ExperimentTestUtils.CreateRandomComponents();
 
-        this.experimentConfigMock.Setup(x => x.IsInControlGroup(this.detectorMock.Object)).Returns(true);
-        this.experimentConfigMock.Setup(x => x.IsInExperimentGroup(this.detectorMock.Object)).Returns(true);
-
         var service = new ExperimentService(
             new[] { this.experimentConfigMock.Object },
             new[] { this.experimentProcessorMock.Object },
@@ -92,4 +116,18 @@ public async Task FinishAsync_SwallowsExceptionsAsync()
         var act = async () => await service.FinishAsync();
         await act.Should().NotThrowAsync<IOException>();
     }
+
+    [TestMethod]
+    public async Task FinishAsync_SkipsEmptyExperimentsAsync()
+    {
+        var service = new ExperimentService(
+            new[] { this.experimentConfigMock.Object },
+            new[] { this.experimentProcessorMock.Object },
+            this.loggerMock.Object);
+        await service.FinishAsync();
+
+        this.experimentProcessorMock.Verify(
+            x => x.ProcessExperimentAsync(It.IsAny<IExperimentConfiguration>(), It.IsAny<ExperimentDiff>()),
+            Times.Never());
+    }
 }