Skip to content

Commit

Permalink
fix!: reconnect, missing and duped events, remove max reconnect (#660)
Browse files Browse the repository at this point in the history
Signed-off-by: Todd Baert <[email protected]>
  • Loading branch information
toddbaert authored Nov 21, 2023
1 parent ab754f5 commit 8489c2f
Show file tree
Hide file tree
Showing 21 changed files with 207 additions and 259 deletions.
12 changes: 10 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,21 @@ jobs:

services:
flagd:
image: ghcr.io/open-feature/flagd-testbed:v0.4.4
image: ghcr.io/open-feature/flagd-testbed:v0.4.6
ports:
- 8013:8013
flagd-unstable:
image: ghcr.io/open-feature/flagd-testbed-unstable:v0.4.6
ports:
- 8014:8013
sync:
image: ghcr.io/open-feature/sync-testbed:v0.4.4
image: ghcr.io/open-feature/sync-testbed:v0.4.6
ports:
- 9090:9090
sync-unstable:
image: ghcr.io/open-feature/sync-testbed-unstable:v0.4.6
ports:
- 9091:9090

steps:
- uses: actions/checkout@v4
Expand Down
25 changes: 12 additions & 13 deletions libs/providers/flagd/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,16 @@ Options can be defined in the constructor or as environment variables. Construct

### Available Configuration Options

| Option name | Environment variable name | Type | Default | Supported values |
|-----------------------|--------------------------------|---------|-----------|------------------|
| host | FLAGD_HOST | string | localhost | |
| port | FLAGD_PORT | number | 8013 | |
| tls | FLAGD_TLS | boolean | false | |
| socketPath | FLAGD_SOCKET_PATH | string | - | |
| resolverType | FLAGD_SOURCE_RESOLVER | string | rpc | rpc, in-process |
| selector | FLAGD_SOURCE_SELECTOR | string | - | |
| cache | FLAGD_CACHE | string | lru | lru,disabled |
| maxCacheSize | FLAGD_MAX_CACHE_SIZE | int | 1000 | |
| maxEventStreamRetries | FLAGD_MAX_EVENT_STREAM_RETRIES | int | 5 | |
| Option name | Environment variable name | Type | Default | Supported values |
| -------------------------------------- | ------------------------------ | ------- | --------- | ---------------- |
| host | FLAGD_HOST | string | localhost | |
| port | FLAGD_PORT | number | 8013 | |
| tls | FLAGD_TLS | boolean | false | |
| socketPath | FLAGD_SOCKET_PATH | string | - | |
| resolverType | FLAGD_SOURCE_RESOLVER | string | rpc | rpc, in-process |
| selector | FLAGD_SOURCE_SELECTOR | string | - | |
| cache | FLAGD_CACHE | string | lru | lru,disabled |
| maxCacheSize | FLAGD_MAX_CACHE_SIZE | int | 1000 | |

Below are examples of usage patterns.

Expand Down Expand Up @@ -80,7 +79,7 @@ In the above example, the provider expects a flag sync service implementation to
The flagd provider emits `PROVIDER_READY`, `PROVIDER_ERROR` and `PROVIDER_CONFIGURATION_CHANGED` events.

| SDK event | Originating action in flagd |
|----------------------------------|---------------------------------------------------------------------------------|
| -------------------------------- | ------------------------------------------------------------------------------- |
| `PROVIDER_READY` | The streaming connection with flagd has been established. |
| `PROVIDER_ERROR` | The streaming connection with flagd has been broken. |
| `PROVIDER_CONFIGURATION_CHANGED` | A flag configuration (default value, targeting rule, etc) in flagd has changed. |
Expand All @@ -90,7 +89,7 @@ For general information on events, see the [official documentation](https://open
### Flag Metadata

| Field | Type | Value |
|---------|--------|---------------------------------------------------|
| ------- | ------ | ------------------------------------------------- |
| `scope` | string | "selector" set for the associated source in flagd |

## Building
Expand Down
2 changes: 1 addition & 1 deletion libs/providers/flagd/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"@openfeature/flagd-core": ">=0.1.1"
},
"peerDependencies": {
"@grpc/grpc-js": "^1.6.0",
"@grpc/grpc-js": "~1.8.0",
"@openfeature/server-sdk": ">=1.6.0"
}
}
1 change: 1 addition & 0 deletions libs/providers/flagd/src/e2e/jest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ export default {
moduleNameMapper: {
'@openfeature/flagd-core': ['<rootDir>/../../../../shared/flagd-core/src'],
},
globalTeardown: './tear-down.ts',
};
11 changes: 7 additions & 4 deletions libs/providers/flagd/src/e2e/setup-in-process-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,13 @@ const FLAGD_NAME = 'flagd Provider';
// register the flagd provider before the tests.
console.log('Setting flagd provider...');
OpenFeature.setProvider(
'e2e',
new FlagdProvider({ cache: 'disabled', resolverType: 'in-process', host: 'localhost', port: 9090 }),
);
assert(
OpenFeature.providerMetadata.name === FLAGD_NAME,
new Error(`Expected ${FLAGD_NAME} provider to be configured, instead got: ${OpenFeature.providerMetadata.name}`),
);
OpenFeature.setProvider('unstable', new FlagdProvider({ resolverType: 'in-process', host: 'localhost', port: 9091 }));
// TODO: update with correct assertions once we have ability to get providerMetadata for any provider
// assert(
// OpenFeature.providerMetadata.name === FLAGD_NAME,
// new Error(`Expected ${FLAGD_NAME} provider to be configured, instead got: ${OpenFeature.providerMetadata.name}`),
// );
console.log('flagd provider configured!');
12 changes: 7 additions & 5 deletions libs/providers/flagd/src/e2e/setup-rpc-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ const FLAGD_NAME = 'flagd Provider';

