From fc3cdc54da5da21b61cc40843c870f12e73b9a7c Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 5 May 2022 13:54:39 -0300 Subject: [PATCH 1/4] updated telemetry default URL. using DEBUG log level for telemetry related logs --- package-lock.json | 2 +- package.json | 2 +- src/services/splitApi.ts | 4 ++-- src/sync/submitters/submitter.ts | 8 ++++---- src/types.ts | 2 +- src/utils/settingsValidation/__tests__/index.spec.ts | 2 +- src/utils/settingsValidation/index.ts | 2 +- src/utils/settingsValidation/url.ts | 2 +- 8 files changed, 12 insertions(+), 12 deletions(-) diff --git a/package-lock.json b/package-lock.json index 73687c88..88509e4a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.3.2-rc.1", + "version": "1.3.2-rc.2", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index 987ecb67..b0fd02a6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.3.2-rc.1", + "version": "1.3.2-rc.2", "description": "Split Javascript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/src/services/splitApi.ts b/src/services/splitApi.ts index 37d2deab..b1b01bfb 100644 --- a/src/services/splitApi.ts +++ b/src/services/splitApi.ts @@ -108,12 +108,12 @@ export function splitApiFactory( }, postMetricsConfig(body: string) { - const url = `${urls.telemetry}/metrics/config`; + const url = `${urls.telemetry}/v1/metrics/config`; return splitHttpClient(url, { method: 'POST', body }, telemetryTracker.trackHttp(TELEMETRY), true); }, postMetricsUsage(body: string) { - const url = `${urls.telemetry}/metrics/usage`; + const url = `${urls.telemetry}/v1/metrics/usage`; return splitHttpClient(url, { method: 'POST', body }, telemetryTracker.trackHttp(TELEMETRY), true); } }; diff --git a/src/sync/submitters/submitter.ts b/src/sync/submitters/submitter.ts index 974bb8e8..a1312c7d 100644 --- a/src/sync/submitters/submitter.ts +++ b/src/sync/submitters/submitter.ts @@ -16,7 +16,7 @@ export function submitterFactory( dataName: string, fromCacheToPayload?: (cacheData: TState) => any, maxRetries: number = 0, - debugLogs?: boolean + debugLogs?: boolean // true for telemetry submitters ): ISyncTask<[], void> { let retries = 0; @@ -37,14 +37,14 @@ export function submitterFactory( sourceCache.clear(); // we clear the queue if request successes. }).catch(err => { if (!maxRetries) { - log.warn(SUBMITTERS_PUSH_FAILS, [dataCountMessage, err]); + log[debugLogs ? 'debug' : 'warn'](SUBMITTERS_PUSH_FAILS, [dataCountMessage, err]); } else if (retries === maxRetries) { retries = 0; sourceCache.clear(); // we clear the queue if request fails after retries. - log.warn(SUBMITTERS_PUSH_FAILS, [dataCountMessage, err]); + log[debugLogs ? 'debug' : 'warn'](SUBMITTERS_PUSH_FAILS, [dataCountMessage, err]); } else { retries++; - log.warn(SUBMITTERS_PUSH_RETRY, [dataCountMessage, err]); + log[debugLogs ? 'debug' : 'warn'](SUBMITTERS_PUSH_RETRY, [dataCountMessage, err]); } }); } diff --git a/src/types.ts b/src/types.ts index 4ea5ce31..dd2cb085 100644 --- a/src/types.ts +++ b/src/types.ts @@ -684,7 +684,7 @@ export namespace SplitIO { /** * String property to override the base URL where the SDK will post telemetry data. * @property {string} telemetry - * @default 'https://telemetry.split.io' + * @default 'https://telemetry.split.io/api' */ telemetry?: string }; diff --git a/src/utils/settingsValidation/__tests__/index.spec.ts b/src/utils/settingsValidation/__tests__/index.spec.ts index f2c2431b..5191d546 100644 --- a/src/utils/settingsValidation/__tests__/index.spec.ts +++ b/src/utils/settingsValidation/__tests__/index.spec.ts @@ -37,7 +37,7 @@ describe('settingsValidation', () => { events: 'https://events.split.io/api', auth: 'https://auth.split.io/api', streaming: 'https://streaming.split.io', - telemetry: 'https://telemetry.split.io', + telemetry: 'https://telemetry.split.io/api', }); expect(settings.sync.impressionsMode).toBe(OPTIMIZED); }); diff --git a/src/utils/settingsValidation/index.ts b/src/utils/settingsValidation/index.ts index 9a20a56b..faf860b3 100644 --- a/src/utils/settingsValidation/index.ts +++ b/src/utils/settingsValidation/index.ts @@ -58,7 +58,7 @@ export const base = { // Streaming Server streaming: 'https://streaming.split.io', // Telemetry Server - telemetry: 'https://telemetry.split.io', + telemetry: 'https://telemetry.split.io/api', }, // Defines which kind of storage we should instanciate. diff --git a/src/utils/settingsValidation/url.ts b/src/utils/settingsValidation/url.ts index 142d2dc4..4dd0179e 100644 --- a/src/utils/settingsValidation/url.ts +++ b/src/utils/settingsValidation/url.ts @@ -1,6 +1,6 @@ import { ISettings } from '../../types'; -const telemetryEndpointMatcher = /^\/metrics\/(config|usage)/; +const telemetryEndpointMatcher = /^\/v1\/metrics\/(config|usage)/; const eventsEndpointMatcher = /^\/(testImpressions|metrics|events)/; const authEndpointMatcher = /^\/v2\/auth/; const streamingEndpointMatcher = /^\/(sse|event-stream)/; From 1ee5ca95528c9cdd81b88a84d671157fd26bcc2c Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Thu, 5 May 2022 16:33:51 -0300 Subject: [PATCH 2/4] updated logic to handle internalReadyCbCount --- package-lock.json | 2 +- package.json | 2 +- src/readiness/__tests__/sdkReadinessManager.spec.ts | 3 ++- src/readiness/sdkReadinessManager.ts | 12 +++++++----- src/readiness/types.ts | 8 +++++++- .../submitters/__tests__/telemetrySubmitter.spec.ts | 5 ++--- src/sync/submitters/telemetrySubmitter.ts | 3 ++- 7 files changed, 22 insertions(+), 13 deletions(-) diff --git a/package-lock.json b/package-lock.json index 88509e4a..9dc16446 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.3.2-rc.2", + "version": "1.3.2-rc.3", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index b0fd02a6..b2d0a8c6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.3.2-rc.2", + "version": "1.3.2-rc.3", "description": "Split Javascript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/src/readiness/__tests__/sdkReadinessManager.spec.ts b/src/readiness/__tests__/sdkReadinessManager.spec.ts index b17ea51f..3c93b7a6 100644 --- a/src/readiness/__tests__/sdkReadinessManager.spec.ts +++ b/src/readiness/__tests__/sdkReadinessManager.spec.ts @@ -172,7 +172,8 @@ describe('SDK Readiness Manager - Event emitter', () => { test('The event callbacks should work as expected - SDK_READY emits with expected internal callbacks', () => { // the sdkReadinessManager expects more than one SDK_READY callback to not log the "No listeners" warning - const sdkReadinessManager = sdkReadinessManagerFactory(loggerMock, EventEmitterMock, undefined /* default readyTimeout */, 1 /* internalReadyCbCount */); + const sdkReadinessManager = sdkReadinessManagerFactory(loggerMock, EventEmitterMock); + sdkReadinessManager.incInternalReadyCbCount(); const gateMock = sdkReadinessManager.readinessManager.gate; // Get the callbacks diff --git a/src/readiness/sdkReadinessManager.ts b/src/readiness/sdkReadinessManager.ts index f50f78f9..c137e040 100644 --- a/src/readiness/sdkReadinessManager.ts +++ b/src/readiness/sdkReadinessManager.ts @@ -15,18 +15,16 @@ const REMOVE_LISTENER_EVENT = 'removeListener'; * It also updates logs related warnings and errors. * * @param readyTimeout time in millis to emit SDK_READY_TIME_OUT event - * @param internalReadyCbCount offset value of SDK_READY listeners that are added/removed internally - * by the SDK. It is required to properly log the warning 'No listeners for SDK Readiness detected' * @param readinessManager optional readinessManager to use. only used internally for `shared` method */ export function sdkReadinessManagerFactory( log: ILogger, EventEmitter: new () => IEventEmitter, readyTimeout = 0, - internalReadyCbCount = 0, readinessManager = readinessManagerFactory(EventEmitter, readyTimeout)): ISdkReadinessManager { /** Ready callback warning */ + let internalReadyCbCount = 0; let readyCbCount = 0; readinessManager.gate.on(REMOVE_LISTENER_EVENT, (event: any) => { if (event === SDK_READY) readyCbCount--; @@ -74,8 +72,12 @@ export function sdkReadinessManagerFactory( return { readinessManager, - shared(readyTimeout = 0, internalReadyCbCount = 0) { - return sdkReadinessManagerFactory(log, EventEmitter, readyTimeout, internalReadyCbCount, readinessManager.shared(readyTimeout)); + shared(readyTimeout = 0) { + return sdkReadinessManagerFactory(log, EventEmitter, readyTimeout, readinessManager.shared(readyTimeout)); + }, + + incInternalReadyCbCount() { + internalReadyCbCount++; }, sdkStatus: objectAssign( diff --git a/src/readiness/types.ts b/src/readiness/types.ts index 288a2717..93986f60 100644 --- a/src/readiness/types.ts +++ b/src/readiness/types.ts @@ -66,6 +66,12 @@ export interface ISdkReadinessManager { readinessManager: IReadinessManager sdkStatus: IStatusInterface + /** + * Increment internalReadyCbCount, an offset value of SDK_READY listeners that are added/removed internally + * by the SDK. It is required to properly log the warning 'No listeners for SDK Readiness detected' + */ + incInternalReadyCbCount(): void + /** for client-side */ - shared(readyTimeout?: number, internalReadyCbCount?: number): ISdkReadinessManager + shared(readyTimeout?: number): ISdkReadinessManager } diff --git a/src/sync/submitters/__tests__/telemetrySubmitter.spec.ts b/src/sync/submitters/__tests__/telemetrySubmitter.spec.ts index cbf65e06..530d4c5f 100644 --- a/src/sync/submitters/__tests__/telemetrySubmitter.spec.ts +++ b/src/sync/submitters/__tests__/telemetrySubmitter.spec.ts @@ -14,9 +14,8 @@ describe('Telemetry submitter', () => { settings: { ...fullSettings, scheduler: { ...fullSettings.scheduler, telemetryRefreshRate } }, splitApi: { postMetricsUsage, postMetricsConfig }, // @ts-ignore storage: InMemoryStorageFactory({}), - platform: { - now: () => 123 // by returning a fixed timestamp, all latencies are equal to 0 - }, + platform: { now: () => 123 }, // by returning a fixed timestamp, all latencies are equal to 0 + sdkReadinessManager: { incInternalReadyCbCount: jest.fn(), }, readiness: { gate: { once: jest.fn((e: string, cb: () => void) => { diff --git a/src/sync/submitters/telemetrySubmitter.ts b/src/sync/submitters/telemetrySubmitter.ts index 5d1ab125..8bcdd41f 100644 --- a/src/sync/submitters/telemetrySubmitter.ts +++ b/src/sync/submitters/telemetrySubmitter.ts @@ -117,7 +117,7 @@ export function telemetrySubmitterFactory(params: ISdkFactoryContextSync) { const { storage: { splits, segments, telemetry } } = params; if (!telemetry) return; // No submitter created if telemetry cache is not defined - const { settings, settings: { log, scheduler: { telemetryRefreshRate } }, splitApi, platform: { now }, readiness } = params; + const { settings, settings: { log, scheduler: { telemetryRefreshRate } }, splitApi, platform: { now }, readiness, sdkReadinessManager } = params; const startTime = timer(now || Date.now); const submitter = firstPushWindowDecorator( @@ -129,6 +129,7 @@ export function telemetrySubmitterFactory(params: ISdkFactoryContextSync) { telemetry.recordTimeUntilReadyFromCache(startTime()); }); + sdkReadinessManager.incInternalReadyCbCount(); readiness.gate.once(SDK_READY, () => { telemetry.recordTimeUntilReady(startTime()); From ca7a3781b92007096b6c9bd34f9c60fe95d19400 Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Mon, 9 May 2022 13:33:27 -0300 Subject: [PATCH 3/4] updated browser listener to post telemetry stats to beacon endpoint --- src/listeners/__tests__/browser.spec.ts | 29 +++++++++++++++++++------ src/listeners/browser.ts | 7 +++++- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/listeners/__tests__/browser.spec.ts b/src/listeners/__tests__/browser.spec.ts index 5ec91eff..4657d24e 100644 --- a/src/listeners/__tests__/browser.spec.ts +++ b/src/listeners/__tests__/browser.spec.ts @@ -3,6 +3,18 @@ import { IEventsCacheSync, IImpressionCountsCacheSync, IImpressionsCacheSync, IS import { ISplitApi } from '../../services/types'; import { fullSettings } from '../../utils/settingsValidation/__tests__/settings.mocks'; +jest.mock('../../sync/submitters/telemetrySubmitter', () => { + return { + telemetryCacheStatsAdapter: () => { + return { + isEmpty: () => false, + clear: () => { }, + state: () => ({}), + }; + } + }; +}); + /* Mocks start */ const fakeImpression = { @@ -26,6 +38,7 @@ const fakeImpressionCounts = { 'someFeature::0': 1 }; +// Storage with impressionsCount and telemetry cache const fakeStorageOptimized = { // @ts-expect-error impressions: { isEmpty: jest.fn(), @@ -48,6 +61,7 @@ const fakeStorageOptimized = { // @ts-expect-error return fakeImpressionCounts; } } as IImpressionCountsCacheSync, + telemetry: {} }; const fakeStorageDebug = { @@ -59,7 +73,8 @@ const fakeStorageDebug = { const fakeSplitApi = { postTestImpressionsBulk: jest.fn(() => Promise.resolve()), postEventsBulk: jest.fn(() => Promise.resolve()), - postTestImpressionsCount: jest.fn(() => Promise.resolve()) + postTestImpressionsCount: jest.fn(() => Promise.resolve()), + postMetricsUsage: jest.fn(() => Promise.resolve()), } as ISplitApi; const UNLOAD_DOM_EVENT = 'unload'; @@ -131,7 +146,7 @@ test('Browser JS listener / consumer mode', () => { expect((global.window.removeEventListener as jest.Mock).mock.calls).toEqual([[UNLOAD_DOM_EVENT, listener.flushData]]); }); -test('Browser JS listener / standalone mode / Impressions optimized mode', () => { +test('Browser JS listener / standalone mode / Impressions optimized mode with telemetry', () => { const syncManagerMock = {}; // @ts-expect-error @@ -144,8 +159,8 @@ test('Browser JS listener / standalone mode / Impressions optimized mode', () => triggerUnloadEvent(); - // Unload event was triggered. Thus sendBeacon method should have been called three times. - expect(global.window.navigator.sendBeacon).toBeCalledTimes(3); + // Unload event was triggered. Thus sendBeacon method should have been called four times. + expect(global.window.navigator.sendBeacon).toBeCalledTimes(4); // Http post services should have not been called expect(fakeSplitApi.postTestImpressionsBulk).not.toBeCalled(); @@ -195,7 +210,7 @@ test('Browser JS listener / standalone mode / Impressions debug mode', () => { }); test('Browser JS listener / standalone mode / Impressions debug mode without sendBeacon API', () => { - // remove sendBeacon API + // remove sendBeacon API temporally const sendBeacon = global.navigator.sendBeacon; // @ts-expect-error global.navigator.sendBeacon = undefined; const syncManagerMockWithoutPushManager = {}; @@ -254,8 +269,8 @@ test('Browser JS listener / standalone mode / user consent status', () => { settings.userConsent = undefined; triggerUnloadEvent(); - // Unload event was triggered when user consent was granted and undefined. Thus sendBeacon should be called 6 times (3 times per event in optimized mode). - expect(global.window.navigator.sendBeacon).toBeCalledTimes(6); + // Unload event was triggered when user consent was granted and undefined. Thus sendBeacon should be called 8 times (4 times per event in optimized mode with telemetry). + expect(global.window.navigator.sendBeacon).toBeCalledTimes(8); listener.stop(); }); diff --git a/src/listeners/browser.ts b/src/listeners/browser.ts index d4be3831..faf5c956 100644 --- a/src/listeners/browser.ts +++ b/src/listeners/browser.ts @@ -12,6 +12,7 @@ import { objectAssign } from '../utils/lang/objectAssign'; import { CLEANUP_REGISTERING, CLEANUP_DEREGISTERING } from '../logger/constants'; import { ISyncManager } from '../sync/types'; import { isConsentGranted } from '../consent'; +import { telemetryCacheStatsAdapter } from '../sync/submitters/telemetrySubmitter'; // 'unload' event is used instead of 'beforeunload', since 'unload' is not a cancelable event, so no other listeners can stop the event from occurring. const UNLOAD_DOM_EVENT = 'unload'; @@ -77,7 +78,11 @@ export class BrowserSignalListener implements ISignalListener { this._flushData(eventsUrl + '/testImpressions/beacon', this.storage.impressions, this.serviceApi.postTestImpressionsBulk, this.fromImpressionsCollector, extraMetadata); this._flushData(eventsUrl + '/events/beacon', this.storage.events, this.serviceApi.postEventsBulk); if (this.storage.impressionCounts) this._flushData(eventsUrl + '/testImpressions/count/beacon', this.storage.impressionCounts, this.serviceApi.postTestImpressionsCount, fromImpressionCountsCollector); - // No beacon endpoint for `/metrics/usage` + if (this.storage.telemetry) { + const telemetryUrl = this.settings.urls.telemetry; + const telemetryCacheAdapter = telemetryCacheStatsAdapter(this.storage.telemetry, this.storage.splits, this.storage.segments); + this._flushData(telemetryUrl + '/v1/metrics/usage/beacon', telemetryCacheAdapter, this.serviceApi.postMetricsUsage); + } } // Close streaming connection From 5b6f2eb14606f1eb64840cfe1192151ec0b8395d Mon Sep 17 00:00:00 2001 From: Emiliano Sanchez Date: Wed, 11 May 2022 17:35:26 -0300 Subject: [PATCH 4/4] rc --- package-lock.json | 2 +- package.json | 2 +- src/utils/murmur3/utfx.ts | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9dc16446..fcbf68a1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.3.2-rc.3", + "version": "1.3.2-rc.4", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/package.json b/package.json index b2d0a8c6..78c90af5 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@splitsoftware/splitio-commons", - "version": "1.3.2-rc.3", + "version": "1.3.2-rc.4", "description": "Split Javascript SDK common components", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/src/utils/murmur3/utfx.ts b/src/utils/murmur3/utfx.ts index 9bc44270..fd9125f4 100644 --- a/src/utils/murmur3/utfx.ts +++ b/src/utils/murmur3/utfx.ts @@ -8,8 +8,7 @@ */ export interface utfx { - encodeUTF16toUTF8(src: () => number | null, dst: (...args: number[]) => string | undefined): void, - + encodeUTF16toUTF8(src: () => number | null, dst: (...args: number[]) => string | undefined): void }