-
Notifications
You must be signed in to change notification settings - Fork 28
fix(logs): Fixing log messages #259
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
Conversation
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.
Some more changes for variable evaluation logs.
@@ -810,7 +810,7 @@ def get_feature_variable_for_variation(feature_flag_key, feature_enabled, variat | |||
end | |||
else | |||
@logger.log(Logger::DEBUG, | |||
"Feature '#{feature_flag_key}' for variation '#{variation['key']}' is not enabled. Returning the default variable value '#{variable_value}'.") | |||
"Feature '#{feature_flag_key}' is not enabled for user '#{user_id}'. Returning the default variable value '#{variable_value}'.") |
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.
The changes are incomplete here.
On line 809 it should say Variable value is not defined. Returning the default variable value <variable value> for variable <variable key>
On line 916 it should say User <user ID> was not bucketed into experiment or rollout for feature flag <feature flag key>. Returning the default variable value <variable value>
lib/optimizely/audience.rb
Outdated
def user_in_experiment?(config, experiment, attributes, logger) | ||
# Determine for given experiment if user satisfies the audiences for the experiment. | ||
def user_meets_audience_conditions?(config, experiment, attributes, logger, logging_hash = nil, logging_key = nil) | ||
# Determine for given experiment/rollout rule if user satisfies the audiences. |
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.
nit. satisfies the audience conditions
.
lib/optimizely/audience.rb
Outdated
# | ||
# config - Representation of the Optimizely project config. | ||
# experiment - Experiment for which visitor is to be bucketed. | ||
# experiment - Experiment/Rollout rule for which visitor is to be bucketed. |
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.
nit. ...in which user is to be bucketed
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.
Looks good.
Summary
Test plan