Telemetry polishing

.eslintrc

@@ -37,7 +37,8 @@
     "no-unused-vars": "off",
     "@typescript-eslint/no-unused-vars": "error",
     "keyword-spacing": "error",
-    "comma-style": "error"
+    "comma-style": "error",
+    "no-trailing-spaces": "error"
   },
 
   "overrides": [

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@splitsoftware/splitio-commons",
-  "version": "1.6.2-rc.10",
+  "version": "1.6.2-rc.13",
   "description": "Split Javascript SDK common components",
   "main": "cjs/index.js",
   "module": "esm/index.js",
@@ -24,6 +24,7 @@
     "build:cjs": "rimraf cjs && tsc -m CommonJS --outDir cjs",
     "test": "jest",
     "test:coverage": "jest --coverage",
+    "all": "npm run check && npm run build && npm run test",
     "publish:rc": "npm run check && npm run test && npm run build && npm publish --tag rc",
     "publish:stable": "npm run check && npm run test && npm run build && npm publish"
   },

src/listeners/__tests__/browser.spec.ts

@@ -1,20 +1,8 @@
 import { BrowserSignalListener } from '../browser';
-import { IEventsCacheSync, IImpressionCountsCacheSync, IImpressionsCacheSync, IStorageSync } from '../../storages/types';
+import { IEventsCacheSync, IImpressionCountsCacheSync, IImpressionsCacheSync, IStorageSync, ITelemetryCacheSync, IUniqueKeysCacheBase } from '../../storages/types';
 import { ISplitApi } from '../../services/types';
 import { fullSettings } from '../../utils/settingsValidation/__tests__/settings.mocks';
 