// register the flagd provider before the tests.
console.log('Setting flagd provider...');
OpenFeature.setProvider(new FlagdProvider({ cache: 'disabled' }));
assert(
OpenFeature.providerMetadata.name === FLAGD_NAME,
new Error(`Expected ${FLAGD_NAME} provider to be configured, instead got: ${OpenFeature.providerMetadata.name}`),
);
OpenFeature.setProvider('e2e', new FlagdProvider({ cache: 'disabled' }));
OpenFeature.setProvider('unstable', new FlagdProvider({ cache: 'disabled', port: 8014 }));
// TODO: update with correct assertions once we have ability to get providerMetadata for any provider
// assert(
// OpenFeature.providerMetadata.name === FLAGD_NAME,
// new Error(`Expected ${FLAGD_NAME} provider to be configured, instead got: ${OpenFeature.providerMetadata.name}`),
// );
console.log('flagd provider configured!');
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { defineFeature, loadFeature } from 'jest-cucumber';
const feature = loadFeature('features/evaluation.feature');

// get a client (flagd provider registered in setup)
const client = OpenFeature.getClient();
const client = OpenFeature.getClient('e2e');

const givenAnOpenfeatureClientIsRegistered = (
given: (stepMatcher: string, stepDefinitionCallback: () => void) => void,
Expand All @@ -29,10 +29,6 @@ defineFeature(feature, (test) => {
});
});

afterAll(async () => {
await OpenFeature.close();
});

