From a33228aa294945349fbb098152c22e6497217e0d Mon Sep 17 00:00:00 2001 From: Bartosz Grajdek Date: Tue, 27 Feb 2024 22:26:40 +0100 Subject: [PATCH 1/3] feat: remove metrics --- lib/Onyx.js | 56 +----- lib/PerformanceUtils.ts | 66 +++++++ lib/metrics/PerformanceUtils.js | 63 ------ lib/metrics/index.js | 10 - lib/metrics/index.native.js | 251 ------------------------ tests/unit/metricsTest.js | 332 -------------------------------- tests/unit/webMetricsTest.js | 19 -- 7 files changed, 68 insertions(+), 729 deletions(-) create mode 100644 lib/PerformanceUtils.ts delete mode 100644 lib/metrics/PerformanceUtils.js delete mode 100644 lib/metrics/index.js delete mode 100644 lib/metrics/index.native.js delete mode 100644 tests/unit/metricsTest.js delete mode 100644 tests/unit/webMetricsTest.js diff --git a/lib/Onyx.js b/lib/Onyx.js index d459bc546..d4ec29e4d 100644 --- a/lib/Onyx.js +++ b/lib/Onyx.js @@ -5,7 +5,7 @@ import * as Logger from './Logger'; import cache from './OnyxCache'; import * as Str from './Str'; import createDeferredTask from './createDeferredTask'; -import * as PerformanceUtils from './metrics/PerformanceUtils'; +import * as PerformanceUtils from './PerformanceUtils'; import Storage from './storage'; import utils from './utils'; import unstable_batchedUpdates from './batch'; @@ -1554,21 +1554,7 @@ function setMemoryOnlyKeys(keyList) { * }, * }); */ -function init({ - keys = {}, - initialKeyStates = {}, - safeEvictionKeys = [], - maxCachedKeysCount = 1000, - captureMetrics = false, - shouldSyncMultipleInstances = Boolean(global.localStorage), - debugSetState = false, -} = {}) { - if (captureMetrics) { - // The code here is only bundled and applied when the captureMetrics is set - // eslint-disable-next-line no-use-before-define - applyDecorators(); - } - +function init({keys = {}, initialKeyStates = {}, safeEvictionKeys = [], maxCachedKeysCount = 1000, shouldSyncMultipleInstances = Boolean(global.localStorage), debugSetState = false} = {}) { if (debugSetState) { PerformanceUtils.setShouldDebugSetState(true); } @@ -1630,42 +1616,4 @@ const Onyx = { hasPendingMergeForKey, }; -/** - * Apply calls statistic decorators to benchmark Onyx - * - * @private - */ -function applyDecorators() { - // We're requiring the script dynamically here so that it's only evaluated when decorators are used - const decorate = require('./metrics'); - - // Re-assign with decorated functions - /* eslint-disable no-func-assign */ - get = decorate.decorateWithMetrics(get, 'Onyx:get'); - set = decorate.decorateWithMetrics(set, 'Onyx:set'); - multiSet = decorate.decorateWithMetrics(multiSet, 'Onyx:multiSet'); - clear = decorate.decorateWithMetrics(clear, 'Onyx:clear'); - merge = decorate.decorateWithMetrics(merge, 'Onyx:merge'); - mergeCollection = decorate.decorateWithMetrics(mergeCollection, 'Onyx:mergeCollection'); - getAllKeys = decorate.decorateWithMetrics(getAllKeys, 'Onyx:getAllKeys'); - initializeWithDefaultKeyStates = decorate.decorateWithMetrics(initializeWithDefaultKeyStates, 'Onyx:defaults'); - update = decorate.decorateWithMetrics(update, 'Onyx:update'); - /* eslint-enable */ - - // Re-expose decorated methods - /* eslint-disable rulesdir/prefer-actions-set-data */ - Onyx.set = set; - Onyx.multiSet = multiSet; - Onyx.clear = clear; - Onyx.merge = merge; - Onyx.mergeCollection = mergeCollection; - Onyx.update = update; - /* eslint-enable */ - - // Expose stats methods on Onyx - Onyx.getMetrics = decorate.getMetrics; - Onyx.resetMetrics = decorate.resetMetrics; - Onyx.printMetrics = decorate.printMetrics; -} - export default Onyx; diff --git a/lib/PerformanceUtils.ts b/lib/PerformanceUtils.ts new file mode 100644 index 000000000..b378eb283 --- /dev/null +++ b/lib/PerformanceUtils.ts @@ -0,0 +1,66 @@ +import lodashTransform from 'lodash/transform'; +import {deepEqual} from 'fast-equals'; + +type UnknownObject = Record; + +type LogParams = { + keyThatChanged?: string; + difference?: unknown; + previousValue?: unknown; + newValue?: unknown; +}; + +type Mapping = Record & { + key: string; + displayName: string; +}; + +let debugSetState = false; + +function setShouldDebugSetState(debug: boolean) { + debugSetState = debug; +} + +/** + * Deep diff between two objects. Useful for figuring out what changed about an object from one render to the next so + * that state and props updates can be optimized. + */ +function diffObject(object: TObject, base: TBase): UnknownObject { + return lodashTransform(object, (result: UnknownObject, value, key) => { + if (deepEqual(value, base[key])) { + return; + } + + if (typeof value === 'object' && typeof base[key] === 'object') { + // eslint-disable-next-line no-param-reassign + result[key] = diffObject(value as UnknownObject, base[key] as UnknownObject); + } else { + // eslint-disable-next-line no-param-reassign + result[key] = value; + } + }); +} + +/** + * Provide insights into why a setState() call occurred by diffing the before and after values. + */ +function logSetStateCall(mapping: Mapping, previousValue: unknown, newValue: unknown, caller: string, keyThatChanged: string) { + if (!debugSetState) { + return; + } + + const logParams: LogParams = {}; + if (keyThatChanged) { + logParams.keyThatChanged = keyThatChanged; + } + if (newValue && previousValue && typeof newValue === 'object' && typeof previousValue === 'object') { + logParams.difference = diffObject(previousValue as UnknownObject, newValue as UnknownObject); + } else { + logParams.previousValue = previousValue; + logParams.newValue = newValue; + } + + console.debug(`[Onyx-Debug] ${mapping.displayName} setState() called. Subscribed to key '${mapping.key}' (${caller})`, logParams); +} + +export {logSetStateCall, setShouldDebugSetState}; diff --git a/lib/metrics/PerformanceUtils.js b/lib/metrics/PerformanceUtils.js deleted file mode 100644 index df6191281..000000000 --- a/lib/metrics/PerformanceUtils.js +++ /dev/null @@ -1,63 +0,0 @@ -import lodashTransform from 'lodash/transform'; -import _ from 'underscore'; - -let debugSetState = false; - -/** - * @param {Boolean} debug - */ -function setShouldDebugSetState(debug) { - debugSetState = debug; -} - -/** - * Deep diff between two objects. Useful for figuring out what changed about an object from one render to the next so - * that state and props updates can be optimized. - * - * @param {Object} object - * @param {Object} base - * @return {Object} - */ -function diffObject(object, base) { - function changes(obj, comparisonObject) { - return lodashTransform(obj, (result, value, key) => { - if (_.isEqual(value, comparisonObject[key])) { - return; - } - - // eslint-disable-next-line no-param-reassign - result[key] = _.isObject(value) && _.isObject(comparisonObject[key]) ? changes(value, comparisonObject[key]) : value; - }); - } - return changes(object, base); -} - -/** - * Provide insights into why a setState() call occurred by diffing the before and after values. - * - * @param {Object} mapping - * @param {*} previousValue - * @param {*} newValue - * @param {String} caller - * @param {String} [keyThatChanged] - */ -function logSetStateCall(mapping, previousValue, newValue, caller, keyThatChanged) { - if (!debugSetState) { - return; - } - - const logParams = {}; - if (keyThatChanged) { - logParams.keyThatChanged = keyThatChanged; - } - if (_.isObject(newValue) && _.isObject(previousValue)) { - logParams.difference = diffObject(previousValue, newValue); - } else { - logParams.previousValue = previousValue; - logParams.newValue = newValue; - } - - console.debug(`[Onyx-Debug] ${mapping.displayName} setState() called. Subscribed to key '${mapping.key}' (${caller})`, logParams); -} - -export {logSetStateCall, setShouldDebugSetState}; diff --git a/lib/metrics/index.js b/lib/metrics/index.js deleted file mode 100644 index 638e45090..000000000 --- a/lib/metrics/index.js +++ /dev/null @@ -1,10 +0,0 @@ -// For web-only implementations of Onyx, this module will just be a no-op - -function decorateWithMetrics(func) { - return func; -} -function getMetrics() {} -function printMetrics() {} -function resetMetrics() {} - -export {decorateWithMetrics, getMetrics, resetMetrics, printMetrics}; diff --git a/lib/metrics/index.native.js b/lib/metrics/index.native.js deleted file mode 100644 index dd1354abc..000000000 --- a/lib/metrics/index.native.js +++ /dev/null @@ -1,251 +0,0 @@ -import _ from 'underscore'; -import performance from 'react-native-performance'; -import MDTable from '../MDTable'; - -const decoratedAliases = new Set(); - -/** - * Capture a start mark to performance entries - * @param {string} alias - * @param {Array<*>} args - * @returns {{name: string, startTime:number, detail: {args: [], alias: string}}} - */ -function addMark(alias, args) { - return performance.mark(alias, {detail: {args, alias}}); -} - -/** - * Capture a measurement between the start mark and now - * @param {{name: string, startTime:number, detail: {args: []}}} startMark - * @param {*} detail - */ -function measureMarkToNow(startMark, detail) { - performance.measure(`${startMark.name} [${startMark.detail.args.toString()}]`, { - start: startMark.startTime, - end: performance.now(), - detail: {...startMark.detail, ...detail}, - }); -} - -/** - * Wraps a function with metrics capturing logic - * @param {function} func - * @param {String} [alias] - * @returns {function} The wrapped function - */ -function decorateWithMetrics(func, alias = func.name) { - if (decoratedAliases.has(alias)) { - throw new Error(`"${alias}" is already decorated`); - } - - decoratedAliases.add(alias); - - function decorated(...args) { - const mark = addMark(alias, args); - - // eslint-disable-next-line no-invalid-this - const originalPromise = func.apply(this, args); - - /* - * Then handlers added here are not affecting the original promise - * They create a separate chain that's not exposed (returned) to the original caller - * */ - originalPromise - .then((result) => { - measureMarkToNow(mark, {result}); - }) - .catch((error) => { - measureMarkToNow(mark, {error}); - }); - - return originalPromise; - } - - return decorated; -} - -/** - * Calculate the total sum of a given key in a list - * @param {Array>} list - * @param {string} prop - * @returns {number} - */ -function sum(list, prop) { - return _.reduce(list, (memo, next) => memo + next[prop], 0); -} - -/** - * Aggregates and returns benchmark information - * @returns {{summaries: Record, totalTime: number, lastCompleteCall: *}} - * An object with - * - `totalTime` - total time spent by decorated methods - * - `lastCompleteCall` - millisecond since launch the last call completed at - * - `summaries` - mapping of all captured stats: summaries.methodName -> method stats - */ -function getMetrics() { - const summaries = _.chain(performance.getEntriesByType('measure')) - .filter((entry) => entry.detail && decoratedAliases.has(entry.detail.alias)) - .groupBy((entry) => entry.detail.alias) - .map((calls, methodName) => { - const total = sum(calls, 'duration'); - const avg = total / calls.length || 0; - const max = _.max(calls, 'duration').duration || 0; - const min = _.min(calls, 'duration').duration || 0; - - // Latest complete call (by end time) for all the calls made to the current method - const lastCall = _.max(calls, (call) => call.startTime + call.duration); - - return [ - methodName, - { - methodName, - total, - max, - min, - avg, - lastCall, - calls, - }, - ]; - }) - .object() // Create a map like methodName -> StatSummary - .value(); - - const totalTime = sum(_.values(summaries), 'total'); - - // Latest complete call (by end time) of all methods up to this point - const lastCompleteCall = _.max(_.values(summaries), (summary) => summary.lastCall.startTime + summary.lastCall.duration).lastCall; - - return { - totalTime, - summaries, - lastCompleteCall, - }; -} - -/** - * Convert milliseconds to human readable time - * @param {number} millis - * @param {boolean} [raw=false] - * @returns {string|number} - */ -function toDuration(millis, raw = false) { - if (raw) { - return millis; - } - - const minute = 60 * 1000; - if (millis > minute) { - return `${(millis / minute).toFixed(1)}min`; - } - - const second = 1000; - if (millis > second) { - return `${(millis / second).toFixed(2)}sec`; - } - - return `${millis.toFixed(3)}ms`; -} - -/** - * Print extensive information on the dev console - * max, min, average, total time for each method - * and a table of individual calls - * - * @param {Object} [options] - * @param {boolean} [options.raw=false] - setting this to true will print raw instead of human friendly times - * Useful when you copy the printed table to excel and let excel do the number formatting - * @param {'console'|'csv'|'json'|'string'} [options.format=console] The output format of this function - * `string` is useful when __DEV__ is set to `false` as writing to the console is disabled, but the result of this - * method would still get printed as output - * @param {string[]} [options.methods] Print stats only for these method names - * @returns {string|undefined} - */ -function printMetrics({raw = false, format = 'console', methods} = {}) { - const {totalTime, summaries, lastCompleteCall} = getMetrics(); - - const tableSummary = MDTable.factory({ - heading: ['method', 'total time spent', 'max', 'min', 'avg', 'time last call completed', 'calls made'], - leftAlignedCols: [0], - }); - - /* Performance marks (startTimes) are relative to system uptime - * timeOrigin is the point at which the app started to init - * We use timeOrigin to display times relative to app launch time - * See: https://github.com/oblador/react-native-performance/issues/50 */ - const timeOrigin = performance.timeOrigin; - const methodNames = _.isArray(methods) ? methods : _.keys(summaries); - - const methodCallTables = _.chain(methodNames) - .filter((methodName) => summaries[methodName] && summaries[methodName].avg > 0) - .map((methodName) => { - const {calls, ...methodStats} = summaries[methodName]; - tableSummary.addRow( - methodName, - toDuration(methodStats.total, raw), - toDuration(methodStats.max, raw), - toDuration(methodStats.min, raw), - toDuration(methodStats.avg, raw), - toDuration(methodStats.lastCall.startTime + methodStats.lastCall.duration - timeOrigin, raw), - calls.length, - ); - - return MDTable.factory({ - title: methodName, - heading: ['start time', 'end time', 'duration', 'args'], - leftAlignedCols: [3], - rows: _.map(calls, (call) => [ - toDuration(call.startTime - performance.timeOrigin, raw), - toDuration(call.startTime + call.duration - timeOrigin, raw), - toDuration(call.duration, raw), - _.map(call.detail.args, String).join(', ').slice(0, 60), // Restrict cell width to 60 chars max - ]), - }); - }) - .value(); - - if (/csv|json|string/i.test(format)) { - const allTables = [tableSummary, ...methodCallTables]; - - return _.map(allTables, (table) => { - switch (format.toLowerCase()) { - case 'csv': - return table.toCSV(); - case 'json': - return table.toJSON(); - default: - return table.toString(); - } - }).join('\n\n'); - } - - const lastComplete = lastCompleteCall && toDuration(lastCompleteCall.startTime + lastCompleteCall.duration - timeOrigin, raw); - - const mainOutput = ['### Onyx Benchmark', ` - Total: ${toDuration(totalTime, raw)}`, ` - Last call finished at: ${lastComplete || 'N/A'}`, '', tableSummary.toString()]; - - /* eslint-disable no-console */ - console.info(mainOutput.join('\n')); - methodCallTables.forEach((table) => { - console.groupCollapsed(table.getTitle()); - console.info(table.toString()); - console.groupEnd(); - }); - /* eslint-enable */ -} - -/** - * Clears all collected metrics. - */ -function resetMetrics() { - const {summaries} = getMetrics(); - - _.chain(summaries) - .map((summary) => summary.calls) - .flatten() - .each((measure) => { - performance.clearMarks(measure.detail.alias); - performance.clearMeasures(measure.name); - }); -} - -export {decorateWithMetrics, getMetrics, resetMetrics, printMetrics}; diff --git a/tests/unit/metricsTest.js b/tests/unit/metricsTest.js deleted file mode 100644 index 734892500..000000000 --- a/tests/unit/metricsTest.js +++ /dev/null @@ -1,332 +0,0 @@ -import _ from 'underscore'; -import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; - -describe('decorateWithMetrics', () => { - let decorateWithMetrics; - let getMetrics; - let resetMetrics; - - beforeEach(() => { - jest.resetModules(); - const metrics = require('../../lib/metrics'); - decorateWithMetrics = metrics.decorateWithMetrics; - getMetrics = metrics.getMetrics; - resetMetrics = metrics.resetMetrics; - }); - - it('Should collect metrics for a single method, single call', () => { - // Given an async function that resolves with an object - const mockedResult = {mockedKey: 'mockedValue'}; - let mockFn = jest.fn().mockResolvedValueOnce(mockedResult); - - // When it is decorated and executed - mockFn = decorateWithMetrics(mockFn, 'mockFn'); - mockFn('mockedKey'); - - return waitForPromisesToResolve().then(() => { - // Then stats should contain expected data regarding the call: timings, args and result - const metrics = getMetrics(); - expect(metrics).toEqual( - expect.objectContaining({ - totalTime: expect.any(Number), - lastCompleteCall: expect.any(Object), - summaries: expect.objectContaining({mockFn: expect.any(Object)}), - }), - ); - - expect(_.keys(metrics.summaries)).toHaveLength(1); - - const firstCall = metrics.summaries.mockFn.calls[0]; - expect(firstCall.startTime).toEqual(expect.any(Number)); - expect(firstCall.detail.args).toEqual(['mockedKey']); - }); - }); - - it('Should use function.name when alias was not provided', () => { - // Given a regular JS function - function mockFunc() { - return Promise.resolve(); - } - - // When decorated without passing an "alias" parameter - // eslint-disable-next-line no-func-assign - mockFunc = decorateWithMetrics(mockFunc); - mockFunc(); - - return waitForPromisesToResolve().then(() => { - // Then the alias should be inferred from the function name - const stats = getMetrics(); - const result = stats.summaries.mockFunc; - expect(result).not.toBeUndefined(); - expect(result).toEqual(expect.objectContaining({methodName: 'mockFunc'})); - }); - }); - - it('Should collect metrics for multiple calls', () => { - // Given an async function that resolves with an object - let mockFn = jest.fn().mockResolvedValueOnce({mock: 'value'}).mockResolvedValueOnce({mock: 'value'}).mockResolvedValueOnce({mock: 'value'}); - - // When it is decorated and executed - mockFn = decorateWithMetrics(mockFn, 'mockFn'); - mockFn('mockedKey'); - mockFn('mockedKey3'); - mockFn('mockedKey2'); - - return waitForPromisesToResolve().then(() => { - // Then stats should have data regarding each call - const calls = getMetrics().summaries.mockFn.calls; - expect(calls).toHaveLength(3); - expect(calls).toEqual([ - expect.objectContaining({ - detail: expect.objectContaining({args: ['mockedKey']}), - }), - expect.objectContaining({ - detail: expect.objectContaining({args: ['mockedKey3']}), - }), - expect.objectContaining({ - detail: expect.objectContaining({args: ['mockedKey2']}), - }), - ]); - }); - }); - - it('Should work for methods that return Promise', () => { - // Given an async function that resolves with no data - let mockFn = jest.fn().mockResolvedValueOnce().mockResolvedValueOnce(); - - // When it is decorated and executed - mockFn = decorateWithMetrics(mockFn, 'mockFn'); - mockFn('mockedKey', {ids: [1, 2, 3]}); - mockFn('mockedKey', {ids: [4, 5, 6]}); - - return waitForPromisesToResolve().then(() => { - // Then stats should still contain data about the calls - const calls = getMetrics().summaries.mockFn.calls; - expect(calls).toHaveLength(2); - expect(calls).toEqual([ - expect.objectContaining({ - detail: {args: ['mockedKey', {ids: [1, 2, 3]}], alias: 'mockFn'}, - }), - expect.objectContaining({ - detail: {args: ['mockedKey', {ids: [4, 5, 6]}], alias: 'mockFn'}, - }), - ]); - }); - }); - - it('Should not affect the returned value from the original method', () => { - // Given an async function that resolves with an object - const mockedResult = {mockedKey: 'mockedValue'}; - let mockFn = jest.fn().mockResolvedValueOnce(mockedResult); - - // When it is decorated and executed - mockFn = decorateWithMetrics(mockFn, 'mockFn'); - - return mockFn('mockedKey') - .then((result) => { - // Then the result should be the same with the result of the non decorated version - expect(result).toEqual(mockedResult); - }) - .then(waitForPromisesToResolve); - }); - - it('Should collect metrics for a multiple functions, single call', () => { - // Given multiple async functions that resolves with objects - let mockGet = jest.fn().mockResolvedValueOnce({mock: 'value'}); - let mockGetAllKeys = jest.fn().mockResolvedValueOnce(['my', 'mock', 'keys']); - - // When each is decorated and executed one time - mockGet = decorateWithMetrics(mockGet, 'mockGet'); - mockGetAllKeys = decorateWithMetrics(mockGetAllKeys, 'mockGetAllKeys'); - - mockGet('mockedKey'); - mockGetAllKeys(); - - return waitForPromisesToResolve().then(() => { - // Then stats should contain data for each function and each call under the correct function alias - const stats = getMetrics().summaries; - expect(_.keys(stats)).toHaveLength(2); - - expect(stats).toEqual( - expect.objectContaining({ - mockGet: expect.any(Object), - mockGetAllKeys: expect.any(Object), - }), - ); - - expect(stats.mockGet.calls).toHaveLength(1); - expect(stats.mockGetAllKeys.calls).toHaveLength(1); - }); - }); - - it('Should collect metrics for a multiple functions, multiple call', () => { - // Given multiple async functions that resolves with objects - let mockGetAllKeys = jest - .fn() - .mockResolvedValueOnce(['my', 'mock', 'keys']) - .mockResolvedValueOnce(['my', 'mock', 'keys', 'and']) - .mockResolvedValueOnce(['my', 'mock', 'keys', 'and', 'more']); - - let mockSetItem = jest.fn().mockResolvedValueOnce().mockResolvedValueOnce(); - - // When they are decorated - mockGetAllKeys = decorateWithMetrics(mockGetAllKeys, 'mockGetAllKeys'); - mockSetItem = decorateWithMetrics(mockSetItem, 'mockSetItem'); - - // When each is executed multiple times - mockGetAllKeys(); - mockSetItem('and', 'Mock value'); - mockGetAllKeys(); - mockSetItem('more', 'Mock value'); - mockGetAllKeys(); - - return waitForPromisesToResolve().then(() => { - // Then stats should contain data for each function and each call under the correct function alias - const stats = getMetrics().summaries; - expect(_.keys(stats)).toHaveLength(2); - - expect(stats).toEqual( - expect.objectContaining({ - mockGetAllKeys: expect.any(Object), - mockSetItem: expect.any(Object), - }), - ); - - expect(stats.mockGetAllKeys.calls).toHaveLength(3); - expect(stats.mockSetItem.calls).toHaveLength(2); - }); - }); - - it('Attempting to decorate already decorated method should throw', () => { - // Given a function that is decorated - let mockFn = jest.fn(); - mockFn = decorateWithMetrics(mockFn, 'mockFn'); - - // When you try to decorate again the same function - expect(() => decorateWithMetrics(mockFn, 'mockFn')) - // Then it should throw an exception - .toThrow('"mockFn" is already decorated'); - }); - - it('Adding more data after clearing should work', () => { - // Given an async function that is decorated - let mockFn = jest.fn().mockResolvedValueOnce().mockResolvedValueOnce().mockResolvedValueOnce(); - - mockFn = decorateWithMetrics(mockFn, 'mockFn'); - - // Given some call made with the decorated function - mockFn('mockedKey', {ids: [1, 2, 3]}); - mockFn('mockedKey', {ids: [4, 5, 6]}); - - return waitForPromisesToResolve() - .then(() => { - // When metrics are reset - expect(getMetrics().summaries.mockFn.calls).toHaveLength(2); - resetMetrics(); - - // Then no data regarding the calls that happened before should exist - expect(getMetrics().summaries.mockFn).not.toBeDefined(); - - // When more calls are made - mockFn('mockedKey', {ids: [1, 2, 3]}); - - return waitForPromisesToResolve(); - }) - .then(() => { - // Then only these calls should appear in stats - expect(getMetrics().summaries.mockFn.calls).toHaveLength(1); - }); - }); - - it('Should work with custom alias', () => { - // Given an async function that resolves with an object - const mockedResult = {mockedKey: 'mockedValue'}; - let mockFn = jest.fn().mockResolvedValueOnce(mockedResult); - - // When it is decorated with a custom alias as a 2nd parameter - mockFn = decorateWithMetrics(mockFn, 'mock:get'); - mockFn('mockKey'); - - return waitForPromisesToResolve() - .then(() => { - // Then stats should contain data regarding the calls under that custom alias - const stats = getMetrics().summaries; - expect(stats).toEqual( - expect.objectContaining({ - 'mock:get': expect.any(Object), - }), - ); - }) - .finally(resetMetrics); - }); - - it('Should return 0 total time and stats when no stats are collected yet', () => { - // Given no calls made - - // When getMetrics is called - const result = getMetrics(); - - // Then stats should be empty and the total time 0 - expect(result.summaries).toEqual({}); - expect(result.totalTime).toEqual(0); - expect(result.lastCompleteCall).not.toBeDefined(); - }); - - it('Should calculate total and average correctly', () => { - // Given an async function that resolves with an object - const mockedResult = {mockedKey: 'mockedValue'}; - let mockFn = jest.fn().mockResolvedValue(mockedResult); - - // Given mocked performance than returns +250ms for each consecutive call - let i = 0; - jest.spyOn(global.performance, 'now').mockImplementation(() => 250 * i++); - - // When it is decorated - mockFn = decorateWithMetrics(mockFn, 'mockFn'); - - // When the decorated function is executed - mockFn('mockedKey') - .then(waitForPromisesToResolve) - .then(() => { - // Then metrics should contain correctly calculated data - const metrics = getMetrics(); - - expect(metrics).toEqual( - expect.objectContaining({ - totalTime: 250, - averageTime: 250, - }), - ); - }); - - // When the decorated function is executed again - mockFn('mockedKey') - .then(waitForPromisesToResolve) - .then(() => { - // Then metrics should contain correctly calculated data - const metrics = getMetrics(); - - expect(metrics).toEqual( - expect.objectContaining({ - totalTime: 500, - averageTime: 250, - }), - ); - }); - - // When the decorated function is executed again - mockFn('mockedKey') - .then(waitForPromisesToResolve) - .then(() => { - // Then metrics should contain correctly calculated data - const metrics = getMetrics(); - - expect(metrics).toEqual( - expect.objectContaining({ - totalTime: 750, - averageTime: 250, - }), - ); - }); - }); -}); diff --git a/tests/unit/webMetricsTest.js b/tests/unit/webMetricsTest.js deleted file mode 100644 index 63fb6314d..000000000 --- a/tests/unit/webMetricsTest.js +++ /dev/null @@ -1,19 +0,0 @@ -describe('decorateWithMetrics', () => { - let decorateWithMetrics; - - beforeEach(() => { - jest.resetModules(); - // eslint-disable-next-line import/extensions - const metrics = require('../../lib/metrics/index.js'); - decorateWithMetrics = metrics.decorateWithMetrics; - }); - - it('Should return original function', () => { - const mockFn = jest.fn(); - - const decoratedFn = decorateWithMetrics(mockFn, 'mockFn'); - - expect(decoratedFn).toBeTruthy(); - expect(decoratedFn).toStrictEqual(mockFn); - }); -}); From a3d0a9b7968c08d8758ddb30a95b3fcf3cc74826 Mon Sep 17 00:00:00 2001 From: Bartosz Grajdek Date: Wed, 28 Feb 2024 10:35:44 +0100 Subject: [PATCH 2/3] feat: remove unnecessary test file --- tests/unit/onyxMetricsDecorationTest.js | 79 ------------------------- 1 file changed, 79 deletions(-) delete mode 100644 tests/unit/onyxMetricsDecorationTest.js diff --git a/tests/unit/onyxMetricsDecorationTest.js b/tests/unit/onyxMetricsDecorationTest.js deleted file mode 100644 index b123fa8c0..000000000 --- a/tests/unit/onyxMetricsDecorationTest.js +++ /dev/null @@ -1,79 +0,0 @@ -import waitForPromisesToResolve from '../utils/waitForPromisesToResolve'; - -describe('Onyx', () => { - describe('Metrics Capturing Decoration', () => { - let Onyx; - - const ONYX_KEYS = { - TEST_KEY: 'test', - OTHER_TEST: 'otherTest', - }; - - // Always use a "fresh" (and undecorated) instance - beforeEach(() => { - jest.resetModules(); - Onyx = require('../../lib').default; - }); - - it('Should expose metrics methods when `captureMetrics` is true', () => { - // When Onyx is initialized with `captureMetrics: true` - Onyx.init({ - keys: ONYX_KEYS, - registerStorageEventListener: jest.fn(), - captureMetrics: true, - }); - - // Then Onyx should have statistic related methods - expect(Onyx.getMetrics).toEqual(expect.any(Function)); - expect(Onyx.printMetrics).toEqual(expect.any(Function)); - expect(Onyx.resetMetrics).toEqual(expect.any(Function)); - }); - - it('Should not expose metrics methods when `captureMetrics` is false or not set', () => { - // When Onyx is initialized without setting `captureMetrics` - Onyx.init({ - keys: ONYX_KEYS, - registerStorageEventListener: jest.fn(), - }); - - // Then Onyx should not have statistic related methods - expect(Onyx.getMetrics).not.toBeDefined(); - expect(Onyx.printMetrics).not.toBeDefined(); - expect(Onyx.resetMetrics).not.toBeDefined(); - - // When Onyx is initialized with `captureMetrics: false` - Onyx.init({ - keys: ONYX_KEYS, - registerStorageEventListener: jest.fn(), - captureMetrics: false, - }); - - // Then Onyx should not have statistic related methods - expect(Onyx.getMetrics).not.toBeDefined(); - expect(Onyx.printMetrics).not.toBeDefined(); - expect(Onyx.resetMetrics).not.toBeDefined(); - }); - - it('Should decorate exposed methods', () => { - // Given Onyx is initialized with `captureMetrics: true` - Onyx.init({ - keys: ONYX_KEYS, - registerStorageEventListener: jest.fn(), - captureMetrics: true, - }); - - // When calling decorated methods through Onyx[methodName] - const methods = ['set', 'multiSet', 'clear', 'merge', 'mergeCollection']; - methods.forEach((name) => Onyx[name]('mockKey', {mockKey: {mockValue: 'mockValue'}})); - - return waitForPromisesToResolve().then(() => { - // Then metrics should have captured data for each method - const summaries = Onyx.getMetrics().summaries; - - methods.forEach((name) => { - expect(summaries[`Onyx:${name}`].total).toBeGreaterThan(0); - }); - }); - }); - }); -}); From b7645d9409e8b75d17966a4ca4ca790346151a04 Mon Sep 17 00:00:00 2001 From: Bartosz Grajdek Date: Fri, 1 Mar 2024 13:44:16 +0100 Subject: [PATCH 3/3] feat: add changes to onyx.d.ts --- lib/Onyx.d.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/Onyx.d.ts b/lib/Onyx.d.ts index b1c0cbe4f..efc0fa689 100644 --- a/lib/Onyx.d.ts +++ b/lib/Onyx.d.ts @@ -97,7 +97,6 @@ type InitOptions = { initialKeyStates?: Partial; safeEvictionKeys?: OnyxKey[]; maxCachedKeysCount?: number; - captureMetrics?: boolean; shouldSyncMultipleInstances?: boolean; debugSetState?: boolean; }; @@ -267,7 +266,6 @@ declare function update(data: OnyxUpdate[]): Promise; * @param [options.maxCachedKeysCount=55] Sets how many recent keys should we try to keep in cache * Setting this to 0 would practically mean no cache * We try to free cache when we connect to a safe eviction key - * @param [options.captureMetrics] Enables Onyx benchmarking and exposes the get/print/reset functions * @param [options.shouldSyncMultipleInstances] Auto synchronize storage events between multiple instances * of Onyx running in different tabs/windows. Defaults to true for platforms that support local storage (web/desktop) * @param [options.debugSetState] Enables debugging setState() calls to connected components.