-
Notifications
You must be signed in to change notification settings - Fork 2
support feature variant #13
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
Changes from 3 commits
504b6e0
e51b959
3ed3d75
71fe5f3
bf5b0e1
a7f0472
ca66f77
63cdd43
422b562
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 |
---|---|---|
@@ -0,0 +1,7 @@ | ||
// Copyright (c) Microsoft Corporation. | ||
// Licensed under the MIT license. | ||
|
||
export interface IFeatureManager { | ||
listFeatureNames(): Promise<string[]>; | ||
isEnabled(featureName: string, context?: unknown): Promise<boolean>; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
// Copyright (c) Microsoft Corporation. | ||
// Licensed under the MIT license. | ||
|
||
export interface ITargetingContext { | ||
userId?: string; | ||
groups?: string[]; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
// Copyright (c) Microsoft Corporation. | ||
// Licensed under the MIT license. | ||
|
||
import { createHash } from "crypto"; | ||
|
||
/** | ||
* Determines if the user is part of the audience, based on the user id and the percentage range. | ||
* | ||
* @param userId user id from app context | ||
* @param hint hint string to be included in the context id | ||
* @param from percentage range start | ||
* @param to percentage range end | ||
* @returns true if the user is part of the audience, false otherwise | ||
*/ | ||
export function isTargetedPercentile(userId: string | undefined, hint: string, from: number, to: number): boolean { | ||
if (from < 0 || from > 100) { | ||
throw new Error("The 'from' value must be between 0 and 100."); | ||
} | ||
if (to <= 0 || to > 100) { | ||
zhiyuanliang-ms marked this conversation as resolved.
Show resolved
Hide resolved
|
||
throw new Error("The 'to' value must be between 0 and 100."); | ||
} | ||
if (from > to) { | ||
throw new Error("The 'from' value cannot be larger than the 'to' value."); | ||
} | ||
|
||
const audienceContextId = constructAudienceContextId(userId, hint); | ||
|
||
// Cryptographic hashing algorithms ensure adequate entropy across hash values. | ||
const contextMarker = stringToUint32(audienceContextId); | ||
const contextPercentage = (contextMarker / 0xFFFFFFFF) * 100; | ||
|
||
// Handle edge case of exact 100 bucket | ||
if (to === 100) { | ||
return contextPercentage >= from; | ||
} | ||
|
||
return contextPercentage >= from && contextPercentage < to; | ||
} | ||
|
||
/** | ||
* Determines if the user is part of the audience, based on the groups they belong to. | ||
* | ||
* @param sourceGroups user groups from app context | ||
* @param targetedGroups targeted groups from feature configuration | ||
* @returns true if the user is part of the audience, false otherwise | ||
*/ | ||
export function isTargetedGroup(sourceGroups: string[] | undefined, targetedGroups: string[]): boolean { | ||
if (sourceGroups === undefined) { | ||
return false; | ||
} | ||
|
||
for (const group of sourceGroups) { | ||
if (targetedGroups.includes(group)) { | ||
return true; | ||
} | ||
} | ||
|
||
return false; | ||
Eskibear marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
/** | ||
* Determines if the user is part of the audience, based on the user id. | ||
* @param userId user id from app context | ||
* @param users targeted users from feature configuration | ||
* @returns true if the user is part of the audience, false otherwise | ||
*/ | ||
export function isTargetedUser(userId: string | undefined, users: string[]): boolean { | ||
if (userId === undefined) { | ||
return false; | ||
} | ||
|
||
return users.includes(userId); | ||
} | ||
|
||
/** | ||
* Constructs the context id for the audience. | ||
* The context id is used to determine if the user is part of the audience for a feature. | ||
* | ||
* @param userId userId from app context | ||
* @param hint hint string to be included in the context id | ||
* @returns a string that represents the context id for the audience | ||
*/ | ||
function constructAudienceContextId(userId: string | undefined, hint: string): string { | ||
return `${userId ?? ""}\n${hint}`; | ||
} | ||
|
||
/** | ||
* Converts a string to a uint32 in little-endian encoding. | ||
* @param str The string to convert. | ||
Eskibear marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* @returns The uint32 value. | ||
*/ | ||
function stringToUint32(str: string): number { | ||
// Create a SHA-256 hash of the string | ||
const hash = createHash("sha256").update(str).digest(); | ||
|
||
// Get the first 4 bytes of the hash | ||
const first4Bytes = hash.subarray(0, 4); | ||
|
||
// Convert the 4 bytes to a uint32 with little-endian encoding | ||
const uint32 = first4Bytes.readUInt32LE(0); | ||
return uint32; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,11 +3,16 @@ | |
|
||
import { TimeWindowFilter } from "./filter/TimeWindowFilter"; | ||
import { IFeatureFilter } from "./filter/FeatureFilter"; | ||
import { RequirementType } from "./model"; | ||
import { FeatureFlag, RequirementType, VariantDefinition } from "./model"; | ||
import { IFeatureFlagProvider } from "./featureProvider"; | ||
import { TargetingFilter } from "./filter/TargetingFilter"; | ||
import { Variant } from "./variant/Variant"; | ||
import { IFeatureManager } from "./IFeatureManager"; | ||
import { IVariantFeatureManager } from "./variant/IVariantFeatureManager"; | ||
import { ITargetingContext } from "./common/ITargetingContext"; | ||
import { isTargetedGroup, isTargetedPercentile, isTargetedUser } from "./common/targetingEvaluator"; | ||
|
||
export class FeatureManager { | ||
export class FeatureManager implements IFeatureManager, IVariantFeatureManager { | ||
#provider: IFeatureFlagProvider; | ||
#featureFilters: Map<string, IFeatureFilter> = new Map(); | ||
|
||
|
@@ -30,15 +35,68 @@ export class FeatureManager { | |
|
||
// If multiple feature flags are found, the first one takes precedence. | ||
async isEnabled(featureName: string, context?: unknown): Promise<boolean> { | ||
const featureFlag = await this.#provider.getFeatureFlag(featureName); | ||
if (featureFlag === undefined) { | ||
// If the feature is not found, then it is disabled. | ||
return false; | ||
const result = await this.#evaluateFeature(featureName, context); | ||
return result.enabled; | ||
} | ||
|
||
async getVariant(featureName: string, context?: ITargetingContext): Promise<Variant | undefined> { | ||
const result = await this.#evaluateFeature(featureName, context); | ||
return result.variant; | ||
} | ||
|
||
async #assignVariant(featureFlag: FeatureFlag, context: ITargetingContext): Promise<{ | ||
variant: VariantDefinition | undefined; | ||
reason: VariantAssignmentReason; | ||
}> { | ||
// user allocation | ||
if (featureFlag.allocation?.user !== undefined) { | ||
rossgrambo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for (const userAllocation of featureFlag.allocation.user) { | ||
if (isTargetedUser(context.userId, userAllocation.users)) { | ||
const variant = featureFlag.variants?.find(v => v.name == userAllocation.variant); | ||
if (variant !== undefined) { | ||
return { variant, reason: VariantAssignmentReason.User }; | ||
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. Should it be { variant: variant, reason: VariantAssignmentReason.User }? 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.
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 am ok with it. But personally, I prefer consistency. Is there linting rule for this? 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. Of course there is corresponding linting rule, it's called object-shorthand. |
||
} else { | ||
console.warn(`Variant ${userAllocation.variant} not found for feature ${featureFlag.id}.`); | ||
Eskibear marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
} | ||
} | ||
|
||
// Ensure that the feature flag is in the correct format. Feature providers should validate the feature flags, but we do it here as a safeguard. | ||
validateFeatureFlagFormat(featureFlag); | ||
// group allocation | ||
if (featureFlag.allocation?.group !== undefined) { | ||
for (const groupAllocation of featureFlag.allocation.group) { | ||
if (isTargetedGroup(context.groups, groupAllocation.groups)) { | ||
const variant = featureFlag.variants?.find(v => v.name == groupAllocation.variant); | ||
if (variant !== undefined) { | ||
return { variant, reason: VariantAssignmentReason.Group }; | ||
} else { | ||
console.warn(`Variant ${groupAllocation.variant} not found for feature ${featureFlag.id}.`); | ||
return { variant: undefined, reason: VariantAssignmentReason.None }; | ||
} | ||
} | ||
} | ||
} | ||
|
||
// percentile allocation | ||
if (featureFlag.allocation?.percentile !== undefined) { | ||
for (const percentileAllocation of featureFlag.allocation.percentile) { | ||
const hint = featureFlag.allocation.seed ?? `allocation\n${featureFlag.id}`; | ||
if (isTargetedPercentile(context.userId, hint, percentileAllocation.from, percentileAllocation.to)) { | ||
const variant = featureFlag.variants?.find(v => v.name == percentileAllocation.variant); | ||
if (variant !== undefined) { | ||
return { variant, reason: VariantAssignmentReason.Percentile }; | ||
} else { | ||
console.warn(`Variant ${percentileAllocation.variant} not found for feature ${featureFlag.id}.`); | ||
return { variant: undefined, reason: VariantAssignmentReason.None }; | ||
} | ||
rossgrambo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
} | ||
|
||
return { variant: undefined, reason: VariantAssignmentReason.None }; | ||
} | ||
|
||
async #isEnabled(featureFlag: FeatureFlag, context?: unknown): Promise<boolean> { | ||
if (featureFlag.enabled !== true) { | ||
// If the feature is not explicitly enabled, then it is disabled by default. | ||
return false; | ||
|
@@ -61,7 +119,7 @@ export class FeatureManager { | |
|
||
for (const clientFilter of clientFilters) { | ||
const matchedFeatureFilter = this.#featureFilters.get(clientFilter.name); | ||
const contextWithFeatureName = { featureName, parameters: clientFilter.parameters }; | ||
const contextWithFeatureName = { featureName: featureFlag.id, parameters: clientFilter.parameters }; | ||
if (matchedFeatureFilter === undefined) { | ||
console.warn(`Feature filter ${clientFilter.name} is not found.`); | ||
return false; | ||
|
@@ -75,14 +133,140 @@ export class FeatureManager { | |
return !shortCircuitEvaluationResult; | ||
} | ||
|
||
async #evaluateFeature(featureName: string, context: unknown): Promise<EvaluationResult> { | ||
const featureFlag = await this.#provider.getFeatureFlag(featureName); | ||
const result = new EvaluationResult(featureFlag); | ||
|
||
if (featureFlag === undefined) { | ||
return result; | ||
} | ||
|
||
// Ensure that the feature flag is in the correct format. Feature providers should validate the feature flags, but we do it here as a safeguard. | ||
// TODO: move to the feature flag provider implementation. | ||
validateFeatureFlagFormat(featureFlag); | ||
|
||
// Evaluate if the feature is enabled. | ||
result.enabled = await this.#isEnabled(featureFlag, context); | ||
|
||
// Determine Variant | ||
let variantDef: VariantDefinition | undefined; | ||
let reason: VariantAssignmentReason = VariantAssignmentReason.None; | ||
rossgrambo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// featureFlag.variant not empty | ||
if (featureFlag.variants !== undefined && featureFlag.variants.length > 0) { | ||
if (!result.enabled) { | ||
// not enabled, assign default if specified | ||
if (featureFlag.allocation?.default_when_disabled !== undefined) { | ||
variantDef = featureFlag.variants.find(v => v.name == featureFlag.allocation?.default_when_disabled); | ||
reason = VariantAssignmentReason.DefaultWhenDisabled; | ||
} else { | ||
// no default specified | ||
variantDef = undefined; | ||
reason = VariantAssignmentReason.DefaultWhenDisabled; | ||
} | ||
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. reason = VariantAssignmentReason.DefaultWhenDisabled; should be put here outside the if-else 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 decide to keep it in the if-else block together with |
||
} else { | ||
// enabled, assign based on allocation | ||
if (context !== undefined && featureFlag.allocation !== undefined) { | ||
const variantAndReason = await this.#assignVariant(featureFlag, context as ITargetingContext); | ||
variantDef = variantAndReason.variant; | ||
reason = variantAndReason.reason; | ||
} | ||
|
||
// allocation failed, assign default if specified | ||
if (variantDef === undefined && reason === VariantAssignmentReason.None) { | ||
if (featureFlag.allocation?.default_when_enabled !== undefined) { | ||
variantDef = featureFlag.variants.find(v => v.name == featureFlag.allocation?.default_when_enabled); | ||
reason = VariantAssignmentReason.DefaultWhenEnabled; | ||
Eskibear marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
} | ||
} | ||
|
||
// TODO: send telemetry for variant assignment reason in the future. | ||
console.log(`Variant assignment for feature ${featureName}: ${variantDef?.name ?? "default"} (${reason})`); | ||
|
||
if (variantDef?.configuration_reference !== undefined) { | ||
console.warn("Configuration reference is not supported yet."); | ||
} | ||
|
||
result.variant = variantDef !== undefined ? new Variant(variantDef.name, variantDef.configuration_value) : undefined; | ||
result.variantAssignmentReason = reason; | ||
|
||
// Status override for isEnabled | ||
if (variantDef !== undefined && featureFlag.enabled) { | ||
if (variantDef.status_override === "Enabled") { | ||
result.enabled = true; | ||
} else if (variantDef.status_override === "Disabled") { | ||
result.enabled = false; | ||
} | ||
} | ||
|
||
return result; | ||
} | ||
} | ||
|
||
interface FeatureManagerOptions { | ||
customFilters?: IFeatureFilter[]; | ||
} | ||
|
||
/** | ||
* Validates the format of the feature flag definition. | ||
* | ||
* FeatureFlag data objects are from IFeatureFlagProvider, depending on the implementation. | ||
* Thus the properties are not guaranteed to have the expected types. | ||
* | ||
* @param featureFlag The feature flag definition to validate. | ||
*/ | ||
function validateFeatureFlagFormat(featureFlag: any): void { | ||
if (featureFlag.enabled !== undefined && typeof featureFlag.enabled !== "boolean") { | ||
throw new Error(`Feature flag ${featureFlag.id} has an invalid 'enabled' value.`); | ||
} | ||
// TODO: add more validations. | ||
// TODO: should be moved to the feature flag provider. | ||
} | ||
|
||
enum VariantAssignmentReason { | ||
/** | ||
* Variant allocation did not happen. No variant is assigned. | ||
*/ | ||
None, | ||
|
||
/** | ||
* The default variant is assigned when a feature flag is disabled. | ||
*/ | ||
DefaultWhenDisabled, | ||
|
||
/** | ||
* The default variant is assigned because of no applicable user/group/percentile allocation when a feature flag is enabled. | ||
*/ | ||
DefaultWhenEnabled, | ||
|
||
/** | ||
* The variant is assigned because of the user allocation when a feature flag is enabled. | ||
*/ | ||
User, | ||
|
||
/** | ||
* The variant is assigned because of the group allocation when a feature flag is enabled. | ||
*/ | ||
Group, | ||
|
||
/** | ||
* The variant is assigned because of the percentile allocation when a feature flag is enabled. | ||
*/ | ||
Percentile | ||
} | ||
|
||
class EvaluationResult { | ||
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. Should this also be exported, since developers will need it to send telemetry. |
||
constructor( | ||
// feature flag definition | ||
public readonly feature: FeatureFlag | undefined, | ||
|
||
// enabled state | ||
public enabled: boolean = false, | ||
|
||
// variant assignment | ||
public variant: Variant | undefined = undefined, | ||
public variantAssignmentReason: VariantAssignmentReason = VariantAssignmentReason.None | ||
) { } | ||
} |
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 think percentile is slightly more explicit here. I'd also use it in the from and to param comments.
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.
@zhiyuanliang-ms please help to follow up with the wording in comments, thanks!