Skip to content

Commit

Permalink
Respect targeting in persistent evaluation (#503)
Browse files Browse the repository at this point in the history
Grammarly needs to allow targeting rules to still apply for persisted evaluations. Currently, even if a user no longer passes targeting, we return the persisted evaluation.
I added an option to `getExperiment` and `getLayer` that allows you to respect the targeting rules of an experiment even for persisted evaluations

Kong: https://app.graphite.dev/github/pr/statsig-io/kong/2655
  • Loading branch information
kenny-statsig authored Oct 2, 2024
1 parent dbe3040 commit d60885a
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 32 deletions.
4 changes: 4 additions & 0 deletions src/ConfigSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ export class ConfigRule {
});
return conditions;
}

isTargetingRule(): boolean {
return this.id === 'inlineTargetingRules' || this.id === 'targetingGate';
}
}

export class ConfigCondition {
Expand Down
80 changes: 57 additions & 23 deletions src/Evaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,8 @@ export default class Evaluator {
}

_evalConfig(ctx: EvaluationContext): ConfigEvaluation {
const { user, spec, userPersistedValues } = ctx;
const { user, spec, userPersistedValues, persistentAssignmentOptions } =
ctx;
if (userPersistedValues == null || !spec.isActive) {
this.persistentStore.delete(user, spec.idType, spec.name);
return this._evalSpec(ctx);
Expand All @@ -687,7 +688,14 @@ export default class Evaluator {
: null;

if (stickyEvaluation) {
return stickyEvaluation;
if (persistentAssignmentOptions?.enforceTargeting) {
const passesTargeting = this._evalTargeting(ctx);
if (passesTargeting) {
return stickyEvaluation;
}
} else {
return stickyEvaluation;
}
}

const evaluation = this._evalSpec(ctx);
Expand All @@ -700,7 +708,8 @@ export default class Evaluator {
}

_evalLayer(ctx: EvaluationContext): ConfigEvaluation {
const { user, spec, userPersistedValues } = ctx;
const { user, spec, userPersistedValues, persistentAssignmentOptions } =
ctx;
if (!userPersistedValues) {
this.persistentStore.delete(user, spec.idType, spec.name);
return this._evalSpec(ctx);
Expand All @@ -714,31 +723,49 @@ export default class Evaluator {
)
: null;

const isAllocatedExperimentActive = (evaluation: ConfigEvaluation) =>
this._isExperimentActive(
evaluation.config_delegate
? this.store.getConfig(evaluation.config_delegate)
: null,
);

if (stickyEvaluation) {
if (isAllocatedExperimentActive(stickyEvaluation)) {
return stickyEvaluation;
const delegateSpec = stickyEvaluation.config_delegate
? this.store.getConfig(stickyEvaluation.config_delegate)
: null;
if (delegateSpec && delegateSpec.isActive) {
if (persistentAssignmentOptions?.enforceTargeting) {
const passesTargeting = this._evalTargeting(ctx, delegateSpec);
if (passesTargeting) {
return stickyEvaluation;
}
} else {
return stickyEvaluation;
}
} else {
this.persistentStore.delete(user, spec.idType, spec.name);
return this._evalSpec(ctx);
}
} else {
const evaluation = this._evalSpec(ctx);
if (isAllocatedExperimentActive(evaluation)) {
if (evaluation.is_experiment_group) {
this.persistentStore.save(user, spec.idType, spec.name, evaluation);
}
} else {
this.persistentStore.delete(user, spec.idType, spec.name);
}

const evaluation = this._evalSpec(ctx);
const delegateSpec = evaluation.config_delegate
? this.store.getConfig(evaluation.config_delegate)
: null;
if (delegateSpec && delegateSpec.isActive) {
if (evaluation.is_experiment_group) {
this.persistentStore.save(user, spec.idType, spec.name, evaluation);
}
return evaluation;
} else {
this.persistentStore.delete(user, spec.idType, spec.name);
}
return evaluation;
}

_evalTargeting(ctx: EvaluationContext, delegateSpec?: ConfigSpec): boolean {
return (
this._evalSpec(
EvaluationContext.get(ctx.getRequestContext(), {
user: ctx.user,
spec: delegateSpec ?? ctx.spec,
onlyEvaluateTargeting: true,
}),
).value === false
); // Fail evaluation means to pass targeting (fall through logic)
}

_evalSpec(ctx: EvaluationContext): ConfigEvaluation {
Expand Down Expand Up @@ -769,9 +796,16 @@ export default class Evaluator {
);
}

let rules = config.rules;
if (ctx.onlyEvaluateTargeting) {
rules = config.rules.filter((rule) => rule.isTargetingRule());
if (rules.length === 0) {
return new ConfigEvaluation(true);
}
}

let secondary_exposures: SecondaryExposure[] = [];
for (let i = 0; i < config.rules.length; i++) {
const rule = config.rules[i];
for (const rule of rules) {
const ruleResult = this._evalRule(user, rule, ctx);
if (ruleResult.unsupported) {
return ConfigEvaluation.unsupported(
Expand Down
11 changes: 11 additions & 0 deletions src/StatsigOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,21 @@ function normalizeUrl(url: string | null): string | null {
return url && url.endsWith('/') ? url.slice(0, -1) : url;
}

export type PersistentAssignmentOptions = {
/* Whether or not to enforce targeting rules before assigning persisted values */
enforceTargeting?: boolean;
};

export type GetExperimentOptions = {
/* Persisted values to use for experiment assignment */
userPersistedValues?: UserPersistedValues | null;
/* Additional options for using persistent assignment */
persistentAssignmentOptions?: PersistentAssignmentOptions;
};

export type GetLayerOptions = {
/* Persisted values to use for layer assignment */
userPersistedValues?: UserPersistedValues | null;
/* Additional options for using persistent assignment */
persistentAssignmentOptions?: PersistentAssignmentOptions;
};
4 changes: 4 additions & 0 deletions src/StatsigServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ export default class StatsigServer {
caller: 'getExperiment',
configName: experimentName,
userPersistedValues: options?.userPersistedValues,
persistentAssignmentOptions: options?.persistentAssignmentOptions,
}),
);
}
Expand All @@ -342,6 +343,7 @@ export default class StatsigServer {
caller: 'getExperimentWithExposureLoggingDisabled',
configName: experimentName,
userPersistedValues: options?.userPersistedValues,
persistentAssignmentOptions: options?.persistentAssignmentOptions,
}),
);
}
Expand Down Expand Up @@ -402,6 +404,7 @@ export default class StatsigServer {
caller: 'getLayer',
configName: layerName,
userPersistedValues: options?.userPersistedValues,
persistentAssignmentOptions: options?.persistentAssignmentOptions,
}),
);
}
Expand All @@ -419,6 +422,7 @@ export default class StatsigServer {
caller: 'getLayerWithExposureLoggingDisabled',
configName: layerName,
userPersistedValues: options?.userPersistedValues,
persistentAssignmentOptions: options?.persistentAssignmentOptions,
}),
);
}
Expand Down
18 changes: 9 additions & 9 deletions src/utils/StatsigContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
InitializationSource,
} from '../InitializationDetails';
import { UserPersistedValues } from '../interfaces/IUserPersistentStorage';
import { PersistentAssignmentOptions } from '../StatsigOptions';
import { StatsigUser } from '../StatsigUser';

