Skip to content

Commit 8dd11d5

Browse files
feat!: fractional bucketing improvements (#1501)
Signed-off-by: Lea Konvalinka <lea.konvalinka@dynatrace.com> Signed-off-by: Todd Baert <todd.baert@dynatrace.com> Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
1 parent 6e24e71 commit 8dd11d5

8 files changed

Lines changed: 277 additions & 41 deletions

File tree

libs/providers/flagd/src/e2e/tests/in-process.spec.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ describe('in-process', () => {
2424
loadFeatures(GHERKIN_FLAGD, {
2525
// remove filters as we add support for features
2626
// see: https://github.com/open-feature/js-sdk-contrib/issues/1096 and child issues
27-
tagFilter: '@in-process and not @targetURI and not @sync and not @unixsocket and not @deprecated',
27+
tagFilter:
28+
'@in-process and not @targetURI and not @sync and not @unixsocket and not @deprecated and not @fractional-v1 and not @operator-errors',
2829
scenarioNameTemplate: (vars) => {
2930
return `${vars.scenarioTitle} (${vars.scenarioTags.join(',')} ${vars.featureTags.join(',')})`;
3031
},

libs/providers/flagd/src/e2e/tests/rpc.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ describe('rpc', () => {
2525
tagFilter:
2626
// remove filters as we add support for features
2727
// see: https://github.com/open-feature/js-sdk-contrib/issues/1096 and child issues
28-
'@rpc and not @targetURI and not @caching and not @unixsocket and not @deprecated',
28+
'@rpc and not @targetURI and not @caching and not @unixsocket and not @deprecated and not @fractional-v1 and not @operator-errors',
2929
scenarioNameTemplate: (vars) => {
3030
return `${vars.scenarioTitle} (${vars.scenarioTags.join(',')} ${vars.featureTags.join(',')})`;
3131
},

libs/shared/flagd-core/src/lib/targeting/fractional.ts

Lines changed: 35 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,17 @@ import type { EvaluationContextWithLogger } from './common';
55

66
export const fractionalRule = 'fractional';
77

8-
export function fractional(data: unknown, context: EvaluationContextWithLogger): string | null {
8+
type VariantValue = string | number | boolean | null;
9+
10+
export function fractional(data: unknown, context: EvaluationContextWithLogger): VariantValue {
911
const logger = getLoggerFromContext(context);
1012
if (!Array.isArray(data)) {
1113
return null;
1214
}
1315

1416
const args = Array.from(data);
15-
if (args.length < 2) {
16-
logger.debug(`Invalid ${fractionalRule} configuration: Expected at least 2 buckets, got ${args.length}`);
17+
if (args.length < 1) {
18+
logger.debug(`Invalid ${fractionalRule} configuration: Expected at least 1 bucket, got ${args.length}`);
1719
return null;
1820
}
1921

@@ -48,37 +50,36 @@ export function fractional(data: unknown, context: EvaluationContextWithLogger):
4850
return null;
4951
}
5052

51-
// hash in signed 32 format. Bitwise operation here works in signed 32 hence the conversion
52-
const hash = new MurmurHash3(bucketBy).result() | 0;
53-
const bucket = (Math.abs(hash) / 2147483648) * 100;
53+
const MAX_WEIGHT = 2147483647;
54+
if (bucketingList.totalWeight > MAX_WEIGHT) {
55+
logger.debug(
56+
`Invalid ${fractionalRule} configuration: sum of weights exceeds Math.MaxInt32 (${MAX_WEIGHT}), got ${bucketingList.totalWeight}`,
57+
);
58+
return null;
59+
}
60+
61+
const hashUint32 = BigInt(new MurmurHash3(bucketBy).result() >>> 0);
62+
const bucket = (hashUint32 * BigInt(bucketingList.totalWeight)) >> BigInt(32);
5463

55-
let sum = 0;
64+
let sum = BigInt(0);
5665
for (let i = 0; i < bucketingList.fractions.length; i++) {
5766
const bucketEntry = bucketingList.fractions[i];
5867

59-
sum += relativeWeight(bucketingList.totalWeight, bucketEntry.fraction);
68+
sum += BigInt(bucketEntry.fraction);
6069

61-
if (sum >= bucket) {
70+
if (sum > bucket) {
6271
return bucketEntry.variant;
6372
}
6473
}
6574

6675
return null;
6776
}
6877

69-
function relativeWeight(totalWeight: number, weight: number): number {
70-
if (weight == 0) {
71-
return 0;
72-
}
73-
return (weight * 100) / totalWeight;
74-
}
75-
7678
function toBucketingList(from: unknown[]): {
77-
fractions: { variant: string; fraction: number }[];
79+
fractions: { variant: VariantValue; fraction: number }[];
7880
totalWeight: number;
7981
} {
80-
// extract bucketing options
81-
const bucketingArray: { variant: string; fraction: number }[] = [];
82+
const bucketingArray: { variant: VariantValue; fraction: number }[] = [];
8283

8384
let totalWeight = 0;
8485
for (let i = 0; i < from.length; i++) {
@@ -91,19 +92,28 @@ function toBucketingList(from: unknown[]): {
9192
throw new Error('Invalid bucketing entry. Requires at least a variant');
9293
}
9394

94-
if (typeof entry[0] !== 'string') {
95-
throw new Error('Bucketing require variant to be present in string format');
95+
let variant: VariantValue;
96+
if (typeof entry[0] === 'string' || typeof entry[0] === 'number' || typeof entry[0] === 'boolean') {
97+
variant = entry[0];
98+
} else if (entry[0] === null || entry[0] === undefined) {
99+
variant = null;
100+
} else {
101+
throw new Error(
102+
'Bucketing requires variant to be a string, number, or boolean (or a JSONLogic expression that evaluates to one)',
103+
);
96104
}
97105

98106
let weight = 1;
99107
if (entry.length >= 2) {
100-
if (typeof entry[1] !== 'number') {
101-
throw new Error('Bucketing require bucketing percentage to be present');
108+
const raw = entry[1];
109+
if (typeof raw !== 'number' || !Number.isInteger(raw)) {
110+
throw new Error('Bucketing requires weight to be an integer');
102111
}
103-
weight = entry[1];
112+
// negative weights can be the result of rollout calculations, so we clamp to 0 rather than throwing an error
113+
weight = Math.max(0, raw);
104114
}
105115

106-
bucketingArray.push({ fraction: weight, variant: entry[0] });
116+
bucketingArray.push({ fraction: weight, variant });
107117
totalWeight += weight;
108118
}
109119

0 commit comments

Comments
 (0)