Skip to content

refact(audience-logs): Added and refactored audience evaluation logs. #336

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

Merged
merged 16 commits into from
Jul 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 40 additions & 27 deletions Sources/Data Model/Audience/AttributeValue.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/****************************************************************************
* Copyright 2019, Optimizely, Inc. and contributors *
* Copyright 2019-2020, Optimizely, Inc. and contributors *
* *
* Licensed under the Apache License, Version 2.0 (the "License"); *
* you may not use this file except in compliance with the License. *
Expand Down Expand Up @@ -113,16 +113,17 @@ enum AttributeValue: Codable, Equatable, CustomStringConvertible {

extension AttributeValue {

func isExactMatch(with target: Any) throws -> Bool {
guard let targetValue = AttributeValue(value: target) else {
throw OptimizelyError.evaluateAttributeInvalidType(prettySrc(#function, target: target))
func isExactMatch(with target: Any, condition: String = "", name: String = "") throws -> Bool {

if !self.isValidForExactMatcher() || (self.doubleValue?.isInfinite ?? false) {
throw OptimizelyError.evaluateAttributeInvalidCondition(condition)
}

guard self.isComparable(with: targetValue) else {
throw OptimizelyError.evaluateAttributeInvalidType(prettySrc(#function, target: target))
guard let targetValue = AttributeValue(value: target), self.isComparable(with: targetValue) else {
throw OptimizelyError.evaluateAttributeInvalidType(condition, target, name)
}

try checkValidAttributeNumber(target)
try checkValidAttributeNumber(target, condition: condition, name: name)

// same type and same value
if self == targetValue {
Expand All @@ -136,45 +137,47 @@ extension AttributeValue {

return false
}

func isSubstring(of target: Any) throws -> Bool {

func isSubstring(of target: Any, condition: String = "", name: String = "") throws -> Bool {

guard case .string(let value) = self else {
throw OptimizelyError.evaluateAttributeInvalidType(prettySrc(#function, target: target))
throw OptimizelyError.evaluateAttributeInvalidCondition(condition)
}

guard let targetStr = target as? String else {
throw OptimizelyError.evaluateAttributeInvalidType(prettySrc(#function, target: target))
throw OptimizelyError.evaluateAttributeInvalidType(condition, target, name)
}

return targetStr.contains(value)
}

func isGreater(than target: Any) throws -> Bool {
guard let targetValue = AttributeValue(value: target) else {
throw OptimizelyError.evaluateAttributeInvalidType(prettySrc(#function, target: target))
func isGreater(than target: Any, condition: String = "", name: String = "") throws -> Bool {

guard let currentDouble = self.doubleValue, currentDouble.isFinite else {
throw OptimizelyError.evaluateAttributeInvalidCondition(condition)
}

guard let currentDouble = self.doubleValue,
guard let targetValue = AttributeValue(value: target),
let targetDouble = targetValue.doubleValue else {
throw OptimizelyError.evaluateAttributeInvalidType(prettySrc(#function, target: target))
throw OptimizelyError.evaluateAttributeInvalidType(condition, target, name)
}

try checkValidAttributeNumber(target)

try checkValidAttributeNumber(target, condition: condition, name: name)
return currentDouble > targetDouble
}

func isLess(than target: Any) throws -> Bool {
guard let targetValue = AttributeValue(value: target) else {
throw OptimizelyError.evaluateAttributeInvalidType(prettySrc(#function, target: target))
func isLess(than target: Any, condition: String = "", name: String = "") throws -> Bool {

guard let currentDouble = self.doubleValue, currentDouble.isFinite else {
throw OptimizelyError.evaluateAttributeInvalidCondition(condition)
}

guard let currentDouble = self.doubleValue,
guard let targetValue = AttributeValue(value: target),
let targetDouble = targetValue.doubleValue else {
throw OptimizelyError.evaluateAttributeInvalidType(prettySrc(#function, target: target))
throw OptimizelyError.evaluateAttributeInvalidType(condition, target, name)
}

try checkValidAttributeNumber(target)
try checkValidAttributeNumber(target, condition: condition, name: name)

return currentDouble < targetDouble
}
Expand Down Expand Up @@ -214,7 +217,7 @@ extension AttributeValue {
}
}

func checkValidAttributeNumber(_ number: Any?, caller: String = #function) throws {
func checkValidAttributeNumber(_ number: Any?, condition: String, name: String, caller: String = #function) throws {
// check range for any value types (Int, Int64, Double, Float...)
// do not check value range for string types

Expand All @@ -231,7 +234,17 @@ extension AttributeValue {

// valid range: [-2^53, 2^53]
if abs(num) > pow(2, 53) {
throw OptimizelyError.evaluateAttributeValueOutOfRange(prettySrc(caller, target: number))
throw OptimizelyError.evaluateAttributeValueOutOfRange(condition, name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why removed prettySrc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated it as per python since it also doesn't log the source of the error.

}
}

func isValidForExactMatcher() -> Bool {
switch (self) {
case (.string): return true
case (.int): return true
case (.double): return true
case (.bool): return true
default: return false
}
}

Expand Down
33 changes: 20 additions & 13 deletions Sources/Data Model/Audience/UserAttribute.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/****************************************************************************
* Copyright 2019, Optimizely, Inc. and contributors *
* Copyright 2019-2020, Optimizely, Inc. and contributors *
* *
* Licensed under the Apache License, Version 2.0 (the "License"); *
* you may not use this file except in compliance with the License. *
Expand All @@ -24,7 +24,8 @@ struct UserAttribute: Codable, Equatable {
var type: String?
var match: String?
var value: AttributeValue?

var stringRepresentation: String = ""

enum CodingKeys: String, CodingKey {
case name
case type
Expand Down Expand Up @@ -71,6 +72,7 @@ struct UserAttribute: Codable, Equatable {
self.type = try container.decodeIfPresent(String.self, forKey: .type)
self.match = try container.decodeIfPresent(String.self, forKey: .match)
self.value = try container.decodeIfPresent(AttributeValue.self, forKey: .value)
self.stringRepresentation = Utils.getConditionString(conditions: self)
} catch {
throw DecodingError.dataCorrupted(DecodingError.Context(codingPath: container.codingPath, debugDescription: "Faild to decode User Attribute)"))
}
Expand All @@ -81,6 +83,7 @@ struct UserAttribute: Codable, Equatable {
self.type = type
self.match = match
self.value = value
self.stringRepresentation = Utils.getConditionString(conditions: self)
}
}

Expand All @@ -92,47 +95,51 @@ extension UserAttribute {

// invalid type - parsed for forward compatibility only (but evaluation fails)
if typeSupported == nil {
throw OptimizelyError.userAttributeInvalidType(self.type ?? "empty")
throw OptimizelyError.userAttributeInvalidType(stringRepresentation)
}

// invalid match - parsed for forward compatibility only (but evaluation fails)
guard let matchFinal = matchSupported else {
throw OptimizelyError.userAttributeInvalidMatch(self.match ?? "empty")
throw OptimizelyError.userAttributeInvalidMatch(stringRepresentation)
}

guard let nameFinal = name else {
throw OptimizelyError.userAttributeInvalidFormat("empty name in condition")
throw OptimizelyError.userAttributeInvalidName(stringRepresentation)
}

let attributes = attributes ?? OptimizelyAttributes()

let rawAttributeValue = attributes[nameFinal] ?? nil // default to nil to avoid warning "coerced from 'Any??' to 'Any?'"
let rawAttributeValue = attributes[nameFinal] ?? nil // default to nil to avoid warning "coerced from 'Any??' to 'Any?'"

if matchFinal != .exists {
if !attributes.keys.contains(nameFinal) {
throw OptimizelyError.missingAttributeValue(stringRepresentation, nameFinal)
}

if value == nil {
throw OptimizelyError.userAttributeInvalidFormat("missing value (\(nameFinal)) in condition)")
throw OptimizelyError.userAttributeNilValue(stringRepresentation)
}

if rawAttributeValue == nil {
throw OptimizelyError.evaluateAttributeInvalidFormat("no attribute value for (\(nameFinal))")
throw OptimizelyError.nilAttributeValue(stringRepresentation, nameFinal)
}
}

switch matchFinal {
case .exists:
return !(rawAttributeValue is NSNull || rawAttributeValue == nil)
case .exact:
return try value!.isExactMatch(with: rawAttributeValue!)
return try value!.isExactMatch(with: rawAttributeValue!, condition: stringRepresentation, name: nameFinal)
case .substring:
return try value!.isSubstring(of: rawAttributeValue!)
return try value!.isSubstring(of: rawAttributeValue!, condition: stringRepresentation, name: nameFinal)
case .lt:
// user attribute "less than" this condition value
// so evaluate if this condition value "isGreater" than the user attribute value
return try value!.isGreater(than: rawAttributeValue!)
return try value!.isGreater(than: rawAttributeValue!, condition: stringRepresentation, name: nameFinal)
case .gt:
// user attribute "greater than" this condition value
// so evaluate if this condition value "isLess" than the user attribute value
return try value!.isLess(than: rawAttributeValue!)
return try value!.isLess(than: rawAttributeValue!, condition: stringRepresentation, name: nameFinal)
}
}

Expand Down
27 changes: 25 additions & 2 deletions Sources/Data Model/Project.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/****************************************************************************
* Copyright 2019, Optimizely, Inc. and contributors *
* Copyright 2019-2020, Optimizely, Inc. and contributors *
* *
* Licensed under the Apache License, Version 2.0 (the "License"); *
* you may not use this file except in compliance with the License. *
Expand Down Expand Up @@ -42,6 +42,24 @@ struct Project: Codable, Equatable {
var typedAudiences: [Audience]?
var featureFlags: [FeatureFlag]
var botFiltering: Bool?

let logger = OPTLoggerFactory.getLogger()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a space above to make it clear from the other JSON fields?


// Required since logger in not decodable
enum CodingKeys: String, CodingKey {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where it's needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding let logger = OPTLoggerFactory.getLogger() caused the struct to throw error since logger does not confirm to Codable and Equatable protocols. The solution is to tell the struct explicitly which properties will confirm to Codable and Equatable.

// V2
case version, projectId, experiments, audiences, groups, attributes, accountId, events, revision
// V3
case anonymizeIP
// V4
case rollouts, typedAudiences, featureFlags, botFiltering
}

// Required since logger in not equatable
static func ==(lhs: Project, rhs: Project) -> Bool {
return lhs.version == rhs.version && lhs.projectId == rhs.projectId && lhs.experiments == rhs.experiments &&
lhs.audiences == rhs.audiences && lhs.groups == rhs.groups && lhs.attributes == rhs.attributes && lhs.accountId == rhs.accountId && lhs.events == rhs.events && lhs.revision == rhs.revision && lhs.anonymizeIP == rhs.anonymizeIP && lhs.rollouts == rhs.rollouts && lhs.typedAudiences == rhs.typedAudiences && lhs.featureFlags == rhs.featureFlags && lhs.botFiltering == rhs.botFiltering
}
}

extension Project: ProjectProtocol {
Expand All @@ -50,8 +68,13 @@ extension Project: ProjectProtocol {
guard let audience = getAudience(id: audienceId) else {
throw OptimizelyError.conditionNoMatchingAudience(audienceId)
}
logger.d { () -> String in
return LogMessage.audienceEvaluationStarted(audienceId, Utils.getConditionString(conditions: audience.conditions)).description
}

return try audience.evaluate(project: self, attributes: attributes)
let result = try audience.evaluate(project: self, attributes: attributes)
logger.d(.audienceEvaluationResult(audienceId, result.description))
return result
}

}
Expand Down
21 changes: 5 additions & 16 deletions Sources/Extensions/Array+Extension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,8 @@ extension Array where Element == ThrowableCondition {
}

for eval in self {
do {
if try eval() == false {
return false
}
} catch let error as OptimizelyError {
throw OptimizelyError.conditionInvalidFormat("AND with invalid items [\(error.reason)]")
if try eval() == false {
return false
}
}

Expand All @@ -55,9 +51,8 @@ extension Array where Element == ThrowableCondition {
}

if let error = foundError {
throw OptimizelyError.conditionInvalidFormat("OR with invalid items [\(error.reason)]")
throw error
}

return false
}

Expand All @@ -67,13 +62,7 @@ extension Array where Element == ThrowableCondition {
throw OptimizelyError.conditionInvalidFormat("NOT with empty items")
}

var error: OptimizelyError!
do {
let result = try eval()
return !result
} catch let err as OptimizelyError {
error = OptimizelyError.conditionInvalidFormat("NOT with invalid items [\(err.reason)]")
}
throw error
let result = try eval()
return !result
}
}
36 changes: 20 additions & 16 deletions Sources/Implementation/DefaultDecisionService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,17 @@ class DefaultDecisionService: OPTDecisionService {
return bucketedVariation
}

func isInExperiment(config: ProjectConfig, experiment: Experiment, userId: String, attributes: OptimizelyAttributes) -> Bool {
func isInExperiment(config: ProjectConfig, experiment: Experiment, userId: String, attributes: OptimizelyAttributes, logType: Constants.EvaluationLogType = .experiment, loggingKey: String? = nil) -> Bool {

var result = true // success as default (no condition, etc)
let evType = logType.rawValue
let finalLoggingKey = loggingKey ?? experiment.key

do {
if let conditions = experiment.audienceConditions {
logger.d { () -> String in
return LogMessage.evaluatingAudiencesCombined(evType, finalLoggingKey, Utils.getConditionString(conditions: conditions)).description
}
switch conditions {
case .array(let arrConditions):
if arrConditions.count > 0 {
Expand All @@ -100,23 +105,25 @@ class DefaultDecisionService: OPTDecisionService {
result = true
}
}
// backward compatibility with audiencIds list
// backward compatibility with audienceIds list
else if experiment.audienceIds.count > 0 {
var holder = [ConditionHolder]()
holder.append(.logicalOp(.or))
for id in experiment.audienceIds {
holder.append(.leaf(.audienceId(id)))
}

logger.d { () -> String in
return LogMessage.evaluatingAudiencesCombined(evType, finalLoggingKey, Utils.getConditionString(conditions: holder)).description
}
result = try holder.evaluate(project: config.project, attributes: attributes)
}
} catch {
logger.i(error as? OptimizelyError, source: "isInExperiment(experiment: \(experiment.key), userId: \(userId))")
logger.i(error as? OptimizelyError)
result = false
}

logger.i(.audienceEvaluationResultCombined(experiment.key, result.description))

logger.i(.audienceEvaluationResultCombined(evType, finalLoggingKey, result.description))
return result
}

Expand Down Expand Up @@ -193,31 +200,28 @@ class DefaultDecisionService: OPTDecisionService {

// Evaluate all rollout rules except for last one
for index in 0..<rolloutRules.count.advanced(by: -1) {
let loggingKey = index + 1
let experiment = rolloutRules[index]
if isInExperiment(config: config, experiment: experiment, userId: userId, attributes: attributes) {
logger.d(.userMeetsConditionsForTargetingRule(userId, index + 1))

if isInExperiment(config: config, experiment: experiment, userId: userId, attributes: attributes, logType: .rolloutRule, loggingKey: "\(loggingKey)") {
logger.d(.userMeetsConditionsForTargetingRule(userId, loggingKey))
if let variation = bucketer.bucketExperiment(config: config, experiment: experiment, bucketingId: bucketingId) {
logger.d(.userBucketedIntoTargetingRule(userId, index + 1))

logger.d(.userBucketedIntoTargetingRule(userId, loggingKey))
return variation
}
logger.d(.userNotBucketedIntoTargetingRule(userId, index + 1))
logger.d(.userNotBucketedIntoTargetingRule(userId, loggingKey))
break
} else {
logger.d(.userDoesntMeetConditionsForTargetingRule(userId, index + 1))
logger.d(.userDoesntMeetConditionsForTargetingRule(userId, loggingKey))
}
}
// Evaluate fall back rule / last rule now
let experiment = rolloutRules[rolloutRules.count - 1]

if isInExperiment(config: config, experiment: experiment, userId: userId, attributes: attributes) {
if isInExperiment(config: config, experiment: experiment, userId: userId, attributes: attributes, logType: .rolloutRule, loggingKey: "Everyone Else") {
if let variation = bucketer.bucketExperiment(config: config, experiment: experiment, bucketingId: bucketingId) {
logger.d(.userBucketedIntoEveryoneTargetingRule(userId))

return variation
} else {
logger.d(.userNotBucketedIntoEveryoneTargetingRule(userId))
}
}

Expand Down
Loading