Skip to content

Commit

Permalink
WIP
Browse files Browse the repository at this point in the history
  • Loading branch information
gthea committed Dec 10, 2024
1 parent 4c3b592 commit 345dc03
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 25 deletions.
16 changes: 11 additions & 5 deletions src/main/java/io/split/android/client/EvaluationResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,22 @@ public final class EvaluationResult {
private final String mLabel;
private final Long mChangeNumber;
private final String mConfigurations;
private final boolean mTrackImpression;

public EvaluationResult(String treatment, String label) {
this(treatment, label, null);
public EvaluationResult(String treatment, String label, boolean trackImpression) {
this(treatment, label, null, null, trackImpression);
}

EvaluationResult(String treatment, String label, Long changeNumber) {
this(treatment, label, changeNumber, null);
EvaluationResult(String treatment, String label, Long changeNumber, boolean trackImpression) {
this(treatment, label, changeNumber, null, trackImpression);
}

public EvaluationResult(String treatment, String label, Long changeNumber, String configurations) {
public EvaluationResult(String treatment, String label, Long changeNumber, String configurations, boolean trackImpression) {
mTreatment = treatment;
mLabel = label;
mChangeNumber = changeNumber;
mConfigurations = configurations;
mTrackImpression = trackImpression;
}

public String getTreatment() {
Expand All @@ -36,4 +38,8 @@ public Long getChangeNumber() {
public String getConfigurations() {
return mConfigurations;
}

public boolean getTrackImpression() {
return mTrackImpression;
}
}
14 changes: 7 additions & 7 deletions src/main/java/io/split/android/client/EvaluatorImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,16 @@ public EvaluationResult getTreatment(String matchingKey, String bucketingKey, St
try {
ParsedSplit parsedSplit = mSplitParser.parse(mSplitsStorage.get(splitName), matchingKey);
if (parsedSplit == null) {
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.DEFINITION_NOT_FOUND);
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.DEFINITION_NOT_FOUND, true);
}

return getTreatment(matchingKey, bucketingKey, parsedSplit, attributes);
} catch (ChangeNumberExceptionWrapper ex) {
Logger.e(ex, "Catch Change Number Exception");
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.EXCEPTION, ex.changeNumber());
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.EXCEPTION, ex.changeNumber(), true);
} catch (Exception e) {
Logger.e(e, "Catch All Exception");
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.EXCEPTION);
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.EXCEPTION, true);
}
}

Expand All @@ -52,7 +52,7 @@ public EvaluationResult getTreatment(String matchingKey, String bucketingKey, St
private EvaluationResult getTreatment(String matchingKey, String bucketingKey, ParsedSplit parsedSplit, Map<String, Object> attributes) throws ChangeNumberExceptionWrapper {
try {
if (parsedSplit.killed()) {
return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.KILLED, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()));
return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.KILLED, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()), parsedSplit.trackImpression());
}

