From 5f45785ece3b0f7a67a08f02a4872ba4de7847df Mon Sep 17 00:00:00 2001 From: James Kebinger Date: Fri, 28 Jul 2023 11:13:11 -0500 Subject: [PATCH] Capture config row, conditional value, and weighted value indexes, send with eval summaries Will enrich feedback about eval histories on web app --- .../cloud/prefab/client/config/Match.java | 21 ++++++ .../client/internal/ConfigResolver.java | 67 ++++++++++++------- .../client/internal/MatchStatsAggregator.java | 41 +++++++++++- .../client/internal/MatchProcessorTest.java | 6 ++ .../internal/MatchStatsAggregatorTest.java | 44 ++++++++++-- 5 files changed, 149 insertions(+), 30 deletions(-) diff --git a/client/src/main/java/cloud/prefab/client/config/Match.java b/client/src/main/java/cloud/prefab/client/config/Match.java index e8fe0479..1f1177c9 100644 --- a/client/src/main/java/cloud/prefab/client/config/Match.java +++ b/client/src/main/java/cloud/prefab/client/config/Match.java @@ -10,20 +10,38 @@ public class Match { private final Prefab.ConfigValue configValue; private final ConfigElement configElement; private final List evaluatedCriterion; + private final int rowIndex; + private final int conditionalValueIndex; private final Optional weightedValueIndex; public Match( Prefab.ConfigValue configValue, ConfigElement configElement, List evaluatedCriterion, + int rowIndex, + int conditionalValueIndex, Optional weightedValueIndex ) { this.configValue = configValue; this.configElement = configElement; this.evaluatedCriterion = evaluatedCriterion; + this.rowIndex = rowIndex; + this.conditionalValueIndex = conditionalValueIndex; this.weightedValueIndex = weightedValueIndex; } + public int getRowIndex() { + return rowIndex; + } + + public int getConditionalValueIndex() { + return conditionalValueIndex; + } + + public Optional getWeightedValueIndex() { + return weightedValueIndex; + } + public Prefab.ConfigValue getConfigValue() { return configValue; } @@ -53,6 +71,9 @@ public String toString() { .add("configElement", configElement) .add("configValue", configValue) .add("evaluatedCriterion", evaluatedCriterion) + .add("rowIndex", rowIndex) + .add("conditionalValueIndex", conditionalValueIndex) + .add("weightedValueIndex", weightedValueIndex) .toString(); } } diff --git a/client/src/main/java/cloud/prefab/client/internal/ConfigResolver.java b/client/src/main/java/cloud/prefab/client/internal/ConfigResolver.java index 4cfefb0f..e9d3cdce 100644 --- a/client/src/main/java/cloud/prefab/client/internal/ConfigResolver.java +++ b/client/src/main/java/cloud/prefab/client/internal/ConfigResolver.java @@ -8,6 +8,7 @@ import cloud.prefab.client.config.logging.AbstractLoggingListener; import cloud.prefab.domain.Prefab; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Streams; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -114,30 +115,36 @@ private Optional evalConfigElementMatch( // Prefer rows that have a projEnvId to ones that don't // There will be 0-1 rows with projenv and 0-1 rows without (the default row) - return configElement - .getRowsProjEnvFirst(projectEnvId) - .map(configRow -> { - if (!configRow.getPropertiesMap().isEmpty()) { - rowPropertiesStack.push(configRow.getPropertiesMap()); - } - // Return the value of the first matching set of criteria - for (Prefab.ConditionalValue conditionalValue : configRow.getValuesList()) { - Optional optionalMatch = evaluateConditionalValue( - conditionalValue, - lookupContext, - rowPropertiesStack, - configElement - ); + return Streams + .mapWithIndex( + configElement.getRowsProjEnvFirst(projectEnvId), + (configRow, rowIndex) -> { + if (!configRow.getPropertiesMap().isEmpty()) { + rowPropertiesStack.push(configRow.getPropertiesMap()); + } + // Return the value of the first matching set of criteria + int conditionalValueIndex = 0; + for (Prefab.ConditionalValue conditionalValue : configRow.getValuesList()) { + Optional optionalMatch = evaluateConditionalValue( + rowIndex, + conditionalValue, + conditionalValueIndex, + lookupContext, + rowPropertiesStack, + configElement + ); - if (optionalMatch.isPresent()) { - return optionalMatch.get(); + if (optionalMatch.isPresent()) { + return optionalMatch.get(); + } + conditionalValueIndex++; } + if (!configRow.getPropertiesMap().isEmpty()) { + rowPropertiesStack.pop(); + } + return null; } - if (!configRow.getPropertiesMap().isEmpty()) { - rowPropertiesStack.pop(); - } - return null; - }) + ) .filter(Objects::nonNull) .findFirst(); } @@ -165,13 +172,14 @@ Optional evalConfigElementMatch( * @return */ private Optional evaluateConditionalValue( + long rowIndex, Prefab.ConditionalValue conditionalValue, + int conditionalValueIndex, LookupContext lookupContext, Deque> rowProperties, ConfigElement configElement ) { List evaluatedCriteria = new ArrayList<>(); - for (Prefab.Criterion criterion : conditionalValue.getCriteriaList()) { for (EvaluatedCriterion evaluateCriterion : evaluateCriterionMatch( criterion, @@ -185,7 +193,14 @@ private Optional evaluateConditionalValue( } } return Optional.of( - simplifyToMatch(conditionalValue, configElement, lookupContext, evaluatedCriteria) + simplifyToMatch( + rowIndex, + conditionalValue, + conditionalValueIndex, + configElement, + lookupContext, + evaluatedCriteria + ) ); } @@ -193,7 +208,9 @@ private Optional evaluateConditionalValue( * A ConfigValue may be a WeightedValue. If so break it down so we can return a simpler form. */ private Match simplifyToMatch( + long rowIndex, Prefab.ConditionalValue selectedConditionalValue, + int conditionalValueIndex, ConfigElement configElement, LookupContext lookupContext, List evaluatedCriteria @@ -208,6 +225,8 @@ private Match simplifyToMatch( result.getValue(), configElement, evaluatedCriteria, + (int) rowIndex, + conditionalValueIndex, Optional.of(result.getIndex()) ); } else { @@ -215,6 +234,8 @@ private Match simplifyToMatch( selectedConditionalValue.getValue(), configElement, evaluatedCriteria, + (int) rowIndex, + conditionalValueIndex, Optional.empty() ); } diff --git a/client/src/main/java/cloud/prefab/client/internal/MatchStatsAggregator.java b/client/src/main/java/cloud/prefab/client/internal/MatchStatsAggregator.java index 4d071c07..f3a1ee7c 100644 --- a/client/src/main/java/cloud/prefab/client/internal/MatchStatsAggregator.java +++ b/client/src/main/java/cloud/prefab/client/internal/MatchStatsAggregator.java @@ -7,6 +7,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; public class MatchStatsAggregator { @@ -68,11 +69,14 @@ void recordMatch(Match match, long timeStamp) { CountKey countKey = new CountKey( match.getConfigElement().getConfig().getId(), + match.getConfigValue(), indexOfMatch( match.getConfigValue(), match.getConfigElement().getConfig().getAllowableValuesList() ), - match.getConfigValue() + match.getRowIndex(), + match.getConditionalValueIndex(), + match.getWeightedValueIndex() ); innerMap.computeIfAbsent(countKey, c -> new Counter(0)).inc(); } @@ -112,6 +116,11 @@ Prefab.ConfigEvaluationSummaries toProto() { if (countKey.selectedIndex >= 0) { counterProtoBuilder.setSelectedIndex(countKey.selectedIndex); } + counterProtoBuilder.setConfigRowIndex(countKey.rowIndex); + counterProtoBuilder.setConditionalValueIndex(countKey.valueIndex); + countKey.weightedValueIndex.ifPresent( + counterProtoBuilder::setWeightedValueIndex + ); counterProtoBuilder.setSelectedValue(countKey.configValue); summaryBuilder.addCounters(counterProtoBuilder); } @@ -162,11 +171,24 @@ static class CountKey { final long configId; final int selectedIndex; + private final int rowIndex; + private final int valueIndex; + private final Optional weightedValueIndex; final Prefab.ConfigValue configValue; - CountKey(long configId, int selectedIndex, Prefab.ConfigValue configValue) { + CountKey( + long configId, + Prefab.ConfigValue configValue, + int selectedIndex, + int rowIndex, + int valueIndex, + Optional weightedValueIndex + ) { this.configId = configId; this.selectedIndex = selectedIndex; + this.rowIndex = rowIndex; + this.valueIndex = valueIndex; + this.weightedValueIndex = weightedValueIndex; this.configValue = configValue; } @@ -182,13 +204,23 @@ public boolean equals(Object o) { return ( configId == countKey.configId && selectedIndex == countKey.selectedIndex && + rowIndex == countKey.rowIndex && + valueIndex == countKey.valueIndex && + Objects.equals(weightedValueIndex, countKey.weightedValueIndex) && Objects.equals(configValue, countKey.configValue) ); } @Override public int hashCode() { - return Objects.hash(configId, selectedIndex, configValue); + return Objects.hash( + configId, + selectedIndex, + rowIndex, + valueIndex, + weightedValueIndex, + configValue + ); } @Override @@ -197,6 +229,9 @@ public String toString() { .toStringHelper(this) .add("configId", configId) .add("selectedIndex", selectedIndex) + .add("rowIndex", rowIndex) + .add("valueIndex", valueIndex) + .add("weightedValueIndex", weightedValueIndex) .add("configValue", configValue) .toString(); } diff --git a/client/src/test/java/cloud/prefab/client/internal/MatchProcessorTest.java b/client/src/test/java/cloud/prefab/client/internal/MatchProcessorTest.java index a5d996bb..ce0ae298 100644 --- a/client/src/test/java/cloud/prefab/client/internal/MatchProcessorTest.java +++ b/client/src/test/java/cloud/prefab/client/internal/MatchProcessorTest.java @@ -58,6 +58,8 @@ void itWorks() throws InterruptedException { ConfigValueUtils.from(false), new ConfigElement(TF_CONFIG, new Provenance(ConfigClient.Source.STREAMING)), Collections.emptyList(), + 1, + 2, Optional.empty() ), new LookupContext( @@ -74,6 +76,8 @@ void itWorks() throws InterruptedException { ConfigValueUtils.from(true), new ConfigElement(TF_CONFIG, new Provenance(ConfigClient.Source.STREAMING)), Collections.emptyList(), + 1, + 2, Optional.empty() ), new LookupContext( @@ -90,6 +94,8 @@ void itWorks() throws InterruptedException { ConfigValueUtils.from(true), new ConfigElement(TF_CONFIG, new Provenance(ConfigClient.Source.STREAMING)), Collections.emptyList(), + 1, + 2, Optional.empty() ), new LookupContext( diff --git a/client/src/test/java/cloud/prefab/client/internal/MatchStatsAggregatorTest.java b/client/src/test/java/cloud/prefab/client/internal/MatchStatsAggregatorTest.java index 840a7696..52646813 100644 --- a/client/src/test/java/cloud/prefab/client/internal/MatchStatsAggregatorTest.java +++ b/client/src/test/java/cloud/prefab/client/internal/MatchStatsAggregatorTest.java @@ -67,6 +67,8 @@ void itAggregates() { ConfigValueUtils.from(true), new ConfigElement(TF_CONFIG_1, new Provenance(ConfigClient.Source.STREAMING)), Collections.emptyList(), + 0, + 2, Optional.empty() ), 101 @@ -77,6 +79,8 @@ void itAggregates() { ConfigValueUtils.from(false), new ConfigElement(TF_CONFIG_1, new Provenance(ConfigClient.Source.STREAMING)), Collections.emptyList(), + 0, + 2, Optional.empty() ), 102 @@ -87,6 +91,8 @@ void itAggregates() { ConfigValueUtils.from(false), new ConfigElement(TF_CONFIG_2, new Provenance(ConfigClient.Source.STREAMING)), Collections.emptyList(), + 0, + 2, Optional.empty() ), 102 @@ -100,6 +106,8 @@ void itAggregates() { new Provenance(ConfigClient.Source.STREAMING) ), Collections.emptyList(), + 0, + 3, Optional.empty() ), 107 @@ -117,7 +125,14 @@ void itAggregates() { Prefab.ConfigType.FEATURE_FLAG ), Map.of( - new MatchStatsAggregator.CountKey(1, 0, ConfigValueUtils.from(1)), + new MatchStatsAggregator.CountKey( + 1, + ConfigValueUtils.from(1), + 0, + 0, + 3, + Optional.empty() + ), new MatchStatsAggregator.Counter(1) ), new MatchStatsAggregator.ConfigKeyAndTypeKey( @@ -125,11 +140,32 @@ void itAggregates() { Prefab.ConfigType.FEATURE_FLAG ), Map.of( - new MatchStatsAggregator.CountKey(1, 0, ConfigValueUtils.from(true)), + new MatchStatsAggregator.CountKey( + 1, + ConfigValueUtils.from(true), + 0, + 0, + 2, + Optional.empty() + ), new MatchStatsAggregator.Counter(1), - new MatchStatsAggregator.CountKey(1, 1, ConfigValueUtils.from(false)), + new MatchStatsAggregator.CountKey( + 1, + ConfigValueUtils.from(false), + 1, + 0, + 2, + Optional.empty() + ), new MatchStatsAggregator.Counter(1), - new MatchStatsAggregator.CountKey(2, 1, ConfigValueUtils.from(false)), + new MatchStatsAggregator.CountKey( + 2, + ConfigValueUtils.from(false), + 1, + 0, + 2, + Optional.empty() + ), new MatchStatsAggregator.Counter(1) ) )