-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(flag-decisions): Add support for sending flag decisions along with decision metadata. #405
Changes from all commits
b558918
ca32f1f
9cee565
fdc1afb
95409f0
029cfd5
42d7165
799e65d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -216,31 +216,65 @@ private Variation activate(@Nullable ProjectConfig projectConfig, | |
return null; | ||
} | ||
|
||
sendImpression(projectConfig, experiment, userId, copiedAttributes, variation); | ||
sendImpression(projectConfig, experiment, userId, copiedAttributes, variation, "experiment"); | ||
|
||
return variation; | ||
} | ||
|
||
/** | ||
* Creates and sends impression event. | ||
* | ||
* @param projectConfig the current projectConfig | ||
* @param experiment the experiment user bucketed into and dispatch an impression event | ||
* @param userId the ID of the user | ||
* @param filteredAttributes the attributes of the user | ||
* @param variation the variation that was returned from activate. | ||
* @param ruleType It can either be experiment in case impression event is sent from activate or it's feature-test or rollout | ||
*/ | ||
private void sendImpression(@Nonnull ProjectConfig projectConfig, | ||
@Nonnull Experiment experiment, | ||
@Nonnull String userId, | ||
@Nonnull Map<String, ?> filteredAttributes, | ||
@Nonnull Variation variation) { | ||
if (!experiment.isRunning()) { | ||
logger.info("Experiment has \"Launched\" status so not dispatching event during activation."); | ||
return; | ||
} | ||
@Nonnull Variation variation, | ||
@Nonnull String ruleType) { | ||
sendImpression(projectConfig, experiment, userId, filteredAttributes, variation, "", ruleType); | ||
} | ||
|
||
/** | ||
* Creates and sends impression event. | ||
* | ||
* @param projectConfig the current projectConfig | ||
* @param experiment the experiment user bucketed into and dispatch an impression event | ||
* @param userId the ID of the user | ||
* @param filteredAttributes the attributes of the user | ||
* @param variation the variation that was returned from activate. | ||
* @param flagKey It can either be empty if ruleType is experiment or it's feature key in case ruleType is feature-test or rollout | ||
* @param ruleType It can either be experiment in case impression event is sent from activate or it's feature-test or rollout | ||
*/ | ||
private void sendImpression(@Nonnull ProjectConfig projectConfig, | ||
@Nonnull Experiment experiment, | ||
@Nonnull String userId, | ||
@Nonnull Map<String, ?> filteredAttributes, | ||
@Nonnull Variation variation, | ||
@Nonnull String flagKey, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it should be featureFlagKey or featureKey There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we are calling it flagKey and flagType in all other sdks. So should I change it in java-sdk? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like flagKey which we use in Decide-API |
||
@Nonnull String ruleType) { | ||
|
||
UserEvent userEvent = UserEventFactory.createImpressionEvent( | ||
projectConfig, | ||
experiment, | ||
variation, | ||
userId, | ||
filteredAttributes); | ||
filteredAttributes, | ||
flagKey, | ||
ruleType); | ||
|
||
if (userEvent == null) { | ||
return; | ||
} | ||
eventProcessor.process(userEvent); | ||
logger.info("Activating user \"{}\" in experiment \"{}\".", userId, experiment.getKey()); | ||
|
||
if (experiment != null) { | ||
mnoman09 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
logger.info("Activating user \"{}\" in experiment \"{}\".", userId, experiment.getKey()); | ||
} | ||
// Kept For backwards compatibility. | ||
// This notification is deprecated and the new DecisionNotifications | ||
// are sent via their respective method calls. | ||
|
@@ -386,16 +420,22 @@ private Boolean isFeatureEnabled(@Nonnull ProjectConfig projectConfig, | |
FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, copiedAttributes, projectConfig); | ||
Boolean featureEnabled = false; | ||
SourceInfo sourceInfo = new RolloutSourceInfo(); | ||
if (featureDecision.decisionSource != null) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in which case, decisionSource can be null. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when experiment and variation is null.
|
||
decisionSource = featureDecision.decisionSource; | ||
} | ||
sendImpression( | ||
projectConfig, | ||
featureDecision.experiment, | ||
userId, | ||
copiedAttributes, | ||
featureDecision.variation, | ||
featureKey, | ||
decisionSource.toString()); | ||
|
||
if (featureDecision.variation != null) { | ||
// This information is only necessary for feature tests. | ||
// For rollouts experiments and variations are an implementation detail only. | ||
if (featureDecision.decisionSource.equals(FeatureDecision.DecisionSource.FEATURE_TEST)) { | ||
sendImpression( | ||
projectConfig, | ||
featureDecision.experiment, | ||
userId, | ||
copiedAttributes, | ||
featureDecision.variation); | ||
decisionSource = featureDecision.decisionSource; | ||
sourceInfo = new FeatureTestSourceInfo(featureDecision.experiment.getKey(), featureDecision.variation.getKey()); | ||
} else { | ||
logger.info("The user \"{}\" is not included in an experiment for feature \"{}\".", | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will appreciate if you can add header doc because of no. of args.