refactor: improvements

Author: netanelavr

.eslintrc

@@ -21,7 +21,36 @@
     "semi": ["error", "always"],
     "@typescript-eslint/explicit-function-return-type": "off",
     "@typescript-eslint/no-explicit-any": "off",
-    "@typescript-eslint/no-unused-vars": ["error", { "argsIgnorePattern": "^_" }]
+    "@typescript-eslint/no-unused-vars": ["error", { "argsIgnorePattern": "^_" }],
+    "object-curly-spacing": ["error", "always"],
+    "@typescript-eslint/brace-style": ["error", "1tbs"],
+    "@typescript-eslint/type-annotation-spacing": [
+      "error",
+      {
+        "before": false,
+        "after": true,
+        "overrides": {
+          "parameter": { "before": false, "after": true },
+          "property": { "before": false, "after": true }
+        }
+      }
+    ],
+    "space-before-function-paren": [
+      "error",
+      {
+        "anonymous": "always",
+        "named": "never",
+        "asyncArrow": "always"
+      }
+    ],
+    "space-in-parens": ["error", "always"], // Enforce spaces inside parentheses
+    "keyword-spacing": [
+      "error",
+      {
+        "before": true,
+        "after": true
+      }
+    ]
   },
   "ignorePatterns": ["dist/", "dist/*"]
 }

CONTRIBUTING.md

@@ -27,7 +27,7 @@
    ```bash
    npm install
    npm run test
-   npm run lint
+   npm run lint: fix
    ```
 
 4. Commit your changes using semantic commits

src/interfaces.ts

@@ -1,4 +1,10 @@
 export interface MetricsConfig {
 	defaultLabels?: Record<string, string>;
 	defaultMetricsEnabled?: boolean;
-}
+}
+
+export interface ReporterAsyncOptions {
+	imports?: any[];
+	useFactory: ( ...args: any[] )=> Promise<MetricsConfig> | MetricsConfig;
+	inject?: any[];
+  }

src/reporter/reporter.module.ts

@@ -1,23 +1,15 @@
 import { DynamicModule, Global, Module } from '@nestjs/common';
 import { ReporterService } from './reporter.service';
 import { collectDefaultMetrics, Registry } from 'prom-client';
