-
Notifications
You must be signed in to change notification settings - Fork 3
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 all 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,26 @@ | ||
// Copyright (c) Microsoft Corporation. | ||
// Licensed under the MIT license. | ||
|
||
import { ITargetingContext } from "./common/ITargetingContext"; | ||
import { Variant } from "./variant/Variant"; | ||
|
||
export interface IFeatureManager { | ||
/** | ||
* Get the list of feature names. | ||
*/ | ||
listFeatureNames(): Promise<string[]>; | ||
|
||
/** | ||
* Check if a feature is enabled. | ||
* @param featureName name of the feature. | ||
* @param context an object providing information that can be used to evaluate whether a feature should be on or off. | ||
*/ | ||
isEnabled(featureName: string, context?: unknown): Promise<boolean>; | ||
|
||
/** | ||
* Get the allocated variant of a feature given the targeting context. | ||
* @param featureName name of the feature. | ||
* @param context a targeting context object used to evaluate which variant the user will be assigned. | ||
*/ | ||
getVariant(featureName: string, context: ITargetingContext): Promise<Variant | undefined>; | ||
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're actively debating this now- but I think context here should be unknown. https://github.com/microsoft/FeatureManagement-Dotnet/pull/484/files#r1717629098 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. Given the possibility that we calculate audience context id based on any other property rather than userid, I agree that the context is unknown (or to limit it to an object?). A good thing is, here all properties in ITargetingContext are optional, meaning it’s compatible with an arbitrary object type. |
||
} |
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,96 @@ | ||||||
// 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. | ||||||
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.
Suggested change
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 commentThe 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! |
||||||
* | ||||||
* @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) { | ||||||
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; | ||||||
} | ||||||
|
||||||
return sourceGroups.some(group => targetedGroups.includes(group)); | ||||||
} | ||||||
|
||||||
/** | ||||||
* 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. | ||||||
* @returns a 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,15 @@ | |
|
||
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 { ITargetingContext } from "./common/ITargetingContext"; | ||
import { isTargetedGroup, isTargetedPercentile, isTargetedUser } from "./common/targetingEvaluator"; | ||
|
||
export class FeatureManager { | ||
export class FeatureManager implements IFeatureManager { | ||
#provider: IFeatureFlagProvider; | ||
#featureFilters: Map<string, IFeatureFilter> = new Map(); | ||
|
||
|
@@ -30,15 +34,48 @@ 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<VariantAssignment> { | ||
// 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)) { | ||
return getVariantAssignment(featureFlag, userAllocation.variant, VariantAssignmentReason.User); | ||
} | ||
} | ||
} | ||
|
||
// 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)) { | ||
return getVariantAssignment(featureFlag, groupAllocation.variant, VariantAssignmentReason.Group); | ||
} | ||
} | ||
} | ||
|
||
// 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)) { | ||
return getVariantAssignment(featureFlag, percentileAllocation.variant, VariantAssignmentReason.Percentile); | ||
} | ||
} | ||
} | ||
|
||
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 +98,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 +112,166 @@ 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
|
||
} else { | ||
variantDef = undefined; | ||
reason = VariantAssignmentReason.DefaultWhenEnabled; | ||
} | ||
} | ||
} | ||
} | ||
|
||
// 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. | ||
} | ||
|
||
/** | ||
* Try to get the variant assignment for the given variant name. If the variant is not found, override the reason with VariantAssignmentReason.None. | ||
* | ||
* @param featureFlag feature flag definition | ||
* @param variantName variant name | ||
* @param reason variant assignment reason | ||
* @returns variant assignment containing the variant definition and the reason | ||
*/ | ||
function getVariantAssignment(featureFlag: FeatureFlag, variantName: string, reason: VariantAssignmentReason): VariantAssignment { | ||
const variant = featureFlag.variants?.find(v => v.name == variantName); | ||
if (variant !== undefined) { | ||
return { variant, reason }; | ||
} else { | ||
console.warn(`Variant ${variantName} not found for feature ${featureFlag.id}.`); | ||
return { variant: undefined, reason: VariantAssignmentReason.None }; | ||
} | ||
} | ||
|
||
type VariantAssignment = { | ||
variant: VariantDefinition | undefined; | ||
reason: VariantAssignmentReason; | ||
}; | ||
|
||
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.
Somewhere you used the third person singular, while in others, you did not.
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 use
Get...
instead of `Gets..." here. But for the properties of options, I use "It gets/specifies ..." if starting with an "It".Those copied from dotnet are using third person singular for verbs I guess. ; ) Yes, they should be updated to be consistent.