diff --git a/proctor-consumer/src/main/java/com/indeed/proctor/consumer/AbstractGroups.java b/proctor-consumer/src/main/java/com/indeed/proctor/consumer/AbstractGroups.java
index 68f3be266..28b73b359 100644
--- a/proctor-consumer/src/main/java/com/indeed/proctor/consumer/AbstractGroups.java
+++ b/proctor-consumer/src/main/java/com/indeed/proctor/consumer/AbstractGroups.java
@@ -355,6 +355,28 @@ public String toLoggingString() {
return sb.toString();
}
+ /**
+ * String to be used for verification/debugging purposes. For historic reasons inside Indeed,
+ * contains two output formats per testname. This method does not filter out 100% allocations.
+ *
+ * Additional custom groups can be added by overriding getCustomGroupsForLogging().
+ *
+ * @return a comma-separated List of {testname}{active-bucket-VALUE} and
+ * {AllocationId}{testname}{active-bucket-VALUE} for all LIVE tests
+ */
+ public String toVerifyGroupsString() {
+ if (isEmpty()) {
+ return "";
+ }
+ final StringBuilder sb = new StringBuilder(proctorResult.getBuckets().size() * 10);
+ appendTestGroups(sb, GROUPS_SEPARATOR, false);
+ // remove trailing comma
+ if (sb.length() > 0) {
+ sb.deleteCharAt(sb.length() - 1);
+ }
+ return sb.toString();
+ }
+
/**
* @return an empty string or a comma-separated, comma-finalized list of groups
* @deprecated use toLoggingString()
@@ -398,12 +420,61 @@ public void appendTestGroups(final StringBuilder sb, final char separator) {
appendTestGroupsWithAllocations(sb, separator, testNames);
}
+ /**
+ * For each testname returned by getLoggingTestNames(), appends each test group in the form of
+ * [allocation id + ":" + test name + bucket value]. For example, it appends "#A1:bgcolortst1"
+ * if test name is bgcolortst and allocation id is #A1 and separator is ",".
+ *
+ *
the separator should be appended after each test group added to the string builder {@link
+ * #toString()} {@link #buildTestGroupString()} or {@link #appendTestGroups(StringBuilder)}
+ *
+ * @param sb a string builder
+ * @param separator a char used as separator x appends an empty string or a x-separated,
+ * x-finalized list of groups
+ */
+ public void appendTestGroups(
+ final StringBuilder sb,
+ final char separator,
+ final boolean filterRolledOutAllocations) {
+ final List testNames =
+ filterRolledOutAllocations ? getLoggingTestNames() : getLoggingTestNamesFullList();
+ appendTestGroupsWithAllocations(sb, separator, testNames);
+ }
+
+ /**
+ * Return test names for tests that are non-silent, doesn't have available definition, is not
+ * 100% rolled out, and have a non-negative active bucket, in a stable sort. Stable sort is
+ * beneficial for log string compression, for debugging, and may help in cases of size-limited
+ * output.
+ */
+ protected final List getLoggingTestNames() {
+ final Map testDefinitions =
+ proctorResult.getTestDefinitions();
+ // following lines should preserve the order in the map to ensure logging values are stable
+ final Map buckets = proctorResult.getBuckets();
+ return buckets.keySet().stream()
+ .filter(
+ testName -> {
+ final ConsumableTestDefinition consumableTestDefinition =
+ testDefinitions.get(testName);
+ // fallback to non-silent when test definition is not available
+ return (consumableTestDefinition == null)
+ || !consumableTestDefinition.getSilent();
+ })
+ // Suppress 100% allocation logging
+ .filter(this::loggableAllocation)
+ // call to getValueWithouMarkingUsage() to allow overrides of getActiveBucket, but
+ // avoid marking
+ .filter(testName -> getValueWithoutMarkingUsage(testName, -1) >= 0)
+ .collect(Collectors.toList());
+ }
+
/**
* Return test names for tests that are non-silent or doesn't have available definition, and
* have a non-negative active bucket, in a stable sort. Stable sort is beneficial for log string
* compression, for debugging, and may help in cases of size-limited output.
*/
- protected final List getLoggingTestNames() {
+ protected final List getLoggingTestNamesFullList() {
final Map testDefinitions =
proctorResult.getTestDefinitions();
// following lines should preserve the order in the map to ensure logging values are stable
@@ -454,6 +525,25 @@ protected final void appendTestGroupsWithAllocations(
}
}
+ private boolean loggableAllocation(final String testName) {
+ final ConsumableTestDefinition td = proctorResult.getTestDefinitions().get(testName);
+ return loggableAllocation(testName, td, proctorResult);
+ }
+
+ public static boolean loggableAllocation(
+ final String testName,
+ final ConsumableTestDefinition td,
+ final ProctorResult proctorResult) {
+ if (td != null) {
+ final Allocation allocation = proctorResult.getAllocations().get(testName);
+ if (allocation != null
+ && allocation.getRanges().stream().anyMatch(range -> range.getLength() == 1)) {
+ return td.getForceLogging();
+ }
+ }
+ return true;
+ }
+
/**
* Generates a Map[testname, bucketValue] for bucketValues >= 0.
*
diff --git a/proctor-consumer/src/main/java/com/indeed/proctor/consumer/ProctorGroupsWriter.java b/proctor-consumer/src/main/java/com/indeed/proctor/consumer/ProctorGroupsWriter.java
index af97f5011..6380f74ba 100644
--- a/proctor-consumer/src/main/java/com/indeed/proctor/consumer/ProctorGroupsWriter.java
+++ b/proctor-consumer/src/main/java/com/indeed/proctor/consumer/ProctorGroupsWriter.java
@@ -14,6 +14,8 @@
import java.util.Optional;
import java.util.function.BiPredicate;
+import static com.indeed.proctor.consumer.AbstractGroups.loggableAllocation;
+
/**
* Helper class to build a Strings to log, helping with analysis of experiments.
*
@@ -203,7 +205,9 @@ public ProctorGroupsWriter build() {
if (additionalFilter != null) {
return additionalFilter.test(testName, proctorResult);
}
- return true;
+ // Suppress 100% allocation logging
+ return loggableAllocation(
+ testName, consumableTestDefinition, proctorResult);
});
}
}
diff --git a/proctor-consumer/src/main/java/com/indeed/proctor/consumer/spring/ShowGroupsHandler.java b/proctor-consumer/src/main/java/com/indeed/proctor/consumer/spring/ShowGroupsHandler.java
index 684db45e0..0525eae1f 100644
--- a/proctor-consumer/src/main/java/com/indeed/proctor/consumer/spring/ShowGroupsHandler.java
+++ b/proctor-consumer/src/main/java/com/indeed/proctor/consumer/spring/ShowGroupsHandler.java
@@ -29,7 +29,7 @@ public void handleRequest(HttpServletRequest request, HttpServletResponse respon
writer.println("Did not determine any groups");
} else {
final StringBuilder sb = new StringBuilder();
- grps.appendTestGroups(sb, '\n');
+ grps.appendTestGroups(sb, '\n', false);
writer.print(sb.toString());
}
}
diff --git a/proctor-consumer/src/test/java/com/indeed/proctor/consumer/ProctorGroupStubber.java b/proctor-consumer/src/test/java/com/indeed/proctor/consumer/ProctorGroupStubber.java
index 419fcbb16..94e0fdc2b 100644
--- a/proctor-consumer/src/test/java/com/indeed/proctor/consumer/ProctorGroupStubber.java
+++ b/proctor-consumer/src/test/java/com/indeed/proctor/consumer/ProctorGroupStubber.java
@@ -48,7 +48,19 @@ public ProctorResultStubBuilder withStubTest(
final StubTest stubTest,
@Nullable final TestBucket resolved,
final TestBucket... definedBuckets) {
- withStubTest(stubTest, resolved, stubDefinitionWithVersion("v1", definedBuckets));
+ withStubTest(stubTest, resolved, stubDefinitionWithVersion(true, "v1", definedBuckets));
+ return this;
+ }
+
+ public ProctorResultStubBuilder withStubTest(
+ final boolean forceLogging,
+ final StubTest stubTest,
+ @Nullable final TestBucket resolved,
+ final TestBucket... definedBuckets) {
+ withStubTest(
+ stubTest,
+ resolved,
+ stubDefinitionWithVersion(forceLogging, "v1", definedBuckets));
return this;
}
@@ -89,10 +101,11 @@ public ProctorResult build() {
}
public static ConsumableTestDefinition stubDefinitionWithVersion(
- final String version, final TestBucket... buckets) {
+ final boolean forceLogging, final String version, final TestBucket... buckets) {
final ConsumableTestDefinition testDefinition = new ConsumableTestDefinition();
testDefinition.setVersion(version);
testDefinition.setBuckets(Arrays.asList(buckets));
+ testDefinition.setForceLogging(forceLogging);
return testDefinition;
}
@@ -126,6 +139,7 @@ public enum StubTest implements com.indeed.proctor.consumer.Test {
HOLDOUT_MASTER_TEST("holdout_tst", -1),
CONTROL_SELECTED_TEST("bgtst", -1),
+ SUPPRESS_LOGGING_TST("suppress_logging_example_tst", -1),
GROUP1_SELECTED_TEST("abtst", -1),
GROUP_WITH_FALLBACK_TEST("groupwithfallbacktst", -1),
INACTIVE_SELECTED_TEST("btntst", -1),
diff --git a/proctor-consumer/src/test/java/com/indeed/proctor/consumer/TestAbstractGroups.java b/proctor-consumer/src/test/java/com/indeed/proctor/consumer/TestAbstractGroups.java
index cf1f56735..303b70e6f 100644
--- a/proctor-consumer/src/test/java/com/indeed/proctor/consumer/TestAbstractGroups.java
+++ b/proctor-consumer/src/test/java/com/indeed/proctor/consumer/TestAbstractGroups.java
@@ -4,16 +4,18 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.indeed.proctor.common.ProctorResult;
-import com.indeed.proctor.common.model.ConsumableTestDefinition;
-import com.indeed.proctor.common.model.Payload;
+import com.indeed.proctor.common.model.*;
import com.indeed.proctor.consumer.ProctorGroupStubber.FakeTest;
import com.indeed.proctor.consumer.logging.TestGroupFormatter;
import com.indeed.proctor.consumer.logging.TestMarkingObserver;
import org.junit.Before;
import org.junit.Test;
+import java.util.ArrayList;
import java.util.Arrays;
+import java.util.List;
+import static com.indeed.proctor.consumer.AbstractGroups.loggableAllocation;
import static com.indeed.proctor.consumer.ProctorGroupStubber.CONTROL_BUCKET_WITH_PAYLOAD;
import static com.indeed.proctor.consumer.ProctorGroupStubber.FALLBACK_BUCKET;
import static com.indeed.proctor.consumer.ProctorGroupStubber.FALLBACK_NOPAYLOAD_BUCKET;
@@ -27,6 +29,7 @@
import static com.indeed.proctor.consumer.ProctorGroupStubber.StubTest.INACTIVE_SELECTED_TEST;
import static com.indeed.proctor.consumer.ProctorGroupStubber.StubTest.MISSING_DEFINITION_TEST;
import static com.indeed.proctor.consumer.ProctorGroupStubber.StubTest.NO_BUCKETS_WITH_FALLBACK_TEST;
+import static com.indeed.proctor.consumer.ProctorGroupStubber.StubTest.SUPPRESS_LOGGING_TST;
import static java.util.Collections.emptyMap;
import static java.util.Collections.singleton;
import static org.assertj.core.api.Assertions.assertThat;
@@ -53,6 +56,13 @@ public void setUp() {
INACTIVE_BUCKET,
CONTROL_BUCKET_WITH_PAYLOAD,
GROUP_1_BUCKET_WITH_PAYLOAD)
+ .withStubTest(
+ false,
+ ProctorGroupStubber.StubTest.SUPPRESS_LOGGING_TST,
+ CONTROL_BUCKET_WITH_PAYLOAD,
+ INACTIVE_BUCKET,
+ CONTROL_BUCKET_WITH_PAYLOAD,
+ GROUP_1_BUCKET_WITH_PAYLOAD)
.withStubTest(
ProctorGroupStubber.StubTest.GROUP1_SELECTED_TEST,
GROUP_1_BUCKET_WITH_PAYLOAD,
@@ -164,7 +174,7 @@ public void testToLongString() {
assertThat(emptyGroup.toLongString()).isEmpty();
assertThat(sampleGroups.toLongString())
.isEqualTo(
- "abtst-group1,bgtst-control,btntst-inactive,groupwithfallbacktst-group1,no_definition_tst-group1");
+ "abtst-group1,bgtst-control,btntst-inactive,groupwithfallbacktst-group1,no_definition_tst-group1,suppress_logging_example_tst-control");
}
@Test
@@ -180,6 +190,106 @@ public void testToLoggingString() {
"#A1:abtst1,#A1:bgtst0,#A1:groupwithfallbacktst2,#A1:no_definition_tst2");
}
+ @Test
+ public void testToVerifyGroupsString() {
+ final ConsumableTestDefinition td =
+ ConsumableTestDefinition.fromTestDefinition(
+ TestDefinition.builder()
+ .setTestType(TestType.RANDOM)
+ .setSalt("foo")
+ .build());
+ final ConsumableTestDefinition tdWithForceLogging =
+ ConsumableTestDefinition.fromTestDefinition(
+ TestDefinition.builder()
+ .setTestType(TestType.RANDOM)
+ .setSalt("foo")
+ .setForceLogging(true)
+ .build());
+ final ProctorResult result =
+ new ProctorGroupStubber.ProctorResultStubBuilder()
+ .withStubTest(
+ false,
+ ProctorGroupStubber.StubTest.SUPPRESS_LOGGING_TST,
+ CONTROL_BUCKET_WITH_PAYLOAD,
+ INACTIVE_BUCKET,
+ CONTROL_BUCKET_WITH_PAYLOAD,
+ GROUP_1_BUCKET_WITH_PAYLOAD)
+ .build();
+
+ assertThat((new AbstractGroups(result) {}).toVerifyGroupsString())
+ .isEqualTo("#A1:suppress_logging_example_tst0");
+ }
+
+ @Test
+ public void testCheckRolledOutAllocation() {
+ final ConsumableTestDefinition td =
+ ConsumableTestDefinition.fromTestDefinition(
+ TestDefinition.builder()
+ .setTestType(TestType.RANDOM)
+ .setSalt("foo")
+ .build());
+ final ConsumableTestDefinition tdWithForceLogging =
+ ConsumableTestDefinition.fromTestDefinition(
+ TestDefinition.builder()
+ .setTestType(TestType.RANDOM)
+ .setSalt("foo")
+ .setForceLogging(true)
+ .build());
+ final ProctorResult result =
+ new ProctorGroupStubber.ProctorResultStubBuilder()
+ .withStubTest(
+ false,
+ ProctorGroupStubber.StubTest.SUPPRESS_LOGGING_TST,
+ CONTROL_BUCKET_WITH_PAYLOAD,
+ INACTIVE_BUCKET,
+ CONTROL_BUCKET_WITH_PAYLOAD,
+ GROUP_1_BUCKET_WITH_PAYLOAD)
+ .build();
+
+ assertThat(loggableAllocation("suppress_logging_example_tst", tdWithForceLogging, result))
+ .isTrue();
+
+ assertThat(loggableAllocation("suppress_logging_example_tst", td, result)).isFalse();
+ }
+
+ @Test
+ public void testCheckNotRolledOutAllocationAlwaysLogs() {
+ final ConsumableTestDefinition td =
+ ConsumableTestDefinition.fromTestDefinition(
+ TestDefinition.builder()
+ .setTestType(TestType.RANDOM)
+ .setSalt("foo")
+ .build());
+ final ConsumableTestDefinition tdWithForceLogging =
+ ConsumableTestDefinition.fromTestDefinition(
+ TestDefinition.builder()
+ .setTestType(TestType.RANDOM)
+ .setSalt("foo")
+ .setForceLogging(true)
+ .build());
+ final ProctorResult result =
+ new ProctorGroupStubber.ProctorResultStubBuilder()
+ .withStubTest(
+ false,
+ SUPPRESS_LOGGING_TST,
+ CONTROL_BUCKET_WITH_PAYLOAD,
+ INACTIVE_BUCKET,
+ CONTROL_BUCKET_WITH_PAYLOAD,
+ GROUP_1_BUCKET_WITH_PAYLOAD)
+ .build();
+
+ final List ranges = new ArrayList<>();
+ ranges.add(new Range(0, 0.5));
+ ranges.add(new Range(1, 0.5));
+
+ result.getAllocations().get("suppress_logging_example_tst").setRanges(ranges);
+
+ assertThat(loggableAllocation("suppress_logging_example_tst", tdWithForceLogging, result))
+ .isTrue();
+
+ assertThat(loggableAllocation("suppress_logging_example_tst", td, result)).isTrue();
+ }
+
@Test
public void testToLoggingStringWithExposureAndObserver() {
@@ -268,11 +378,12 @@ public void testGetJavaScriptConfig() {
assertThat(emptyGroup.getJavaScriptConfig()).hasSize(0);
assertThat(sampleGroups.getJavaScriptConfig())
- .hasSize(4)
+ .hasSize(5)
.containsEntry(GROUP1_SELECTED_TEST.getName(), 1)
.containsEntry(CONTROL_SELECTED_TEST.getName(), 0)
.containsEntry(GROUP_WITH_FALLBACK_TEST.getName(), 2)
- .containsEntry(MISSING_DEFINITION_TEST.getName(), 2);
+ .containsEntry(MISSING_DEFINITION_TEST.getName(), 2)
+ .containsEntry(SUPPRESS_LOGGING_TST.getName(), 0);
}
@Test