From dcdf84d3a41372c98e74acaa07cb4df8a39c6c63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Mon, 6 May 2024 21:27:54 +0200 Subject: [PATCH 1/3] feat: make inital metrics interval configurable. --- README.md | 1 + src/index.ts | 3 +++ src/metrics.test.ts | 54 +++++++++++++++++++++++++++++++++++++++++++++ src/metrics.ts | 16 +++++++++++--- 4 files changed, 71 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index e3a8b5d..0a6d1da 100644 --- a/README.md +++ b/README.md @@ -123,6 +123,7 @@ The Unleash SDK takes the following options: | refreshInterval | no | `30` | How often, in seconds, the SDK should check for updated toggle configuration. If set to 0 will disable checking for updates | | disableRefresh | no | `false` | If set to true, the client will not check for updated toggle configuration | | metricsInterval | no | `60` | How often, in seconds, the SDK should send usage metrics back to Unleash Proxy | +| metricsIntervalInitial | no | `2` | How long the SDK should wait for the first metrics report back to the Unleash API. It cannot be longer than the `metricsInterval`. If you want to disable the initial metrics call you can set it to 0. | | disableMetrics | no | `false` | Set this option to `true` if you want to disable usage metrics | | storageProvider | no | `LocalStorageProvider` in browser, `InMemoryStorageProvider` otherwise | Allows you to inject a custom storeProvider | | fetch | no | `window.fetch` or global `fetch` | Allows you to override the fetch implementation to use. Useful in Node.js environments where you can inject `node-fetch` | diff --git a/src/index.ts b/src/index.ts index 0bac3a4..0616e8a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -41,6 +41,7 @@ interface IConfig extends IStaticContext { disableRefresh?: boolean; refreshInterval?: number; metricsInterval?: number; + metricsIntervalInitial?: number; disableMetrics?: boolean; storageProvider?: IStorageProvider; context?: IMutableContext; @@ -153,6 +154,7 @@ export class UnleashClient extends TinyEmitter { disableRefresh = false, refreshInterval = 30, metricsInterval = 30, + metricsIntervalInitial = 2, disableMetrics = false, appName, environment = 'default', @@ -232,6 +234,7 @@ export class UnleashClient extends TinyEmitter { fetch, headerName, customHeaders, + metricsIntervalInitial, }); } diff --git a/src/metrics.test.ts b/src/metrics.test.ts index 56806ee..0eced24 100644 --- a/src/metrics.test.ts +++ b/src/metrics.test.ts @@ -21,6 +21,7 @@ test('should be disabled by flag disableMetrics', async () => { clientKey: '123', fetch: fetchMock, headerName: 'Authorization', + metricsIntervalInitial: 0, }); metrics.count('foo', true); @@ -40,6 +41,7 @@ test('should send metrics', async () => { clientKey: '123', fetch: fetchMock, headerName: 'Authorization', + metricsIntervalInitial: 0, }); metrics.count('foo', true); @@ -76,6 +78,7 @@ test('should send metrics with custom auth header', async () => { clientKey: '123', fetch: fetchMock, headerName: 'NotAuthorization', + metricsIntervalInitial: 0, }); metrics.count('foo', true); @@ -99,6 +102,7 @@ test('Should send initial metrics after 2 seconds', () => { clientKey: '123', fetch: fetchMock, headerName: 'Authorization', + metricsIntervalInitial: 2, }); metrics.start(); @@ -112,6 +116,54 @@ test('Should send initial metrics after 2 seconds', () => { expect(fetchMock.mock.calls.length).toEqual(1); }); +test('Should send initial metrics after 5 seconds, when metricsIntervalInitial is higher than metricsInterval', () => { + const metrics = new Metrics({ + onError: console.error, + appName: 'test', + metricsInterval: 5, + disableMetrics: false, + url: 'http://localhost:3000', + clientKey: '123', + fetch: fetchMock, + headerName: 'Authorization', + metricsIntervalInitial: 20, + }); + + metrics.start(); + + metrics.count('foo', true); + metrics.count('foo', true); + metrics.count('foo', false); + metrics.count('bar', false); + // Account for 2 second timeout before the set interval starts + jest.advanceTimersByTime(5000); + expect(fetchMock.mock.calls.length).toEqual(1); +}); + +test('Should not send initial metrics if disabled', () => { + const metrics = new Metrics({ + onError: console.error, + appName: 'test', + metricsInterval: 5, + disableMetrics: false, + url: 'http://localhost:3000', + clientKey: '123', + fetch: fetchMock, + headerName: 'Authorization', + metricsIntervalInitial: 0, + }); + + metrics.start(); + + metrics.count('foo', true); + metrics.count('foo', true); + metrics.count('foo', false); + metrics.count('bar', false); + // Account for 2 second timeout before the set interval starts + jest.advanceTimersByTime(2000); + expect(fetchMock.mock.calls.length).toEqual(0); +}); + test('should send metrics based on timer interval', async () => { const metrics = new Metrics({ onError: console.error, @@ -122,6 +174,7 @@ test('should send metrics based on timer interval', async () => { clientKey: '123', fetch: fetchMock, headerName: 'Authorization', + metricsIntervalInitial: 2, }); metrics.start(); @@ -162,6 +215,7 @@ describe('Custom headers for metrics', () => { fetch: fetchMock, headerName: 'Authorization', customHeaders, + metricsIntervalInitial: 2, }); metrics.count('foo', true); diff --git a/src/metrics.ts b/src/metrics.ts index 7ef67d2..29b89c4 100644 --- a/src/metrics.ts +++ b/src/metrics.ts @@ -13,6 +13,7 @@ export interface MetricsOptions { fetch: any; headerName: string; customHeaders?: Record; + metricsIntervalInitial: number; } interface VariantBucket { @@ -51,6 +52,7 @@ export default class Metrics { private fetch: any; private headerName: string; private customHeaders: Record; + private metricsIntervalInitial: number; constructor({ onError, @@ -63,11 +65,13 @@ export default class Metrics { fetch, headerName, customHeaders = {}, + metricsIntervalInitial, }: MetricsOptions) { this.onError = onError; this.onSent = onSent || doNothing; this.disabled = disableMetrics; this.metricsInterval = metricsInterval * 1000; + this.metricsIntervalInitial = Math.min(metricsInterval, metricsIntervalInitial) * 1000; this.appName = appName; this.url = url instanceof URL ? url : new URL(url); this.clientKey = clientKey; @@ -75,6 +79,7 @@ export default class Metrics { this.fetch = fetch; this.headerName = headerName; this.customHeaders = customHeaders; + } public start() { @@ -87,10 +92,15 @@ export default class Metrics { this.metricsInterval > 0 ) { // send first metrics after two seconds. - setTimeout(() => { + if(this.metricsIntervalInitial > 0) { + setTimeout(() => { + this.startTimer(); + this.sendMetrics(); + }, this.metricsIntervalInitial); + } else { this.startTimer(); - this.sendMetrics(); - }, 2000); + } + } } From 262ef34c3df51a242122d0e793e057d6c9fe07ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Tue, 7 May 2024 15:02:46 +0200 Subject: [PATCH 2/3] Update src/metrics.ts Co-authored-by: Simon Hornby --- src/metrics.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/metrics.ts b/src/metrics.ts index 29b89c4..700db2a 100644 --- a/src/metrics.ts +++ b/src/metrics.ts @@ -91,7 +91,6 @@ export default class Metrics { typeof this.metricsInterval === 'number' && this.metricsInterval > 0 ) { - // send first metrics after two seconds. if(this.metricsIntervalInitial > 0) { setTimeout(() => { this.startTimer(); From f08c2a487ad1f30cfaadd8edfdb6f61e184cb1b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivar=20Conradi=20=C3=98sthus?= Date: Tue, 7 May 2024 21:56:59 +0200 Subject: [PATCH 3/3] fix: allow any metricsIntervalInitial --- README.md | 4 ++-- src/metrics.test.ts | 31 +++++++++++++++++++++++++++++-- src/metrics.ts | 6 ++---- 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 0a6d1da..2c19777 100644 --- a/README.md +++ b/README.md @@ -122,8 +122,8 @@ The Unleash SDK takes the following options: | appName | yes | n/a | The name of the application using this SDK. Will be used as part of the metrics sent to Unleash Proxy. Will also be part of the Unleash Context. | | refreshInterval | no | `30` | How often, in seconds, the SDK should check for updated toggle configuration. If set to 0 will disable checking for updates | | disableRefresh | no | `false` | If set to true, the client will not check for updated toggle configuration | -| metricsInterval | no | `60` | How often, in seconds, the SDK should send usage metrics back to Unleash Proxy | -| metricsIntervalInitial | no | `2` | How long the SDK should wait for the first metrics report back to the Unleash API. It cannot be longer than the `metricsInterval`. If you want to disable the initial metrics call you can set it to 0. | +| metricsInterval | no | `60` | How often, in seconds, the SDK should send usage metrics back to Unleash Proxy. It will be started after the initial metrics report, which is sent after the configured `metricsIntervalInitial` | +| metricsIntervalInitial | no | `2` | How long the SDK should wait for the first metrics report back to the Unleash API. If you want to disable the initial metrics call you can set it to 0. | | disableMetrics | no | `false` | Set this option to `true` if you want to disable usage metrics | | storageProvider | no | `LocalStorageProvider` in browser, `InMemoryStorageProvider` otherwise | Allows you to inject a custom storeProvider | | fetch | no | `window.fetch` or global `fetch` | Allows you to override the fetch implementation to use. Useful in Node.js environments where you can inject `node-fetch` | diff --git a/src/metrics.test.ts b/src/metrics.test.ts index 0eced24..d460c73 100644 --- a/src/metrics.test.ts +++ b/src/metrics.test.ts @@ -116,7 +116,7 @@ test('Should send initial metrics after 2 seconds', () => { expect(fetchMock.mock.calls.length).toEqual(1); }); -test('Should send initial metrics after 5 seconds, when metricsIntervalInitial is higher than metricsInterval', () => { +test('Should send initial metrics after 20 seconds, when metricsIntervalInitial is higher than metricsInterval', () => { const metrics = new Metrics({ onError: console.error, appName: 'test', @@ -131,13 +131,40 @@ test('Should send initial metrics after 5 seconds, when metricsIntervalInitial i metrics.start(); + metrics.count('foo', true); + metrics.count('foo', true); + metrics.count('foo', false); + metrics.count('bar', false); + // Account for 20 second timeout before the set interval starts + jest.advanceTimersByTime(20000); + expect(fetchMock.mock.calls.length).toEqual(1); +}); + +test('Should send metrics for initial and after metrics interval', () => { + const metrics = new Metrics({ + onError: console.error, + appName: 'test', + metricsInterval: 5, + disableMetrics: false, + url: 'http://localhost:3000', + clientKey: '123', + fetch: fetchMock, + headerName: 'Authorization', + metricsIntervalInitial: 2, + }); + + metrics.start(); + metrics.count('foo', true); metrics.count('foo', true); metrics.count('foo', false); metrics.count('bar', false); // Account for 2 second timeout before the set interval starts + jest.advanceTimersByTime(2000); + metrics.count('foo', false); + metrics.count('bar', false); jest.advanceTimersByTime(5000); - expect(fetchMock.mock.calls.length).toEqual(1); + expect(fetchMock.mock.calls.length).toEqual(2); }); test('Should not send initial metrics if disabled', () => { diff --git a/src/metrics.ts b/src/metrics.ts index 700db2a..77abcbe 100644 --- a/src/metrics.ts +++ b/src/metrics.ts @@ -71,7 +71,7 @@ export default class Metrics { this.onSent = onSent || doNothing; this.disabled = disableMetrics; this.metricsInterval = metricsInterval * 1000; - this.metricsIntervalInitial = Math.min(metricsInterval, metricsIntervalInitial) * 1000; + this.metricsIntervalInitial = metricsIntervalInitial * 1000; this.appName = appName; this.url = url instanceof URL ? url : new URL(url); this.clientKey = clientKey; @@ -79,7 +79,6 @@ export default class Metrics { this.fetch = fetch; this.headerName = headerName; this.customHeaders = customHeaders; - } public start() { @@ -91,7 +90,7 @@ export default class Metrics { typeof this.metricsInterval === 'number' && this.metricsInterval > 0 ) { - if(this.metricsIntervalInitial > 0) { + if (this.metricsIntervalInitial > 0) { setTimeout(() => { this.startTimer(); this.sendMetrics(); @@ -99,7 +98,6 @@ export default class Metrics { } else { this.startTimer(); } - } }