Merge pull request #135 from zacharygoodwin/PROC-1314

PROC-1314: Add interface for logging proctor results

proctor-common/src/main/java/com/indeed/proctor/common/AbstractProctorLoader.java

@@ -207,13 +207,28 @@ public Proctor doLoad() throws IOException, MissingTestMatrixException {
         }
 
         final Proctor proctor =
-                Proctor.construct(testMatrix, loadResult, functionMapper, identifierValidator);
+                Proctor.construct(
+                        testMatrix,
+                        loadResult,
+                        functionMapper,
+                        identifierValidator,
+                        getProctorResultReporter());
         //  kind of lame to modify lastAudit here but current in load(), but the interface is a
         // little constraining
         setLastAudit(newAudit);
         return proctor;
     }
 
+    /**
+     * user can override this function to provide a Proctor Result Reporter for monitoring
+     * determining Groups of proctor tests
+     *
+     * @return a ProctorResultReporter
+     */
+    protected ProctorResultReporter getProctorResultReporter() {
+        return null;
+    }
+
     @VisibleForTesting
     protected void logDynamicTests(final String testName, final Exception exception) {
         if (!loggedDynamicTests.contains(testName)) {

proctor-common/src/main/java/com/indeed/proctor/common/Proctor.java

@@ -19,6 +19,7 @@
 import org.apache.logging.log4j.Logger;
 
 import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import javax.el.FunctionMapper;
 import javax.el.ValueExpression;
 import java.io.IOException;
@@ -27,6 +28,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -80,6 +82,15 @@ public static Proctor construct(
             @Nonnull final ProctorLoadResult loadResult,
             @Nonnull final FunctionMapper functionMapper,
             @Nonnull final IdentifierValidator identifierValidator) {
+        return construct(matrix, loadResult, functionMapper, identifierValidator, null);
+    }
+
+    public static Proctor construct(
+            @Nonnull final TestMatrixArtifact matrix,
+            @Nonnull final ProctorLoadResult loadResult,
+            @Nonnull final FunctionMapper functionMapper,
+            @Nonnull final IdentifierValidator identifierValidator,
+            @Nullable final ProctorResultReporter resultReporter) {
         final Map<String, TestChooser<?>> testChoosers = Maps.newLinkedHashMap();
         final Map<String, String> versions = Maps.newLinkedHashMap();
 
@@ -111,7 +122,12 @@ public static Proctor construct(
                 TestDependencies.determineEvaluationOrder(matrix.getTests());
 
         return new Proctor(
-                matrix, loadResult, testChoosers, testEvaluationOrder, identifierValidator);
+                matrix,
+                loadResult,
+                testChoosers,
+                testEvaluationOrder,
+                identifierValidator,
+                resultReporter);
     }
 
     @Nonnull
@@ -136,7 +152,8 @@ static Proctor createEmptyProctor() {
                 loadResult,
                 choosers,
                 testEvaluationOrder,
-                new IdentifierValidator.Noop());
+                new IdentifierValidator.Noop(),
+                null);
     }
 
     static final long INT_RANGE = (long) Integer.MAX_VALUE - (long) Integer.MIN_VALUE;