test('Resolves boolean value', ({ given, when, then }) => {
let value: boolean;
let flagKey: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { StepsDefinitionCallbackFunction } from 'jest-cucumber/dist/src/feature-
const feature = loadFeature('features/flagd-json-evaluator.feature');

// get a client (flagd provider registered in setup)
const client = OpenFeature.getClient();
const client = OpenFeature.getClient('e2e');

const aFlagProviderIsSet = (given: (stepMatcher: string, stepDefinitionCallback: () => void) => void) => {
given('a flagd provider is set', () => undefined);
Expand Down Expand Up @@ -39,10 +39,6 @@ defineFeature(feature, (test) => {
});
});

afterAll(async () => {
await OpenFeature.close();
});

test('Evaluator reuse', evaluateStringFlagWithContext);

test('Fractional operator', ({ given, when, and, then }) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { OpenFeature, ProviderEvents } from '@openfeature/server-sdk';
import { defineFeature, loadFeature } from 'jest-cucumber';

jest.setTimeout(30000);

// load the feature file.
const feature = loadFeature('features/flagd-reconnect.feature');

// get a client (flagd provider registered in setup)
const client = OpenFeature.getClient('unstable');

defineFeature(feature, (test) => {
let readyRunCount = 0;
let errorRunCount = 0;

beforeAll((done) => {
client.addHandler(ProviderEvents.Ready, async () => {
readyRunCount++;
done();
});
});

test('Provider reconnection', ({ given, when, then, and }) => {
given('a flagd provider is set', () => {
// handled in beforeAll
});
when('a PROVIDER_READY handler and a PROVIDER_ERROR handler are added', () => {
client.addHandler(ProviderEvents.Error, () => {
errorRunCount++;
});
});
then('the PROVIDER_READY handler must run when the provider connects', async () => {
// should already be at 1 from `beforeAll`
expect(readyRunCount).toEqual(1);
});
and("the PROVIDER_ERROR handler must run when the provider's connection is lost", async () => {
await new Promise((resolve) => setTimeout(resolve, 10000));
expect(errorRunCount).toBeGreaterThan(0);
});
and('when the connection is reestablished the PROVIDER_READY handler must run again', async () => {
await new Promise((resolve) => setTimeout(resolve, 10000));
expect(readyRunCount).toBeGreaterThan(1);
});
});
});
6 changes: 1 addition & 5 deletions libs/providers/flagd/src/e2e/step-definitions/flagd.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { defineFeature, loadFeature } from 'jest-cucumber';
const feature = loadFeature('features/flagd.feature');

// get a client (flagd provider registered in setup)
const client = OpenFeature.getClient();
const client = OpenFeature.getClient('e2e');

const aFlagProviderIsSet = (given: (stepMatcher: string, stepDefinitionCallback: () => void) => void) => {
given('a flagd provider is set', () => undefined);
Expand All @@ -18,10 +18,6 @@ defineFeature(feature, (test) => {
});
});

afterAll(async () => {
await OpenFeature.close();
});

test('Provider ready event', ({ given, when, then }) => {
let ran = false;

Expand Down
8 changes: 8 additions & 0 deletions libs/providers/flagd/src/e2e/tear-down.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { OpenFeature } from '@openfeature/server-sdk';

const tearDownTests = async () => {
console.log('Shutting down OpenFeature...');
await OpenFeature.close();
};

export default tearDownTests;
5 changes: 0 additions & 5 deletions libs/providers/flagd/src/lib/configuration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ describe('Configuration', () => {
port: 8013,
tls: false,
maxCacheSize: DEFAULT_MAX_CACHE_SIZE,
maxEventStreamRetries: DEFAULT_MAX_EVENT_STREAM_RETRIES,
cache: 'lru',
resolverType: 'rpc',
selector: '',
Expand All @@ -28,7 +27,6 @@ describe('Configuration', () => {
const tls = true;
const socketPath = '/tmp/flagd.socks';
const maxCacheSize = 333;
const maxEventStreamRetries = 10;
const cache = 'disabled';
const resolverType = 'in-process';
const selector = 'app=weather';
Expand All @@ -39,7 +37,6 @@ describe('Configuration', () => {
process.env['FLAGD_SOCKET_PATH'] = socketPath;
process.env['FLAGD_CACHE'] = cache;
process.env['FLAGD_MAX_CACHE_SIZE'] = `${maxCacheSize}`;
process.env['FLAGD_MAX_EVENT_STREAM_RETRIES'] = `${maxEventStreamRetries}`;
process.env['FLAGD_SOURCE_SELECTOR'] = `${selector}`;
process.env['FLAGD_RESOLVER'] = `${resolverType}`;

Expand All @@ -49,7 +46,6 @@ describe('Configuration', () => {
tls,
socketPath,
maxCacheSize,
maxEventStreamRetries,
cache,
resolverType,
selector,
Expand All @@ -62,7 +58,6 @@ describe('Configuration', () => {
port: 3000,
tls: true,
maxCacheSize: 1000,
maxEventStreamRetries: 5,
cache: 'lru',
resolverType: 'rpc',
selector: '',
Expand Down
12 changes: 0 additions & 12 deletions libs/providers/flagd/src/lib/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,6 @@ export interface Config {
* @default 1000
*/
maxCacheSize?: number;

/**
* Amount of times to attempt to reconnect to the event stream.
*
* @default 5
*/
maxEventStreamRetries?: number;
}

export type FlagdProviderOptions = Partial<Config>;
Expand All @@ -81,7 +74,6 @@ const DEFAULT_CONFIG: Config = {
selector: '',
cache: 'lru',
maxCacheSize: DEFAULT_MAX_CACHE_SIZE,
maxEventStreamRetries: DEFAULT_MAX_EVENT_STREAM_RETRIES,
};

enum ENV_VAR {
Expand All @@ -91,7 +83,6 @@ enum ENV_VAR {
FLAGD_SOCKET_PATH = 'FLAGD_SOCKET_PATH',
FLAGD_CACHE = 'FLAGD_CACHE',
FLAGD_MAX_CACHE_SIZE = 'FLAGD_MAX_CACHE_SIZE',
FLAGD_MAX_EVENT_STREAM_RETRIES = 'FLAGD_MAX_EVENT_STREAM_RETRIES',
FLAGD_SOURCE_SELECTOR = 'FLAGD_SOURCE_SELECTOR',
FLAGD_RESOLVER = 'FLAGD_RESOLVER',
}
Expand All @@ -115,9 +106,6 @@ const getEnvVarConfig = (): Partial<Config> => ({
...(process.env[ENV_VAR.FLAGD_MAX_CACHE_SIZE] && {
maxCacheSize: Number(process.env[ENV_VAR.FLAGD_MAX_CACHE_SIZE]),
}),
...(process.env[ENV_VAR.FLAGD_MAX_EVENT_STREAM_RETRIES] && {
maxEventStreamRetries: Number(process.env[ENV_VAR.FLAGD_MAX_EVENT_STREAM_RETRIES]),
}),
...(process.env[ENV_VAR.FLAGD_SOURCE_SELECTOR] && {
selector: process.env[ENV_VAR.FLAGD_SOURCE_SELECTOR],
}),
Expand Down
2 changes: 1 addition & 1 deletion libs/providers/flagd/src/lib/constants.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export const BASE_EVENT_STREAM_RETRY_BACKOFF_MS = 1000;
export const DEFAULT_MAX_EVENT_STREAM_RETRIES = 5;
export const DEFAULT_MAX_EVENT_STREAM_RETRIES = Infinity;
export const EVENT_CONFIGURATION_CHANGE = 'configuration_change';
export const EVENT_PROVIDER_READY = 'provider_ready';
export const DEFAULT_MAX_CACHE_SIZE = 1000;
Loading

0 comments on commit 8489c2f

Please sign in to comment.