Skip to content

Commit 5cb36c7

Browse files
satyakighsatyakigh
committed
Improve feature flag building
1 parent 2322485 commit 5cb36c7

File tree

11 files changed

+178
-99
lines changed

11 files changed

+178
-99
lines changed

‎assets/featureFlag/alpha.json‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,13 @@
33
"description": "Feature flags for alpha environment",
44
"features": {
55
"EnhancedDryRun": {
6-
"enabled": true
7-
},
8-
"AnotherFeature": {
9-
"enabled": false,
10-
"fleetPercentage": 10.5
6+
"enabled": true,
7+
"fleetPercentage": 100,
8+
"allowlistedRegions": [
9+
"us-east-1",
10+
"us-west-2",
11+
"eu-west-1"
12+
]
1113
}
1214
}
13-
}
15+
}

‎assets/featureFlag/beta.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@
33
"description": "Feature flags for beta environment",
44
"features": {
55
}
6-
}
6+
}

‎assets/featureFlag/prod.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@
33
"description": "Feature flags for prod environment",
44
"features": {
55
}
6-
}
6+
}

‎src/featureFlag/CombinedFeatureFlags.ts‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { hostname } from 'os';
22
import { FleetTargetedFeatureFlag } from './FeatureFlag';
3-
import { FeatureFlag } from './FeatureFlagI';
3+
import { FeatureFlag, TargetedFeatureFlag } from './FeatureFlagI';
44

55
export class LocalHostTargetedFeatureFlag implements FeatureFlag {
66
private readonly enabled: boolean;
@@ -42,3 +42,22 @@ export class AndFeatureFlag implements FeatureFlag {
4242
.join(', ');
4343
}
4444
}
45+
46+
export class CompoundFeatureFlag<T> implements TargetedFeatureFlag<T> {
47+
constructor(
48+
private readonly featureFlag: FeatureFlag,
49+
private readonly targetedFeatureFlag: TargetedFeatureFlag<T>,
50+
) {}
51+
52+
isEnabled(target: T): boolean {
53+
return this.featureFlag.isEnabled() && this.targetedFeatureFlag.isEnabled(target);
54+
}
55+
56+
describe(): string {
57+
return [this.featureFlag, this.targetedFeatureFlag]
58+
.map((feature) => {
59+
return feature.describe();
60+
})
61+
.join(', ');
62+
}
63+
}

‎src/featureFlag/FeatureFlag.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,20 +37,24 @@ export class FleetTargetedFeatureFlag implements TargetedFeatureFlag<string> {
3737
}
3838

3939
export class RegionAllowlistFeatureFlag implements TargetedFeatureFlag<string> {
40+
private readonly allowlist: Set<AwsRegion>;
41+
4042
constructor(
4143
private readonly featureName: string,
42-
private readonly allowedRegions: Set<AwsRegion>,
43-
) {}
44+
allowedRegions: AwsRegion[],
45+
) {
46+
this.allowlist = new Set(allowedRegions);
47+
}
4448

4549
isEnabled(region: string): boolean {
4650
try {
47-
return this.allowedRegions.has(getRegion(region));
51+
return this.allowlist.has(getRegion(region));
4852
} catch {
4953
return false;
5054
}
5155
}
5256

5357
describe(): string {
54-
return `RegionAllowlistFeatureFlag(feature=${this.featureName}, regions=[${[...this.allowedRegions].join(', ')}])`;
58+
return `RegionAllowlistFeatureFlag(feature=${this.featureName}, regions=[${[...this.allowlist].join(', ')}])`;
5559
}
5660
}