@@ -150,14 +167,16 @@ static Proctor createEmptyProctor() {
 
     private final List<String> testEvaluationOrder;
     private final Map<String, Integer> evaluationOrderMap;
+    @Nullable private final ProctorResultReporter resultReporter;
 
     @VisibleForTesting
     Proctor(
             @Nonnull final TestMatrixArtifact matrix,
             @Nonnull final ProctorLoadResult loadResult,
             @Nonnull final Map<String, TestChooser<?>> testChoosers,
             @Nonnull final List<String> testEvaluationOrder,
-            @Nonnull final IdentifierValidator identifierValidator) {
+            @Nonnull final IdentifierValidator identifierValidator,
+            @Nullable final ProctorResultReporter resultReporter) {
         this.matrix = matrix;
         this.loadResult = loadResult;
         this.testChoosers = testChoosers;
@@ -174,6 +193,7 @@ static Proctor createEmptyProctor() {
         VarExporter.forNamespace(Proctor.class.getSimpleName()).includeInGlobal().export(this, "");
         VarExporter.forNamespace(DetailedExport.class.getSimpleName())
                 .export(new DetailedExport(), ""); //  intentionally not in global
+        this.resultReporter = resultReporter;
     }
 
     private static class DetailedExport {
@@ -330,7 +350,7 @@ public ProctorResult determineTestGroups(
         final Map<String, ValueExpression> localContext =
                 ProctorUtils.convertToValueExpressionMap(
                         RuleEvaluator.EXPRESSION_FACTORY, inputContext);
-
+        final Map<TestType, Integer> invalidIdentifierCount = new HashMap<>();
         for (final String testName : filteredEvaluationOrder) {
             final TestChooser<?> testChooser = testChoosers.get(testName);
             final String identifier;
@@ -342,6 +362,8 @@ public ProctorResult determineTestGroups(
             if (testChooser instanceof StandardTestChooser) {
                 final TestType testType = testChooser.getTestDefinition().getTestType();
                 if (testTypesWithInvalidIdentifier.contains(testType)) {
+                    invalidIdentifierCount.put(
+                            testType, invalidIdentifierCount.getOrDefault(testType, 0) + 1);
                     // skipping here to make it use the fallback bucket.
                     continue;
                 }
@@ -388,13 +410,19 @@ public ProctorResult determineTestGroups(
 
         // TODO Can we make getAudit nonnull?
         final Audit audit = Preconditions.checkNotNull(matrix.getAudit(), "Missing audit");
-        return new ProctorResult(
+        final ProctorResult result = new ProctorResult(
                 audit.getVersion(),
                 testGroups,
                 testAllocations,
                 testDefinitions,
                 identifiers,
                 inputContext);
+
+        if (resultReporter != null) {
+            resultReporter.reportMetrics(result, invalidIdentifierCount);
+        }
+
+        return result;
     }
 
     TestMatrixArtifact getArtifact() {

proctor-common/src/main/java/com/indeed/proctor/common/ProctorResult.java

@@ -9,10 +9,10 @@
 import javax.annotation.Nullable;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.SortedMap;
 import java.util.TreeMap;
-import java.util.HashSet;
 
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.emptySortedMap;

proctor-common/src/main/java/com/indeed/proctor/common/ProctorResultReporter.java

@@ -0,0 +1,23 @@
+package com.indeed.proctor.common;
+
+import com.indeed.proctor.common.model.TestType;
+
+import java.util.Map;
+
+public interface ProctorResultReporter {
+    default void reportMetrics(
+            final ProctorResult result,
+            final Map<TestType, Integer> testTypesWithInvalidIdentifier) {
+        reportTotalEvaluatedTests(result);
+        reportFallbackTests(result);
+        reportInvalidIdentifierTests(result, testTypesWithInvalidIdentifier);
+    }
+
+    void reportTotalEvaluatedTests(final ProctorResult result);
+
+    void reportFallbackTests(final ProctorResult result);
+
+    void reportInvalidIdentifierTests(
+            final ProctorResult result,
+            final Map<TestType, Integer> testTypesWithInvalidIdentifier);
+}

proctor-common/src/test/java/com/indeed/proctor/common/TestProctor.java

@@ -285,7 +285,8 @@ public void testDetermineTestGroupsForRandomTestWithRandomEnabled() {
                         null,
                         Collections.singletonMap(testName, testChooser),
                         Collections.singletonList(testName),
-                        new IdentifierValidator.Noop());
+                        new IdentifierValidator.Noop(),
+                        null);
 
         final Identifiers identifiersWithRandom = new Identifiers(Collections.emptyMap(), true);
         final Identifiers identifiersWithoutRandom = new Identifiers(Collections.emptyMap(), false);