-jest.mock('../../sync/submitters/telemetrySubmitter', () => {
-  return {
-    telemetryCacheStatsAdapter: () => {
-      return {
-        isEmpty: () => false,
-        clear: () => { },
-        pop: () => ({}),
-      };
-    }
-  };
-});
-
 /* Mocks start */
 
 const fakeImpression = {
@@ -37,31 +25,42 @@ const fakeEvent = {
 const fakeImpressionCounts = {
   'someFeature::0': 1
 };
+const fakeUniqueKeys = {
+  keys: [{ k: 'emi', fs: ['split1'] }],
+};
 
 // Storage with impressionsCount and telemetry cache
 const fakeStorageOptimized = { // @ts-expect-error
   impressions: {
     isEmpty: jest.fn(),
-    clear: jest.fn(),
     pop() {
       return [fakeImpression];
     }
   } as IImpressionsCacheSync, // @ts-expect-error
   events: {
     isEmpty: jest.fn(),
-    clear: jest.fn(),
     pop() {
       return [fakeEvent];
     }
   } as IEventsCacheSync, // @ts-expect-error
   impressionCounts: {
     isEmpty: jest.fn(),
-    clear: jest.fn(),
     pop() {
       return fakeImpressionCounts;
     }
-  } as IImpressionCountsCacheSync,
-  telemetry: {}
+  } as IImpressionCountsCacheSync, // @ts-expect-error
+  uniqueKeys: {
+    isEmpty: jest.fn(),
+    pop() {
+      return fakeUniqueKeys;
+    }
+  } as IUniqueKeysCacheBase, // @ts-expect-error
+  telemetry: {
+    isEmpty: jest.fn(),
+    pop() {
+      return 'fake telemetry';
+    }
+  } as ITelemetryCacheSync
 };
 
 const fakeStorageDebug = {
@@ -75,6 +74,7 @@ const fakeSplitApi = {
   postEventsBulk: jest.fn(() => Promise.resolve()),
   postTestImpressionsCount: jest.fn(() => Promise.resolve()),
   postMetricsUsage: jest.fn(() => Promise.resolve()),
+  postUniqueKeysBulkCs: jest.fn(() => Promise.resolve()),
 } as ISplitApi;
 
 const VISIBILITYCHANGE_EVENT = 'visibilitychange';
@@ -190,8 +190,8 @@ test('Browser JS listener / standalone mode / Impressions optimized mode with te
 
   triggerEvent(VISIBILITYCHANGE_EVENT, 'hidden');
 
-  // Visibility change event was triggered. Thus sendBeacon method should be called four times.
-  expect(global.window.navigator.sendBeacon).toBeCalledTimes(4);
+  // Visibility change event was triggered. Thus sendBeacon method should be called five times (events, impressions, impression count, unique keys and telemetry).
+  expect(global.window.navigator.sendBeacon).toBeCalledTimes(5);
 
   // Http post services should have not been called
   expect(fakeSplitApi.postTestImpressionsBulk).not.toBeCalled();
@@ -297,8 +297,8 @@ test('Browser JS listener / standalone mode / user consent status', () => {
   settings.userConsent = undefined;
   triggerEvent(VISIBILITYCHANGE_EVENT, 'hidden');
 
-  // 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);
+  // Unload event was triggered when user consent was granted and undefined. Thus sendBeacon should be called 10 times (5 times per event).
+  expect(global.window.navigator.sendBeacon).toBeCalledTimes(10);
 
   listener.stop();
 });

src/listeners/browser.ts

@@ -1,7 +1,7 @@
 /* eslint-disable no-undef */
 // @TODO eventually migrate to JS-Browser-SDK package.
 import { ISignalListener } from './types';
-import { IRecorderCacheProducerSync, IStorageSync } from '../storages/types';
+import { IRecorderCacheSync, IStorageSync } from '../storages/types';
 import { fromImpressionsCollector } from '../sync/submitters/impressionsSubmitter';
 import { fromImpressionCountsCollector } from '../sync/submitters/impressionCountsSubmitter';
 import { IResponse, ISplitApi } from '../services/types';
@@ -12,7 +12,6 @@ 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';
 
 const VISIBILITYCHANGE_EVENT = 'visibilitychange';
 const PAGEHIDE_EVENT = 'pagehide';
@@ -84,42 +83,39 @@ export class BrowserSignalListener implements ISignalListener {
    */
   flushData() {
     if (!this.syncManager) return; // In consumer mode there is not sync manager and data to flush
+    const { events, telemetry } = this.settings.urls;
 
     // Flush impressions & events data if there is user consent
     if (isConsentGranted(this.settings)) {
-      const eventsUrl = this.settings.urls.events;
       const sim = this.settings.sync.impressionsMode;
       const extraMetadata = {
         // sim stands for Sync/Split Impressions Mode
         sim: sim === OPTIMIZED ? OPTIMIZED : sim === DEBUG ? DEBUG : NONE
       };
 
-      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);
+      this._flushData(events + '/testImpressions/beacon', this.storage.impressions, this.serviceApi.postTestImpressionsBulk, this.fromImpressionsCollector, extraMetadata);
+      this._flushData(events + '/events/beacon', this.storage.events, this.serviceApi.postEventsBulk);
+      if (this.storage.impressionCounts) this._flushData(events + '/testImpressions/count/beacon', this.storage.impressionCounts, this.serviceApi.postTestImpressionsCount, fromImpressionCountsCollector);
+      // @ts-ignore
+      if (this.storage.uniqueKeys) this._flushData(telemetry + '/v1/keys/cs/beacon', this.storage.uniqueKeys, this.serviceApi.postUniqueKeysBulkCs);
     }
 
     // Flush telemetry data
-    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);
-    }
+    if (this.storage.telemetry) this._flushData(telemetry + '/v1/metrics/usage/beacon', this.storage.telemetry, this.serviceApi.postMetricsUsage);
   }
 
   flushDataIfHidden() {
     // Precondition: document defined
     if (document.visibilityState === 'hidden') this.flushData(); // On a 'visibilitychange' event, flush data if state is hidden
   }
 
-  private _flushData<T>(url: string, cache: IRecorderCacheProducerSync<T>, postService: (body: string) => Promise<IResponse>, fromCacheToPayload?: (cacheData: T) => any, extraMetadata?: {}) {
+  private _flushData<T>(url: string, cache: IRecorderCacheSync<T>, postService: (body: string) => Promise<IResponse>, fromCacheToPayload?: (cacheData: T) => any, extraMetadata?: {}) {
     // if there is data in cache, send it to backend
     if (!cache.isEmpty()) {
       const dataPayload = fromCacheToPayload ? fromCacheToPayload(cache.pop()) : cache.pop();
       if (!this._sendBeacon(url, dataPayload, extraMetadata)) {
         postService(JSON.stringify(dataPayload)).catch(() => { }); // no-op just to catch a possible exception
       }
-      cache.clear();
     }
   }
 

src/sdkClient/sdkClient.ts

@@ -39,7 +39,7 @@ export function sdkClientFactory(params: ISdkFactoryContext, isSharedClient?: bo
 
           // Release the API Key if it is the main client
           if (!isSharedClient) releaseApiKey(settings.core.authorizationKey);
-          
+
           if (uniqueKeysTracker) uniqueKeysTracker.stop();
 
           // Cleanup storage

src/sdkFactory/index.ts

@@ -3,14 +3,10 @@ import { sdkReadinessManagerFactory } from '../readiness/sdkReadinessManager';
 import { impressionsTrackerFactory } from '../trackers/impressionsTracker';
 import { eventTrackerFactory } from '../trackers/eventTracker';
 import { telemetryTrackerFactory } from '../trackers/telemetryTracker';
-import { IStorageFactoryParams } from '../storages/types';
 import { SplitIO } from '../types';
-import { getMatching } from '../utils/key';
-import { shouldBeOptimized } from '../trackers/impressionObserver/utils';
 import { validateAndTrackApiKey } from '../utils/inputValidation/apiKey';
 import { createLoggerAPI } from '../logger/sdkLogger';
 import { NEW_FACTORY, RETRIEVE_MANAGER } from '../logger/constants';
-import { metadataBuilder } from '../storages/metadataBuilder';
 import { SDK_SPLITS_ARRIVED, SDK_SEGMENTS_ARRIVED } from '../readiness/constants';
 import { objectAssign } from '../utils/lang/objectAssign';
 import { strategyDebugFactory } from '../trackers/strategy/strategyDebug';
@@ -28,7 +24,7 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO.
     syncManagerFactory, SignalListener, impressionsObserverFactory,
     integrationsManagerFactory, sdkManagerFactory, sdkClientMethodFactory,
     filterAdapterFactory } = params;
-  const log = settings.log;
+  const { log, sync: { impressionsMode } } = settings;
 
   // @TODO handle non-recoverable errors, such as, global `fetch` not available, invalid API Key, etc.
   // On non-recoverable errors, we should mark the SDK as destroyed and not start synchronization.
@@ -39,42 +35,24 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO.
   const sdkReadinessManager = sdkReadinessManagerFactory(log, platform.EventEmitter, settings.startup.readyTimeout);
   const readiness = sdkReadinessManager.readinessManager;
 
-  // @TODO consider passing the settings object, so that each storage access only what it needs
-  const storageFactoryParams: IStorageFactoryParams = {
-    impressionsQueueSize: settings.scheduler.impressionsQueueSize,
-    eventsQueueSize: settings.scheduler.eventsQueueSize,
-    optimize: shouldBeOptimized(settings),
-
-    // ATM, only used by InLocalStorage
-    matchingKey: getMatching(settings.core.key),
-    splitFiltersValidation: settings.sync.__splitFiltersValidation,
-
-    // ATM, only used by PluggableStorage
-    mode: settings.mode,
-    impressionsMode: settings.sync.impressionsMode,
-
-    // Callback used to emit SDK_READY in consumer mode, where `syncManagerFactory` is undefined,
-    // or partial consumer mode, where it only has submitters, and therefore it doesn't emit readiness events.
+  const storage = storageFactory({
+    settings,
     onReadyCb: (error) => {
       if (error) return; // Don't emit SDK_READY if storage failed to connect. Error message is logged by wrapperAdapter
       readiness.splits.emit(SDK_SPLITS_ARRIVED);
       readiness.segments.emit(SDK_SEGMENTS_ARRIVED);
     },
-    metadata: metadataBuilder(settings),
-    log
-  };
-
-  const storage = storageFactory(storageFactoryParams);
+  });
   // @TODO add support for dataloader: `if (params.dataLoader) params.dataLoader(storage);`
 
   const telemetryTracker = telemetryTrackerFactory(storage.telemetry, platform.now);
   const integrationsManager = integrationsManagerFactory && integrationsManagerFactory({ settings, storage, telemetryTracker });
 
   const observer = impressionsObserverFactory();
-  const uniqueKeysTracker = storageFactoryParams.impressionsMode === NONE ? uniqueKeysTrackerFactory(log, storage.uniqueKeys!, filterAdapterFactory && filterAdapterFactory()) : undefined;
+  const uniqueKeysTracker = impressionsMode === NONE ? uniqueKeysTrackerFactory(log, storage.uniqueKeys!, filterAdapterFactory && filterAdapterFactory()) : undefined;
 
   let strategy;
-  switch (storageFactoryParams.impressionsMode) {
+  switch (impressionsMode) {
     case OPTIMIZED:
       strategy = strategyOptimizedFactory(observer, storage.impressionCounts!);
       break;
@@ -120,7 +98,7 @@ export function sdkFactory(params: ISdkFactoryParams): SplitIO.ICsSDK | SplitIO.
     },
 
     // Logger wrapper API
-    Logger: createLoggerAPI(settings.log),
+    Logger: createLoggerAPI(log),
 
     settings,
   }, extraProps && extraProps(ctx));

src/sdkFactory/types.ts

@@ -97,9 +97,9 @@ export interface ISdkFactoryParams {
   // It Allows to distinguish SDK clients with the client-side API (`ICsSDK`) or server-side API (`ISDK` or `IAsyncSDK`).
   sdkClientMethodFactory: (params: ISdkFactoryContext) => ({ (): SplitIO.ICsClient; (key: SplitIO.SplitKey, trafficType?: string | undefined): SplitIO.ICsClient; } | (() => SplitIO.IClient) | (() => SplitIO.IAsyncClient))
 
-  // Impression observer factory. If provided, will be used for impressions dedupe
+  // Impression observer factory.
   impressionsObserverFactory: () => IImpressionObserver
-  
+
   filterAdapterFactory?: () => IFilterAdapter
 
   // Optional signal listener constructor. Used to handle special app states, like shutdown, app paused or resumed.

src/services/splitApi.ts

@@ -106,7 +106,7 @@ export function splitApiFactory(
       const url = `${urls.events}/testImpressions/count`;
       return splitHttpClient(url, { method: 'POST', body, headers }, telemetryTracker.trackHttp(IMPRESSIONS_COUNT));
     },
-    
+
     /**
      * Post unique keys for client side.
      *
@@ -117,7 +117,7 @@ export function splitApiFactory(
       const url = `${urls.telemetry}/v1/keys/cs`;
       return splitHttpClient(url, { method: 'POST', body, headers }, telemetryTracker.trackHttp(TELEMETRY));
     },
-    
+
     /**
      * Post unique keys for server side.
      *

src/storages/KeyBuilderSS.ts

@@ -1,9 +1,8 @@
 import { KeyBuilder } from './KeyBuilder';
 import { IMetadata } from '../dtos/types';
 import { Method } from '../sync/submitters/types';
-import { MAX_LATENCY_BUCKET_COUNT } from './inMemory/TelemetryCacheInMemory';
 
-const METHOD_NAMES: Record<Method, string> = {
+export const METHOD_NAMES: Record<Method, string> = {
   t: 'treatment',
   ts: 'treatments',
   tc: 'treatmentWithConfig',
@@ -37,7 +36,7 @@ export class KeyBuilderSS extends KeyBuilder {
   buildImpressionsCountKey() {
     return `${this.prefix}.impressions.count`;
   }
-  
+
   buildUniqueKeysKey() {
     return `${this.prefix}.uniquekeys`;
   }
@@ -65,44 +64,3 @@ export class KeyBuilderSS extends KeyBuilder {
   }
 
 }
-
-// Used by consumer methods of TelemetryCacheInRedis and TelemetryCachePluggable
-
-const REVERSE_METHOD_NAMES = Object.keys(METHOD_NAMES).reduce((acc, key) => {
-  acc[METHOD_NAMES[key as Method]] = key as Method;
-  return acc;
-}, {} as Record<string, Method>);
-
-
-export function parseMetadata(field: string): [metadata: string] | string {
-  const parts = field.split('/');
-  if (parts.length !== 3) return `invalid subsection count. Expected 3, got: ${parts.length}`;
-
-  const [s /* metadata.s */, n /* metadata.n */, i /* metadata.i */] = parts;
-  return [JSON.stringify({ s, n, i })];
-}
-
-export function parseExceptionField(field: string): [metadata: string, method: Method] | string {
-  const parts = field.split('/');
-  if (parts.length !== 4) return `invalid subsection count. Expected 4, got: ${parts.length}`;
-
-  const [s /* metadata.s */, n /* metadata.n */, i /* metadata.i */, m] = parts;
-  const method = REVERSE_METHOD_NAMES[m];
-  if (!method) return `unknown method '${m}'`;
-
-  return [JSON.stringify({ s, n, i }), method];
-}
-
-export function parseLatencyField(field: string): [metadata: string, method: Method, bucket: number] | string {
-  const parts = field.split('/');
-  if (parts.length !== 5) return `invalid subsection count. Expected 5, got: ${parts.length}`;
-
-  const [s /* metadata.s */, n /* metadata.n */, i /* metadata.i */, m, b] = parts;
-  const method = REVERSE_METHOD_NAMES[m];
-  if (!method) return `unknown method '${m}'`;
-
-  const bucket = parseInt(b);
-  if (isNaN(bucket) || bucket >= MAX_LATENCY_BUCKET_COUNT) return `invalid bucket. Expected a number between 0 and ${MAX_LATENCY_BUCKET_COUNT - 1}, got: ${b}`;
-
-  return [JSON.stringify({ s, n, i }), method, bucket];
-}

src/storages/__tests__/findLatencyIndex.spec.ts

@@ -4,8 +4,11 @@ const latenciesInMsAndBuckets = [
   // First bucket is up to 0.5 ms
   [0, 0],
   [0.500, 0],
+  [1.000, 0],
+  [1.001, 1],
   [1.400, 1],
   [1.500, 1],
+  [1.501, 2],
   [8.000, 6],
   [11.39, 6],
   [11.392, 7],