Revert "Report result was found or not (#46)" (#49)

korengal · web-flow · commit f825cd7278bd · 2022-07-18T12:54:14.000+03:00
This reverts commit 5cd4919.
diff --git a/README.md b/README.md
@@ -101,10 +101,9 @@ type MonitoredOptions = {
     level?: 'info' | 'debug'; //which log level to write (debug is the default)
     logAsError?: boolean; //enables to write error log in case the global `logErrorsAsWarnings` is on
     logErrorAsInfo?: boolean //enables to write the error as info log
-    shouldMonitorError: e => boolean // define the conditions of reporting and logging an error, defaults to true
-    shouldMonitorSuccess: (r: T) => boolean // define the conditions of reporting and logging wether a result was successful or not, defaults to true
+    shouldMonitorError: e => boolean //determines if error should be monitored and logged, defaults to true
+    shouldMonitorSuccess: (r: T) => boolean //determines if success result should be monitored and logged, defaults to true
     tags?: Record<string, string>; //add more information/labels to metrics
-    shouldMonitorResultFound?: (r: Awaited<T>) => boolean; // define the conditions of reporting wether a result was found or not, defaults to false
 };
 ```
 
@@ -144,18 +143,6 @@ const result = monitored(
 );
 ```
 
-#### You can report an empty result by defining a predicate in  `shouldMonitorResultFound`:
-
-```ts
-const result = monitored(
-    'functionName',
-    () => {
-        return [1, 2, 3];
-    },
-    {context: {id: 'some context'}, shouldMonitorResultFound: (result)=>{return result.length > 0}}
-);
-```
-
 ### `flush`
 
 Wait until all current metrics are sent to the server. <br>
diff --git a/__tests__/PrometheusPlugin.spec.ts b/__tests__/PrometheusPlugin.spec.ts
@@ -18,10 +18,6 @@ const gauge = {
     labels: jest.fn().mockImplementation(() => ({set: gaugeSet})),
 };
 
-const isResultFoundCallback = (result: Awaited<number[]>): boolean => {
-    return result.length > 0;
-};
-
 jest.mock('prom-client', () => ({
     Histogram: jest.fn().mockImplementation(() => histogram),
     Counter: jest.fn().mockImplementation(() => counter),
@@ -68,60 +64,6 @@ describe('PrometheusPlugin', () => {
         expect(counterInc).toHaveBeenCalledWith(1);
         expect(counter.labels).toHaveBeenCalledWith({result: 'success'});
         expect(counterInc).toHaveBeenCalledWith(1);
-        expect(counter.labels).not.toHaveBeenCalledWith({result: 'found'});
-        expect(counter.labels).not.toHaveBeenCalledWith({result: 'notFound'});
-        expect(histogram.labels).toHaveBeenCalledWith({result: 'success'});
-        expect(histogramObserve).toHaveBeenCalledWith(expect.any(Number));
-    });
-
-    it('reports result found on onSuccess', () => {
-        monitor.monitored('abc', () => [1, 2, 3], {shouldMonitorResultFound: isResultFoundCallback});
-
-        expect(Counter).toHaveBeenCalledWith({
-            name: `abc_count`,
-            help: `abc_count`,
-            labelNames: ['result'],
-        });
-
-        expect(Histogram).toHaveBeenCalledWith({
-            name: `abc_execution_time`,
-            help: `abc_execution_time`,
-            buckets: DEFAULT_BUCKETS,
-            labelNames: ['result'],
-        });
-
-        expect(counter.labels).toHaveBeenCalledWith({result: 'start'});
-        expect(counterInc).toHaveBeenCalledWith(1);
-        expect(counter.labels).toHaveBeenCalledWith({result: 'success'});
-        expect(counterInc).toHaveBeenCalledWith(1);
-        expect(counter.labels).toHaveBeenCalledWith({result: 'found'});
-        expect(counterInc).toHaveBeenCalledWith(1);
-        expect(histogram.labels).toHaveBeenCalledWith({result: 'success'});
-        expect(histogramObserve).toHaveBeenCalledWith(expect.any(Number));
-    });
-
-    it('reports result not found on onSuccess', () => {
-        monitor.monitored('abc', () => [], {shouldMonitorResultFound: isResultFoundCallback});
-
-        expect(Counter).toHaveBeenCalledWith({
-            name: `abc_count`,
-            help: `abc_count`,
-            labelNames: ['result'],
-        });
-
-        expect(Histogram).toHaveBeenCalledWith({
-            name: `abc_execution_time`,
-            help: `abc_execution_time`,
-            buckets: DEFAULT_BUCKETS,
-            labelNames: ['result'],
-        });
-
-        expect(counter.labels).toHaveBeenCalledWith({result: 'start'});
-        expect(counterInc).toHaveBeenCalledWith(1);
-        expect(counter.labels).toHaveBeenCalledWith({result: 'success'});
-        expect(counterInc).toHaveBeenCalledWith(1);
-        expect(counter.labels).toHaveBeenCalledWith({result: 'notFound'});
-        expect(counterInc).toHaveBeenCalledWith(1);
         expect(histogram.labels).toHaveBeenCalledWith({result: 'success'});
         expect(histogramObserve).toHaveBeenCalledWith(expect.any(Number));
     });
@@ -137,30 +79,6 @@ describe('PrometheusPlugin', () => {
         expect(counterInc).toHaveBeenCalledWith(1);
         expect(counter.labels).toHaveBeenCalledWith({result: 'failure'});
         expect(counterInc).toHaveBeenCalledWith(1);
-        expect(counter.labels).not.toHaveBeenCalledWith({result: 'found'});
-        expect(counter.labels).not.toHaveBeenCalledWith({result: 'notFound'});
-    });
-
-    it('not to report result found onFailure', () => {
-        expect(() =>
-            monitor.monitored(
-                'abc',
-                () => {
-                    if (false) {
-                        return [1, 2];
-                    }
-                    throw new Error('123');
-                },
-                {shouldMonitorResultFound: isResultFoundCallback}
-            )
-        ).toThrow();
-
-        expect(counter.labels).toHaveBeenCalledWith({result: 'start'});
-        expect(counterInc).toHaveBeenCalledWith(1);
-        expect(counter.labels).toHaveBeenCalledWith({result: 'failure'});
-        expect(counterInc).toHaveBeenCalledWith(1);
-        expect(counter.labels).not.toHaveBeenCalledWith({result: 'found'});
-        expect(counter.labels).not.toHaveBeenCalledWith({result: 'notFound'});
     });
 
     it('should call registry', () => {
diff --git a/__tests__/StatsdPlugin.spec.ts b/__tests__/StatsdPlugin.spec.ts
@@ -2,20 +2,11 @@ import {StatsCb, Tags} from 'hot-shots';
 import {mocked} from 'jest-mock';
 import Monitor from '../src/Monitor';
 import {StatsdPlugin, StatsdPluginOptions} from '../src/plugins/StatsdPlugin';
-import {
-    assertGaugeWasCalled,
-    assertIncrementWasCalled,
-    assertIncrementWasNotCalled,
-    assertTimingWasCalled,
-} from './utils';
+import {assertGaugeWasCalled, assertIncrementWasCalled, assertTimingWasCalled} from './utils';
 import {MockStatsdPlugin} from './__mocks__/plugins/StatsdPlugin';
 
 jest.mock('hot-shots');
 
-const isResultFoundCallback = (result: Awaited<number[]>): boolean => {
-    return result.length > 0;
-};
-
 beforeEach(() => {
     jest.resetAllMocks();
 });
@@ -35,26 +26,6 @@ describe('StatsdPlugin', () => {
         monitor.monitored('abc', () => 123);
 
         assertIncrementWasCalled(plugin, 'abc.start');
-        assertIncrementWasNotCalled(plugin, 'abc.result.found');
-        assertIncrementWasNotCalled(plugin, 'abc.result.notFound');
-        assertGaugeWasCalled(plugin, 'abc.ExecutionTime');
-        assertTimingWasCalled(plugin, 'abc.ExecutionTime');
-    });
-
-    test('onSuccess report result is found', () => {
-        monitor.monitored('abc', () => [1, 2], {shouldMonitorResultFound: isResultFoundCallback});
-
-        assertIncrementWasCalled(plugin, 'abc.start');
-        assertIncrementWasCalled(plugin, 'abc.result.found');
-        assertGaugeWasCalled(plugin, 'abc.ExecutionTime');
-        assertTimingWasCalled(plugin, 'abc.ExecutionTime');
-    });
-
-    test('onSuccess report result is not found', () => {
-        monitor.monitored('abc', () => [], {shouldMonitorResultFound: isResultFoundCallback});
-
-        assertIncrementWasCalled(plugin, 'abc.start');
-        assertIncrementWasCalled(plugin, 'abc.result.notFound');
         assertGaugeWasCalled(plugin, 'abc.ExecutionTime');
         assertTimingWasCalled(plugin, 'abc.ExecutionTime');
     });
@@ -75,28 +46,6 @@ describe('StatsdPlugin', () => {
 
         assertIncrementWasCalled(plugin, 'abc.start');
         assertIncrementWasCalled(plugin, 'abc.error');
-        assertIncrementWasNotCalled(plugin, 'abc.result.found');
-        assertIncrementWasNotCalled(plugin, 'abc.result.notFound');
-    });
-
-    test('onFailure dont report result is found', () => {
-        expect(() => {
-            monitor.monitored(
-                'abc',
-                () => {
-                    if (false) {
-                        return [1, 2];
-                    }
-                    throw new Error('123');
-                },
-                {shouldMonitorResultFound: isResultFoundCallback}
-            );
-        }).toThrow(new Error('123'));
-
-        assertIncrementWasCalled(plugin, 'abc.start');
-        assertIncrementWasCalled(plugin, 'abc.error');
-        assertIncrementWasNotCalled(plugin, 'abc.result.found');
-        assertIncrementWasNotCalled(plugin, 'abc.result.notFound');
     });
 });
 
diff --git a/__tests__/__mocks__/plugins/StatsdPlugin.ts b/__tests__/__mocks__/plugins/StatsdPlugin.ts
@@ -7,7 +7,6 @@ export class MockStatsdPlugin {
     onStart = StatsdPlugin.prototype.onStart;
     onSuccess = StatsdPlugin.prototype.onSuccess;
     onFailure = StatsdPlugin.prototype.onFailure;
-    reportResultIsFound = StatsdPlugin.prototype.reportResultIsFound;
     increment = jest.fn();
     gauge = jest.fn();
     timing = jest.fn();
diff --git a/__tests__/monitor.spec.ts b/__tests__/monitor.spec.ts
@@ -7,7 +7,6 @@ const mockPlugin: jest.Mocked<MonitoredPlugin> = {
     onStart: jest.fn(),
     onSuccess: jest.fn(),
     onFailure: jest.fn(),
-    reportResultIsFound: jest.fn(),
     flush: jest.fn(),
     gauge: jest.fn(),
     increment: jest.fn(),
@@ -189,50 +188,5 @@ describe('Monitor', () => {
             expect(() => monitor.monitored('test', doThrow(new Error()), {shouldMonitorError})).toThrow();
             expect(mockPlugin.onFailure).toHaveBeenCalled();
         });
-
-        describe('reportIsResultFound for %p', () => {
-            const callBack = (result: Awaited<number[]>): boolean => {
-                return result.length > 0;
-            };
-
-            test('async success', async () => {
-                const returnedValue = [1, 2];
-                const asyncMockFunc = jest.fn().mockResolvedValue(returnedValue);
-
-                await monitor.monitored('monitorFound', asyncMockFunc, {shouldMonitorResultFound: callBack});
-                expect(mockPlugin.reportResultIsFound).toHaveBeenCalledTimes(1);
-            });
-
-            test('async failed', async () => {
-                const mockError = new Error('error');
-                const asyncFailMockFunc = async () => {
-                    return Promise.reject(mockError);
-                };
-
-                await expect(
-                    monitor.monitored('monitorFound', asyncFailMockFunc, {shouldMonitorResultFound: callBack})
-                ).rejects.toBe(mockError);
-                expect(mockPlugin.reportResultIsFound).toHaveBeenCalledTimes(0);
-            });
-
-            test('sync success', () => {
-                const returnedValue = [1, 2];
-                const syncMockFunc = jest.fn().mockReturnValue(returnedValue);
-
-                monitor.monitored('monitorFound', syncMockFunc, {shouldMonitorResultFound: callBack});
-                expect(mockPlugin.reportResultIsFound).toHaveBeenCalledTimes(1);
-            });
-
-            test('sync failed', () => {
-                const mockError = new Error('error');
-                const syncFailMockFunc = () => {
-                    throw mockError;
-                };
-                expect(() =>
-                    monitor.monitored('monitorFound', syncFailMockFunc, {shouldMonitorResultFound: callBack})
-                ).toThrow(mockError);
-                expect(mockPlugin.reportResultIsFound).toHaveBeenCalledTimes(0);
-            });
-        });
     });
 });
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "monitored",
-  "version": "0.1.3",
+  "version": "0.1.2",
   "description": "A utility for monitoring services",
   "main": "dist/index.js",
   "types": "dist/index.d.ts",
diff --git a/src/Monitor.ts b/src/Monitor.ts
@@ -58,29 +58,13 @@ class Monitor {
         startTime: number,
         options: MonitoredOptions<T>
     ): Awaited<T> {
-        const {
-            shouldMonitorSuccess,
-            logResult,
-            level = 'debug',
-            context,
-            parseResult,
-            shouldMonitorResultFound,
-        } = options;
+        const {shouldMonitorSuccess, logResult, level = 'debug', context, parseResult} = options;
         const executionTime = Date.now() - startTime;
 
         if (shouldMonitorSuccess?.(result) ?? true) {
             this.plugins.onSuccess({scope, executionTime, options});
         }
 
-        if (shouldMonitorResultFound) {
-            try {
-                const isFound = shouldMonitorResultFound(result);
-                this.plugins.reportResultIsFound({scope, options}, isFound);
-            } catch {
-                this.monitoredLogger('debug', 'isResultFound callback failed', {extra: {context}});
-            }
-        }
-
         if (!this.config.disableSuccessLogs) {
             this.monitoredLogger(level, `${scope}.success`, {
                 extra: {
diff --git a/src/plugins/PluginsWrapper.ts b/src/plugins/PluginsWrapper.ts
@@ -1,11 +1,4 @@
-import {
-    EventOptions,
-    InitializationOptions,
-    MonitoredPlugin,
-    OnFailureOptions,
-    OnStartOptions,
-    OnSuccessOptions,
-} from './types';
+import {InitializationOptions, MonitoredPlugin, OnFailureOptions, OnStartOptions, OnSuccessOptions} from './types';
 
 export class PluginsWrapper implements Omit<MonitoredPlugin, 'initialize'> {
     constructor(private readonly plugins: MonitoredPlugin[], opts: InitializationOptions) {
@@ -24,10 +17,6 @@ export class PluginsWrapper implements Omit<MonitoredPlugin, 'initialize'> {
         this.plugins.forEach(p => p.onFailure(opts));
     }
 
-    reportResultIsFound(opts: EventOptions, isFound: boolean): void {
-        this.plugins.forEach(p => p.reportResultIsFound(opts, isFound));
-    }
-
     async increment(name: string, value?: number, tags?: Record<string, string>): Promise<void> {
         await Promise.all(this.plugins.map(p => p.increment(name, value, tags)));
     }
diff --git a/src/plugins/PrometheusPlugin.ts b/src/plugins/PrometheusPlugin.ts
@@ -1,5 +1,5 @@
 import {Counter, Histogram, register as registry, Gauge} from 'prom-client';
-import {EventOptions, MonitoredPlugin, OnFailureOptions, OnStartOptions, OnSuccessOptions} from './types';
+import {MonitoredPlugin, OnFailureOptions, OnStartOptions, OnSuccessOptions} from './types';
 
 export interface PrometheusPluginOptions {
     histogramBuckets?: number[];
@@ -31,12 +31,6 @@ export class PrometheusPlugin implements MonitoredPlugin {
         this.timing(scope, executionTime, labels);
     }
 
-    reportResultIsFound({scope, options}: EventOptions, isFound: boolean): void {
-        const isFoundLabel = isFound ? 'found' : 'notFound';
-        const labels = {result: isFoundLabel, ...options?.tags};
-        this.increment(scope, 1, labels);
-    }
-
     private getMetrics(scope: string) {
         if (!this.histograms[scope]) {
             this.histograms[scope] = new Histogram({
diff --git a/src/plugins/StatsdPlugin.ts b/src/plugins/StatsdPlugin.ts
@@ -2,14 +2,7 @@ import {ClientOptions, StatsD, Tags} from 'hot-shots';
 import {promisify} from 'util';
 import {timeoutPromise} from '../utils';
 import {Logger} from '../Logger';
-import {
-    EventOptions,
-    InitializationOptions,
-    MonitoredPlugin,
-    OnFailureOptions,
-    OnStartOptions,
-    OnSuccessOptions,
-} from './types';
+import {InitializationOptions, MonitoredPlugin, OnFailureOptions, OnStartOptions, OnSuccessOptions} from './types';
 
 const noop = () => {};
 
@@ -72,11 +65,6 @@ export class StatsdPlugin implements MonitoredPlugin {
         this.increment(`${scope}.error`, 1, options?.tags);
     }
 
-    reportResultIsFound({scope, options}: EventOptions, isFound: boolean): void {
-        const isFoundLabel = isFound ? 'found' : 'notFound';
-        this.increment(`${scope}.result.${isFoundLabel}`, 1, options?.tags);
-    }
-
     get statsd() {
         return this.client;
     }
@@ -106,7 +94,7 @@ export class StatsdPlugin implements MonitoredPlugin {
     }
 
     async flush(timeout: number = 2000) {
-        const remainingPromises = Object.values(this.pendingPromises).map(p => p.catch(noop));
+        const remainingPromises = Object.values(this.pendingPromises).map((p) => p.catch(noop));
         if (!remainingPromises.length) {
             return true;
         }
diff --git a/src/plugins/types.ts b/src/plugins/types.ts
diff --git a/src/types.ts b/src/types.ts

Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`{`
`2`	`2`	`"name": "monitored",`
`3`		`- "version": "0.1.3",`
	`3`	`+ "version": "0.1.2",`
`4`	`4`	`"description": "A utility for monitoring services",`
`5`	`5`	`"main": "dist/index.js",`
`6`	`6`	`"types": "dist/index.d.ts",`