/*
Expand All @@ -75,7 +75,7 @@ private EvaluationResult getTreatment(String matchingKey, String bucketingKey, P

if (bucket > parsedSplit.trafficAllocation()) {
// out of split
return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.NOT_IN_SPLIT, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()));
return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.NOT_IN_SPLIT, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()), parsedSplit.trackImpression());
}

}
Expand All @@ -84,11 +84,11 @@ private EvaluationResult getTreatment(String matchingKey, String bucketingKey, P

if (parsedCondition.matcher().match(matchingKey, bucketingKey, attributes, this)) {
String treatment = Splitter.getTreatment(bk, parsedSplit.seed(), parsedCondition.partitions(), parsedSplit.algo());
return new EvaluationResult(treatment, parsedCondition.label(), parsedSplit.changeNumber(), configForTreatment(parsedSplit, treatment));
return new EvaluationResult(treatment, parsedCondition.label(), parsedSplit.changeNumber(), configForTreatment(parsedSplit, treatment), parsedSplit.trackImpression());
}
}

return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.DEFAULT_RULE, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()));
return new EvaluationResult(parsedSplit.defaultTreatment(), TreatmentLabels.DEFAULT_RULE, parsedSplit.changeNumber(), configForTreatment(parsedSplit, parsedSplit.defaultTreatment()), parsedSplit.trackImpression());
} catch (Exception e) {
throw new ChangeNumberExceptionWrapper(e, parsedSplit.changeNumber());
}
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/io/split/android/client/dtos/Split.java
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,7 @@ public class Split {
@Nullable
@SerializedName("sets")
public Set<String> sets;

@SerializedName("trackImpression")
public boolean trackImpression = true;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package io.split.android.client.impressions;

import java.util.Map;

public class DecoratedImpression extends Impression {

private final boolean mTrackImpression;

public DecoratedImpression(String key, String bucketingKey, String split, String treatment, long time, String appliedRule, Long changeNumber, Map<String, Object> atributes, boolean trackImpression) {
super(key, bucketingKey, split, treatment, time, appliedRule, changeNumber, atributes);
mTrackImpression = trackImpression;
}

public boolean getTrackImpression() {
return mTrackImpression;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;

import io.split.android.client.impressions.DecoratedImpression;
import io.split.android.client.impressions.Impression;
import io.split.android.client.service.impressions.strategy.PeriodicTracker;
import io.split.android.client.service.impressions.strategy.ProcessStrategy;
Expand Down Expand Up @@ -39,7 +40,7 @@ public void pushImpression(Impression impression) {
return;
}

if (track(impression)) {
if (shouldTrack(impression)) {
mProcessStrategy.apply(impression);
} else {
mNoneStrategy.apply(impression);
Expand Down Expand Up @@ -72,7 +73,12 @@ public void stopPeriodicRecording() {
}
}

private static boolean track(Impression impression) {
return true; // TODO: Placeholder method
private static boolean shouldTrack(Impression impression) {
if (impression instanceof DecoratedImpression) {
return ((DecoratedImpression) impression).getTrackImpression();
} else {
// default behaviour; will never get here.
return true;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import io.split.android.client.attributes.AttributesMerger;
import io.split.android.client.events.ListenableEventsManager;
import io.split.android.client.events.SplitEvent;
import io.split.android.client.impressions.Impression;
import io.split.android.client.impressions.DecoratedImpression;
import io.split.android.client.impressions.ImpressionListener;
import io.split.android.client.storage.splits.SplitsStorage;
import io.split.android.client.telemetry.model.Method;
Expand Down Expand Up @@ -294,7 +294,8 @@ private TreatmentResult getTreatmentWithConfigWithoutMetrics(String split, Map<S
evaluationResult.getTreatment(),
mLabelsEnabled ? evaluationResult.getLabel() : null,
evaluationResult.getChangeNumber(),
mergedAttributes);
mergedAttributes,
evaluationResult.getTrackImpression());

return new TreatmentResult(splitResult, false);
} catch (Exception ex) {
Expand All @@ -307,16 +308,17 @@ private TreatmentResult getTreatmentWithConfigWithoutMetrics(String split, Map<S
Treatments.CONTROL,
TreatmentLabels.EXCEPTION,
(evaluationResult != null) ? evaluationResult.getChangeNumber() : null,
mergedAttributes);
mergedAttributes,
evaluationResult == null || evaluationResult.getTrackImpression());
}

return new TreatmentResult(new SplitResult(Treatments.CONTROL), true);
}
}

private void logImpression(String matchingKey, String bucketingKey, String splitName, String result, String label, Long changeNumber, Map<String, Object> attributes) {
private void logImpression(String matchingKey, String bucketingKey, String splitName, String result, String label, Long changeNumber, Map<String, Object> attributes, boolean trackImpression) {
try {
mImpressionListener.log(new Impression(matchingKey, bucketingKey, splitName, result, System.currentTimeMillis(), label, changeNumber, attributes));
mImpressionListener.log(new DecoratedImpression(matchingKey, bucketingKey, splitName, result, System.currentTimeMillis(), label, changeNumber, attributes, trackImpression));
} catch (Throwable t) {
Logger.e("An error occurred logging impression: " + t.getLocalizedMessage());
}
Expand All @@ -339,7 +341,7 @@ private EvaluationResult evaluateIfReady(String featureFlagName,
mValidationLogger.w("the SDK is not ready, results may be incorrect for feature flag " + featureFlagName + ". Make sure to wait for SDK readiness before using this method", validationTag);
mTelemetryStorageProducer.recordNonReadyUsage();

return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.NOT_READY, null, null);
return new EvaluationResult(Treatments.CONTROL, TreatmentLabels.NOT_READY, null, null, true);
}
return mEvaluator.getTreatment(mMatchingKey, mBucketingKey, featureFlagName, attributes);
}
Expand Down
15 changes: 12 additions & 3 deletions src/main/java/io/split/android/engine/experiments/ParsedSplit.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public class ParsedSplit {
private final int mAlgo;
private final Map<String, String> mConfigurations;
private final Set<String> mSets;
private final boolean mTrackImpression;

public ParsedSplit(
String feature,
Expand All @@ -36,7 +37,8 @@ public ParsedSplit(
int trafficAllocationSeed,
int algo,
Map<String, String> configurations,
Set<String> sets
Set<String> sets,
boolean trackImpression
) {
mSplit = feature;
mSeed = seed;
Expand All @@ -47,6 +49,7 @@ public ParsedSplit(
mChangeNumber = changeNumber;
mAlgo = algo;
mConfigurations = configurations;
mTrackImpression = trackImpression;

if (mDefaultTreatment == null) {
throw new IllegalArgumentException("DefaultTreatment is null");
Expand Down Expand Up @@ -104,6 +107,10 @@ public Set<String> sets() {
return mSets;
}

public boolean trackImpression() {
return mTrackImpression;
}

@Override
public int hashCode() {
int result = 17;
Expand All @@ -116,6 +123,7 @@ public int hashCode() {
result = 31 * result + (int) (mChangeNumber ^ (mChangeNumber >>> 32));
result = 31 * result + (mAlgo ^ (mAlgo >>> 32));
result = 31 * result + ((mSets != null) ? mSets.hashCode() : 0);
result = 31 * result + (mTrackImpression ? 1 : 0);
return result;
}

Expand All @@ -135,7 +143,8 @@ public boolean equals(Object obj) {
&& mChangeNumber == other.mChangeNumber
&& mAlgo == other.mAlgo
&& (Objects.equals(mConfigurations, other.mConfigurations))
&& (Objects.equals(mSets, other.mSets));
&& (Objects.equals(mSets, other.mSets)
&& mTrackImpression == other.mTrackImpression);

}

Expand All @@ -146,7 +155,7 @@ public String toString() {
", default treatment:" + mDefaultTreatment +
", parsedConditions:" + mParsedCondition +
", trafficTypeName:" + mTrafficTypeName + ", changeNumber:" + mChangeNumber +
", algo:" + mAlgo + ", config:" + mConfigurations + ", sets:" + mSets;
", algo:" + mAlgo + ", config:" + mConfigurations + ", sets:" + mSets + ", trackImpression:" + mTrackImpression;

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ private ParsedSplit parseWithoutExceptionHandling(Split split, String matchingKe
split.trafficAllocationSeed,
split.algo,
split.configurations,
split.sets);
split.sets,
split.trackImpression);
}

private CombiningMatcher toMatcher(MatcherGroup matcherGroup, String matchingKey) throws UnsupportedMatcherException {
Expand Down
16 changes: 16 additions & 0 deletions src/test/java/io/split/android/client/TreatmentManagerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyMap;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
Expand All @@ -27,6 +28,7 @@
import io.split.android.client.dtos.Split;
import io.split.android.client.events.ListenableEventsManager;
import io.split.android.client.events.SplitEvent;
import io.split.android.client.impressions.DecoratedImpression;
import io.split.android.client.impressions.ImpressionListener;
import io.split.android.client.storage.mysegments.MySegmentsStorage;
import io.split.android.client.storage.mysegments.MySegmentsStorageContainer;
Expand Down Expand Up @@ -316,6 +318,20 @@ public void evaluationWhenNotReadyLogsCorrectMessage() {
verify(validationMessageLogger).w(eq("the SDK is not ready, results may be incorrect for feature flag test_split. Make sure to wait for SDK readiness before using this method"), any());
}

@Test
public void trackValueFromEvaluationResultGetsPassedInToImpression() {
Evaluator evaluatorMock = mock(Evaluator.class);
when(evaluatorMock.getTreatment(eq("matching_key"), eq("bucketing_key"), eq("test_split"), anyMap()))
.thenReturn(new EvaluationResult("test", "test"));
TreatmentManagerImpl tManager = initializeTreatmentManager(evaluatorMock);

tManager.getTreatment("test_split", null, false);

verify(impressionListener).log(argThat(argument -> {
return ((DecoratedImpression) argument).getTrackImpression();
}));
}

private void assertControl(List<String> splitList, String treatment, Map<String, String> treatmentList, SplitResult splitResult, Map<String, SplitResult> splitResultList) {
Assert.assertNotNull(treatment);
Assert.assertEquals(Treatments.CONTROL, treatment);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.split.android.client.service.impressions

import io.split.android.client.impressions.DecoratedImpression
import io.split.android.client.impressions.Impression
import io.split.android.client.service.impressions.strategy.PeriodicTracker
import io.split.android.client.service.impressions.strategy.ProcessStrategy
Expand All @@ -8,6 +9,7 @@ import org.junit.Test
import org.mockito.Mock
import org.mockito.Mockito.mock
import org.mockito.Mockito.verify
import org.mockito.Mockito.`when`
import org.mockito.MockitoAnnotations

class StrategyImpressionManagerTest {
Expand Down Expand Up @@ -60,4 +62,17 @@ class StrategyImpressionManagerTest {

verify(strategy).apply(impression)
}

@Test
fun `pushImpression when it is decorated uses value from trackImpression to track`() {
val impression = mock(DecoratedImpression::class.java)
val impression2 = mock(DecoratedImpression::class.java)
`when`(impression.trackImpression).thenReturn(false)
`when`(impression2.trackImpression).thenReturn(true)
impressionManager.pushImpression(impression)
impressionManager.pushImpression(impression2)

verify(strategy).apply(impression2)
verify(noneStrategy).apply(impression)
}
}

0 comments on commit 345dc03

Please sign in to comment.