-import { MetricsConfig } from '../interfaces';
+import { MetricsConfig, ReporterAsyncOptions } from '../interfaces';
 import { MetricsService } from '../metrics/metrics.service';
 import { MetricsController } from '../metrics/metrics.controller';
 
 @Global()
 @Module( {} )
 export class ReporterModule {
 	static forRoot( config: MetricsConfig = {} ): DynamicModule {
-		const registry = new Registry();
-          
-		if ( config.defaultLabels ) {
-			registry.setDefaultLabels( config.defaultLabels );
-		}
-          
-		if ( config.defaultMetricsEnabled === true ) {
-			collectDefaultMetrics( { register: registry } );
-		}
+		const registry = this.configureRegistry( config );
 
 		return {
 			module: ReporterModule,
@@ -34,11 +26,7 @@ export class ReporterModule {
 		};
 	}
 
-	static forRootAsync( options: {
-          imports?: any[];
-          useFactory: ( ...args: any[] ) => Promise<MetricsConfig> | MetricsConfig;
-          inject?: any[];
-     } ): DynamicModule {
+	static forRootAsync( options: ReporterAsyncOptions ): DynamicModule {
 		return {
 			module: ReporterModule,
 			imports: options.imports,
@@ -51,17 +39,7 @@ export class ReporterModule {
 				{
 					provide: Registry,
 					useFactory: async ( config: MetricsConfig ) => {
-						const registry = new Registry();
-                              
-						if ( config.defaultLabels ) {
-							registry.setDefaultLabels( config.defaultLabels );
-						}
-                              
-						if ( config.defaultMetricsEnabled === true ) {
-							collectDefaultMetrics( { register: registry } );
-						}
-                              
-						return registry;
+						return ReporterModule.configureRegistry( config );
 					},
 					inject: [ 'CONFIG_OPTIONS' ],
 				},
@@ -72,4 +50,18 @@ export class ReporterModule {
 			exports: [ ReporterService ]
 		};
 	}
+     
+	private static configureRegistry( config: MetricsConfig = {} ): Registry {
+		const registry = new Registry();
+          
+		if ( config.defaultLabels ) {
+			registry.setDefaultLabels( config.defaultLabels );
+		}
+          
+		if ( config.defaultMetricsEnabled ) {
+			collectDefaultMetrics( { register: registry } );
+		}
+          
+		return registry;
+	}
 }

src/reporter/reporter.service.ts

@@ -7,39 +7,49 @@ export class ReporterService implements OnApplicationBootstrap {
 	private static metricsService: MetricsService;
 
 	constructor( private readonly metrics: MetricsService ) {}
+	
 	onApplicationBootstrap() {
 		ReporterService.metricsService = this.metrics;
 	}
 
-	static counter(key: string, labels?: Record<string, string | number>): void {
+	static counter( key: string, labels?: Record<string, string | number> ): void {
 		try {
 			ReporterService.metricsService.incCounter( key, labels );
 		} catch ( error ) {
-			this.logger.error( `Error while incrementing counter - ${ key }`, error );
+			this.logError( 'increment counter', key, labels, error );
 		}
 	}
 
-	static gauge(key: string, value: number, labels?: Record<string, string | number>): void {
+	static gauge( key: string, value: number, labels?: Record<string, string | number> ): void {
 		try {
 			ReporterService.metricsService.setGauge( key, value, labels );
 		} catch ( error ) {
-			this.logger.error( `Error while setting gauge - ${ key }, ${ value }`, error );
+			this.logError( 'set gauge', key, labels, error );
 		}
 	}
 
-	static histogram(key: string, value: number, labels?: Record<string, string | number>, buckets?: number[]): void {
+	static histogram( key: string, value: number, labels?: Record<string, string | number>, buckets?: number[] ): void {
 		try {
 			ReporterService.metricsService.observeHistogram( key, value, labels, buckets );
 		} catch ( error ) {
-			this.logger.error( `Error while observing histogram - ${ key }, ${ value }`, error );
+			this.logError( 'observe histogram', key, labels, error );
 		}
 	}
 
-	static summary(key: string, value: number, labels?: Record<string, string | number>, percentiles?: number[]): void {
+	static summary( key: string, value: number, labels?: Record<string, string | number>, percentiles?: number[] ): void {
 		try {
 			ReporterService.metricsService.observeSummary( key, value, labels, percentiles );
 		} catch ( error ) {
-			this.logger.error( `Error while observing summary - ${ key }, ${ value }`, error );
+			this.logError( 'observe summary', key, labels, error );
 		}
 	}
+	
+	private static logError( action: string, key: string, labels: Record<string, string | number> | undefined, error: unknown ): void {
+		this.logger.error( {
+			message: `Failed to ${action}`,
+			metric: key,
+			labels,
+			error: error instanceof Error ? error.message : String( error ),
+		} );
+	}
 }

tests/metrics.controller.spec.ts

@@ -2,56 +2,56 @@ import { Test, TestingModule } from '@nestjs/testing';
 import { Registry, Counter } from 'prom-client';
 import { MetricsController } from '../src';
 
-describe('MetricsController', () => {
+describe( 'MetricsController', () => {
 	let controller: MetricsController;
 	let registry: Registry;
 
-	beforeEach(async () => {
+	beforeEach( async () => {
 		registry = new Registry();
-		const module: TestingModule = await Test.createTestingModule({
+		const module: TestingModule = await Test.createTestingModule( {
 			controllers: [MetricsController],
 			providers: [
 				{
 					provide: Registry,
 					useValue: registry,
 				},
 			],
-		}).compile();
+		} ).compile();
 
-		controller = module.get<MetricsController>(MetricsController);
-	});
+		controller = module.get<MetricsController>( MetricsController );
+	} );
 
-	afterEach(() => {
+	afterEach( () => {
 		registry.clear();
-	});
+	} );
 
-	describe('getMetrics', () => {
-		it('should return prometheus metrics', async () => {
-			const counter = new Counter({
+	describe( 'getMetrics', () => {
+		it( 'should return prometheus metrics', async () => {
+			const counter = new Counter( {
 				name: 'test_counter',
 				help: 'test counter help',
 				registers: [registry]
-			});
+			} );
 
 			counter.inc();
 
 			const metrics = await controller.getMetrics();
-			expect(metrics).toBeDefined();
-			expect(typeof metrics).toBe('string');
-			expect(metrics).toContain('# HELP test_counter test counter help');
-			expect(metrics).toContain('test_counter 1');
-		});
+			expect( metrics ).toBeDefined();
+			expect( typeof metrics ).toBe( 'string' );
+			expect( metrics ).toContain( '# HELP test_counter test counter help' );
+			expect( metrics ).toContain( 'test_counter 1' );
+		} );
 
-		it('should handle empty metrics registry', async () => {
+		it( 'should handle empty metrics registry', async () => {
 			const metrics = await controller.getMetrics();
-			expect(metrics).toBeDefined();
-			expect(typeof metrics).toBe('string');
-			expect(metrics.trim()).toBe('');
-		});
+			expect( metrics ).toBeDefined();
+			expect( typeof metrics ).toBe( 'string' );
+			expect( metrics.trim() ).toBe( '' );
+		} );
 
-		it('should handle registry errors', async () => {
-			jest.spyOn(registry, 'metrics').mockRejectedValue(new Error('Registry error'));
-			await expect(controller.getMetrics()).rejects.toThrow('Registry error');
-		});
-	});
-});
+		it( 'should handle registry errors', async () => {
+			jest.spyOn( registry, 'metrics' ).mockRejectedValue( new Error( 'Registry error' ) );
+			await expect( controller.getMetrics() ).rejects.toThrow( 'Registry error' );
+		} );
+	} );
+} );