‎src/featureFlag/FeatureFlagBuilder.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import { z } from 'zod';
2+
import { AwsRegion } from '../utils/Region';
3+
import { AndFeatureFlag, LocalHostTargetedFeatureFlag } from './CombinedFeatureFlags';
4+
import { FleetTargetedFeatureFlag, RegionAllowlistFeatureFlag, StaticFeatureFlag } from './FeatureFlag';
5+
6+
const FeatureFlagSchema = z.object({
7+
enabled: z.boolean(),
8+
fleetPercentage: z.number().optional(),
9+
allowlistedRegions: z.array(z.enum(Object.values(AwsRegion))).optional(),
10+
});
11+
12+
export const FeatureFlagConfigSchema = z.object({
13+
version: z.number(),
14+
description: z.string(),
15+
features: z.record(z.string(), FeatureFlagSchema),
16+
});
17+
export type FeatureFlagConfigType = z.infer<typeof FeatureFlagSchema>;
18+
19+
export function buildStatic(name: string, config?: FeatureFlagConfigType) {
20+
let enabled = false;
21+
22+
if (config?.enabled !== undefined) {
23+
enabled = config.enabled;
24+
}
25+
26+
return new StaticFeatureFlag(name, enabled);
27+
}
28+
29+
export function buildLocalHost(name: string, config?: FeatureFlagConfigType) {
30+
let pct = 0;
31+
32+
if (config?.fleetPercentage !== undefined) {
33+
pct = config.fleetPercentage;
34+
}
35+
36+
return new AndFeatureFlag(
37+
buildStatic(name, config),
38+
new LocalHostTargetedFeatureFlag(new FleetTargetedFeatureFlag(name, pct)),
39+
);
40+
}
41+
42+
export function buildRegional(name: string, config?: FeatureFlagConfigType) {
43+
let allowlist: AwsRegion[] = [];
44+
45+
if (config?.allowlistedRegions !== undefined) {
46+
allowlist = config.allowlistedRegions;
47+
}
48+
49+
return new RegionAllowlistFeatureFlag(name, allowlist);
50+
}

‎src/featureFlag/FeatureFlagConfig.ts‎

Lines changed: 14 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,16 @@
1-
import { z } from 'zod';
2-
import { AwsRegion } from '../utils/Region';
3-
import { AndFeatureFlag, LocalHostTargetedFeatureFlag } from './CombinedFeatureFlags';
4-
import { FleetTargetedFeatureFlag, StaticFeatureFlag } from './FeatureFlag';
1+
import { CompoundFeatureFlag } from './CombinedFeatureFlags';
2+
import { StaticFeatureFlag } from './FeatureFlag';
3+
import { buildLocalHost, buildRegional, FeatureFlagConfigSchema, FeatureFlagConfigType } from './FeatureFlagBuilder';
54
import { Describable, FeatureFlag, TargetedFeatureFlag } from './FeatureFlagI';
65