type RequestContext = {
Expand All @@ -17,6 +18,7 @@ type RequestContext = {
user?: StatsigUser;
spec?: ConfigSpec;
userPersistedValues?: UserPersistedValues | null;
persistentAssignmentOptions?: PersistentAssignmentOptions;
};

export class StatsigContext {
Expand All @@ -28,6 +30,7 @@ export class StatsigContext {
readonly hash?: string;
readonly bypassDedupe?: boolean;
readonly userPersistedValues?: UserPersistedValues | null;
readonly persistentAssignmentOptions?: PersistentAssignmentOptions;

protected constructor(protected ctx: RequestContext) {
this.startTime = Date.now();
Expand All @@ -38,6 +41,7 @@ export class StatsigContext {
this.hash = ctx.clientKey;
this.bypassDedupe = ctx.bypassDedupe;
this.userPersistedValues = ctx.userPersistedValues;
this.persistentAssignmentOptions = ctx.persistentAssignmentOptions;
}

// Create a new context to avoid modifying context up the stack
Expand All @@ -64,17 +68,20 @@ export class EvaluationContext extends StatsigContext {
readonly user: StatsigUser;
readonly spec: ConfigSpec;
readonly targetAppID?: string;
readonly onlyEvaluateTargeting?: boolean;

protected constructor(
ctx: RequestContext,
user: StatsigUser,
spec: ConfigSpec,
targetAppID?: string,
onlyEvaluateTargeting?: boolean,
) {
super(ctx);
this.user = user;
this.spec = spec;
this.targetAppID = targetAppID;
this.onlyEvaluateTargeting = onlyEvaluateTargeting;
}

public static new(
Expand All @@ -90,22 +97,15 @@ export class EvaluationContext extends StatsigContext {
user: StatsigUser;
spec: ConfigSpec;
targetAppID?: string;
onlyEvaluateTargeting?: boolean;
},
): EvaluationContext {
return new EvaluationContext(
ctx,
evalCtx.user,
evalCtx.spec,
evalCtx.targetAppID,
);
}

public withTargetAppID(targetAppID: string): EvaluationContext {
return new EvaluationContext(
this.getRequestContext(),
this.user,
this.spec,
targetAppID,
evalCtx.onlyEvaluateTargeting,
);
}
}
Expand Down

0 comments on commit d60885a

Please sign in to comment.