76
export class FeatureFlagConfig implements Describable {
8-
private readonly EnhancedDryRun: FeatureFlag;
9-
private readonly AnotherFeature: FeatureFlag;
7+
private readonly StaticFlag = new StaticFeatureFlag('TestFlag', false); // Here to generate types
8+
private readonly EnhancedDryRun: TargetedFeatureFlag<string>;
109

1110
private readonly describables: Describable[];
1211

1312
constructor(config?: unknown) {
14-
let features: Record<string, FeatureFlagType>;
13+
let features: Record<string, FeatureFlagConfigType>;
1514

1615
if (config) {
1716
const parsed = FeatureFlagConfigSchema.parse(config);
@@ -20,17 +19,19 @@ export class FeatureFlagConfig implements Describable {
2019
features = {};
2120
}
2221

23-
this.EnhancedDryRun = buildStatic('EnhancedDryRun', features);
24-
this.AnotherFeature = buildLocalHost('AnotherFeature', features);
22+
this.EnhancedDryRun = new CompoundFeatureFlag(
23+
buildLocalHost('EnhancedDryRun', features['EnhancedDryRun']),
24+
buildRegional('EnhancedDryRun', features['EnhancedDryRun']),
25+
);
2526

26-
this.describables = [this.EnhancedDryRun, this.AnotherFeature];
27+
this.describables = [this.EnhancedDryRun];
2728
}
2829

2930
get(key: FeatureFlagConfigKey): FeatureFlag {
3031
return this[key];
3132
}
3233

33-
getTargeted<T>(key: FeatureFlagConfigKey): TargetedFeatureFlag<T> {
34+
getTargeted(key: TargetedFeatureFlagConfigKey): TargetedFeatureFlag<unknown> {
3435
return this[key];
3536
}
3637

@@ -51,40 +52,5 @@ export class FeatureFlagConfig implements Describable {
5152
}
5253
}
5354

54-
export type FeatureFlagConfigKey = 'EnhancedDryRun' | 'AnotherFeature';
55-
56-
const FeatureFlagSchema = z.object({
57-
enabled: z.boolean(),
58-
fleetPercentage: z.number().optional(),
59-
allowlistedRegions: z.array(z.enum(Object.values(AwsRegion))).optional(),
60-
});
61-
62-
const FeatureFlagConfigSchema = z.object({
63-
version: z.number(),
64-
description: z.string(),
65-
features: z.record(z.string(), FeatureFlagSchema),
66-
});
67-
type FeatureFlagType = z.infer<typeof FeatureFlagSchema>;
68-
69-
function buildStatic(name: string, features: Record<string, FeatureFlagType>) {
70-
let enabled = false;
71-
72-
if (features[name] !== undefined) {
73-
enabled = features[name].enabled;
74-
}
75-
76-
return new StaticFeatureFlag(name, enabled);
77-
}
78-
79-
function buildLocalHost(name: string, features: Record<string, FeatureFlagType>) {
80-
let pct = 0;
81-
82-
if (features[name]?.fleetPercentage !== undefined) {
83-
pct = features[name].fleetPercentage;
84-
}
85-
86-
return new AndFeatureFlag(
87-
buildStatic(name, features),
88-
new LocalHostTargetedFeatureFlag(new FleetTargetedFeatureFlag(name, pct)),
89-
);
90-
}
55+
export type TargetedFeatureFlagConfigKey = 'EnhancedDryRun';
56+
export type FeatureFlagConfigKey = 'StaticFlag';

‎src/featureFlag/FeatureFlagProvider.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ import axios from 'axios';
44
import { LoggerFactory } from '../telemetry/LoggerFactory';
55
import { Closeable } from '../utils/Closeable';
66
import { AwsEnv } from '../utils/Environment';
7-
import { FeatureFlagConfig, FeatureFlagConfigKey } from './FeatureFlagConfig';
8-
import { Describable } from './FeatureFlagI';
7+
import { FeatureFlagConfig, FeatureFlagConfigKey, TargetedFeatureFlagConfigKey } from './FeatureFlagConfig';
8+
import { Describable, FeatureFlag, TargetedFeatureFlag } from './FeatureFlagI';
99

1010
const log = LoggerFactory.getLogger('FeatureFlagProvider');
1111

@@ -27,21 +27,21 @@ export class FeatureFlagProvider implements Closeable {
2727

2828
// https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2022-11-28#primary-rate-limit-for-unauthenticated-users
2929
// GitHub rate limits unauthenticated users to 60 requests per minute, so our refresh cycle has to be less than that
30-
// Using 2 mins i.e. 30 requests in 1 hour
30+
// Using 5 mins i.e. 12 requests in 1 hour
3131
this.timeout = setInterval(
3232
() => {
3333
this.refresh().catch(log.error);
3434
},
35-
2 * 60 * 1000,
35+
5 * 60 * 1000,
3636
);
3737
}
3838

39-
get(key: FeatureFlagConfigKey): boolean {
40-
return this.config.get(key).isEnabled();
39+
get(key: FeatureFlagConfigKey): FeatureFlag {
40+
return this.config.get(key);
4141
}
4242

43-
getTargeted<T>(key: FeatureFlagConfigKey, target: T): boolean {
44-
return this.config.getTargeted(key).isEnabled(target);
43+
getTargeted<T>(key: TargetedFeatureFlagConfigKey): TargetedFeatureFlag<T> {
44+
return this.config.getTargeted(key);
4545
}
4646

4747
private async refresh() {

‎tst/unit/featureFlag/CombinedFeatureFlags.test.ts‎

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,16 @@
11
import * as os from 'os';
22
import { describe, it, expect, vi } from 'vitest';
3-
import { LocalHostTargetedFeatureFlag, AndFeatureFlag } from '../../../src/featureFlag/CombinedFeatureFlags';
4-
import { FleetTargetedFeatureFlag, StaticFeatureFlag } from '../../../src/featureFlag/FeatureFlag';
3+
import {
4+
LocalHostTargetedFeatureFlag,
5+
AndFeatureFlag,
6+
CompoundFeatureFlag,
7+
} from '../../../src/featureFlag/CombinedFeatureFlags';
8+
import {
9+
FleetTargetedFeatureFlag,
10+
StaticFeatureFlag,
11+
RegionAllowlistFeatureFlag,
12+
} from '../../../src/featureFlag/FeatureFlag';
13+
import { AwsRegion } from '../../../src/utils/Region';
514

615
vi.mock('os', () => ({
716
hostname: vi.fn(),
@@ -93,3 +102,35 @@ describe('AndFeatureFlag', () => {
93102
expect(description).toContain('feature2');
94103
});
95104
});
105+
106+
describe('CompoundFeatureFlag', () => {
107+
it('should enable when both flags are enabled', () => {
108+
const staticFlag = new StaticFeatureFlag('test', true);
109+
const regionalFlag = new RegionAllowlistFeatureFlag('test', [AwsRegion.US_EAST_1]);
110+
const compound = new CompoundFeatureFlag(staticFlag, regionalFlag);
111+
expect(compound.isEnabled('us-east-1')).toBe(true);
112+
});
113+
114+
it('should disable when base flag is disabled', () => {
115+
const staticFlag = new StaticFeatureFlag('test', false);
116+
const regionalFlag = new RegionAllowlistFeatureFlag('test', [AwsRegion.US_EAST_1]);
117+
const compound = new CompoundFeatureFlag(staticFlag, regionalFlag);
118+
expect(compound.isEnabled('us-east-1')).toBe(false);
119+
});
120+
121+
it('should disable when targeted flag is disabled', () => {
122+
const staticFlag = new StaticFeatureFlag('test', true);
123+
const regionalFlag = new RegionAllowlistFeatureFlag('test', [AwsRegion.US_EAST_1]);
124+
const compound = new CompoundFeatureFlag(staticFlag, regionalFlag);
125+
expect(compound.isEnabled('us-west-2')).toBe(false);
126+
});
127+
128+
it('should describe both flags', () => {
129+
const staticFlag = new StaticFeatureFlag('test', true);
130+
const regionalFlag = new RegionAllowlistFeatureFlag('test', [AwsRegion.US_EAST_1]);
131+
const compound = new CompoundFeatureFlag(staticFlag, regionalFlag);
132+
const description = compound.describe();
133+
expect(description).toContain('StaticFeatureFlag');
134+
expect(description).toContain('RegionAllowlistFeatureFlag');
135+
});
136+
});

‎tst/unit/featureFlag/FeatureFlag.test.ts‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,32 +50,29 @@ describe('FleetTargetedFeatureFlag', () => {
5050

5151
describe('RegionAllowlistFeatureFlag', () => {
5252
it('should enable for allowlisted region', () => {
53-
const flag = new RegionAllowlistFeatureFlag('test-feature', new Set([AwsRegion.US_EAST_1]));
53+
const flag = new RegionAllowlistFeatureFlag('test-feature', [AwsRegion.US_EAST_1]);
5454
expect(flag.isEnabled('us-east-1')).toBe(true);
5555
});
5656

5757
it('should disable for non-allowlisted region', () => {
58-
const flag = new RegionAllowlistFeatureFlag('test-feature', new Set([AwsRegion.US_EAST_1]));
58+
const flag = new RegionAllowlistFeatureFlag('test-feature', [AwsRegion.US_EAST_1]);
5959
expect(flag.isEnabled('us-west-2')).toBe(false);
6060
});
6161

6262
it('should handle multiple allowlisted regions', () => {
63-
const flag = new RegionAllowlistFeatureFlag(
64-
'test-feature',
65-
new Set([AwsRegion.US_EAST_1, AwsRegion.EU_WEST_1]),
66-
);
63+
const flag = new RegionAllowlistFeatureFlag('test-feature', [AwsRegion.US_EAST_1, AwsRegion.EU_WEST_1]);
6764
expect(flag.isEnabled('us-east-1')).toBe(true);
6865
expect(flag.isEnabled('eu-west-1')).toBe(true);
6966
expect(flag.isEnabled('ap-south-1')).toBe(false);
7067
});
7168

7269
it('should return false for invalid region', () => {
73-
const flag = new RegionAllowlistFeatureFlag('test-feature', new Set([AwsRegion.US_EAST_1]));
70+
const flag = new RegionAllowlistFeatureFlag('test-feature', [AwsRegion.US_EAST_1]);
7471
expect(flag.isEnabled('invalid-region')).toBe(false);
7572
});
7673

7774
it('should describe itself correctly', () => {
78-
const flag = new RegionAllowlistFeatureFlag('my-feature', new Set([AwsRegion.US_EAST_1, AwsRegion.EU_WEST_1]));
75+
const flag = new RegionAllowlistFeatureFlag('my-feature', [AwsRegion.US_EAST_1, AwsRegion.EU_WEST_1]);
7976
const description = flag.describe();
8077
expect(description).toContain('RegionAllowlistFeatureFlag');
8178
expect(description).toContain('my-feature');

0 commit comments

Comments
 (0)