Merge pull request #891 from myrotvorets/wip

Add more observability; remove docker healthcheck

Dockerfile

@@ -26,8 +26,6 @@ RUN \
     install -d -o nobody -g nobody /usr/share/GeoIP && \
     wget https://cdn.myrotvorets.center/m/geoip/GeoIP2-City.mmdb.enc?_=20220530 -U "Mozilla/5.0" -O /usr/share/GeoIP/GeoIP2-City.mmdb.enc && \
     wget https://cdn.myrotvorets.center/m/geoip/GeoIP2-ISP.mmdb.enc?_=20220530 -U "Mozilla/5.0" -O /usr/share/GeoIP/GeoIP2-ISP.mmdb.enc
-COPY healthcheck.sh entrypoint.sh /usr/local/bin/
-HEALTHCHECK --interval=60s --timeout=10s --start-period=5s --retries=3 CMD ["/usr/local/bin/healthcheck.sh"]
 USER nobody:nobody
 ENTRYPOINT ["/usr/local/bin/entrypoint.sh"]
 COPY --chown=nobody:nobody ./src/specs ./specs

mocha.setup.mjs

@@ -4,10 +4,19 @@ import { reset } from 'testdouble';
 
 use(chaiAsPromised);
 
+const env = { ...process.env };
+process.env = {
+    NODE_ENV: 'test',
+    OTEL_SDK_DISABLED: 'true',
+};
+
 /** @type {import('mocha').RootHookObject} */
 export const mochaHooks = {
     /** @returns {void} */
     afterEach() {
         reset();
     },
+    afterAll() {
+        process.env = { ...env };
+    },
 };

package.json

@@ -25,6 +25,7 @@
     "@myrotvorets/express-request-logger": "^1.1.0",
     "@myrotvorets/oav-installer": "^4.1.0",
     "@myrotvorets/opentelemetry-configurator": "^6.4.2",
+    "@opentelemetry/api": "^1.6.0",
     "awilix": "^8.0.1",
     "envalid": "^8.0.0",
     "express": "^4.18.2",

src/controllers/geoip.mts

@@ -1,6 +1,6 @@
 import { type NextFunction, type Request, type Response, Router } from 'express';
-import { type GeoResponse } from '../services/geoip.mjs';
-import { type LocalsWithContainer } from '../lib/container.mjs';
+import type { GeoResponse } from '../services/geoip.mjs';
+import type { LocalsWithContainer } from '../lib/container.mjs';
 
 interface GeolocateParams {
     ip: string;

src/lib/container.mts

@@ -8,6 +8,7 @@ export interface Container {
     geoIPService: GeoIPService;
     environment: ReturnType<typeof environment>;
     logger: ReturnType<(typeof configurator)['logger']>;
+    meter: ReturnType<(typeof configurator)['meter']>;
 }
 
 export interface RequestContainer {
@@ -29,10 +30,14 @@ function createLogger({ req }: RequestContainer): ReturnType<(typeof configurato
     return logger;
 }
 
-function createGeoIPService({ environment: env }: Container): GeoIPService {
-    const service = new GeoIPService();
-    service.setCityDatabase(env.GEOIP_CITY_FILE);
-    service.setISPDatabase(env.GEOIP_ISP_FILE);
+function createMeter(): ReturnType<(typeof configurator)['meter']> {
+    return configurator.meter();
+}
+
+function createGeoIPService({ environment, meter }: Container): GeoIPService {
+    const service = new GeoIPService({ meter });
+    service.setCityDatabase(environment.GEOIP_CITY_FILE);
+    service.setISPDatabase(environment.GEOIP_ISP_FILE);
     return service;
 }
 
@@ -43,6 +48,7 @@ export function initializeContainer(): typeof container {
         geoIPService: asFunction(createGeoIPService).singleton(),
         environment: asFunction(createEnvironment).singleton(),
         logger: asFunction(createLogger).scoped(),
+        meter: asFunction(createMeter).singleton(),
     });
 
     return container;

src/lib/otel.mts

@@ -9,9 +9,6 @@ if (!+(process.env.ENABLE_TRACING || 0)) {
     process.env.OTEL_SDK_DISABLED = 'true';
 }
 
-process.env.OTEL_LOG_LEVEL = 'info';
-process.env.OTEL_METRICS_EXPORTER = 'otlp';
-
 export const configurator = new OpenTelemetryConfigurator({
     serviceName: 'psb-api-ipgeo',
     instrumentations: [...getExpressInstrumentations(), getFsInstrumentation(true)],

src/lib/utils.mts

@@ -0,0 +1,9 @@
+import { hrtime } from 'node:process';
+
+export function observe<TArgs extends unknown[]>(what: (...args: TArgs) => void, ...args: TArgs): number {
+    const start = hrtime.bigint();
+    what(...args);
+    const end = hrtime.bigint();
+
+    return Number((end - start) / 1000n);
+}

src/middleware/logger.mts

@@ -1,5 +1,5 @@
 import { requestLogger } from '@myrotvorets/express-request-logger';
-import { type LocalsWithContainer } from '../lib/container.mjs';
+import type { LocalsWithContainer } from '../lib/container.mjs';
 
 export const loggerMiddleware = requestLogger<never, never, never, never, LocalsWithContainer>({
     format: '[PSBAPI-ipgeo] :req[X-Request-ID]\t:method\t:url\t:status :res[content-length]\t:date[iso]\t:total-time',

src/services/geoip.mts

@@ -1,5 +1,7 @@
 import { readFileSync } from 'node:fs';
+import { type Histogram, type Meter, ValueType } from '@opentelemetry/api';
 import { type CityResponse, type IspResponse, Reader, type Response } from 'maxmind';
+import { observe } from '../lib/utils.mjs';
 
 export interface GeoCityResponse {
     cc: string | null;
@@ -22,10 +24,35 @@ export interface GeoPrefixes {
 
 export type GeoResponse = GeoCityResponse & GeoIspResponse & GeoPrefixes;
 
+type ConstructorParams = {
+    meter: Meter;
+};
+
 export class GeoIPService {
     private _city: Reader<CityResponse> | undefined;
     private _isp: Reader<IspResponse> | undefined;
 
+    private static _cityLookupHistogram: Histogram;
+    private static _ispLookupHistogram: Histogram;
+
+    public constructor({ meter }: ConstructorParams) {
+        if (!GeoIPService._cityLookupHistogram) {
+            GeoIPService._cityLookupHistogram = meter.createHistogram('geolocate.city.duration', {
+                description: 'Measures the duration of city lookups.',
+                unit: 'us',
+                valueType: ValueType.DOUBLE,
+            });
+        }
+
+        if (!GeoIPService._ispLookupHistogram) {
+            GeoIPService._ispLookupHistogram = meter.createHistogram('geolocate.isp.duration', {
+                description: 'Measures the duration of ISP lookups.',
+                unit: 'us',
+                valueType: ValueType.DOUBLE,
+            });
+        }
+    }
+
     public setCityDatabase(file: string): boolean {
         this._city = GeoIPService.readDatabaseSync(file);
         return this._city !== undefined;
@@ -50,8 +77,26 @@ export class GeoIPService {
     }
 
     public geolocate(ip: string): GeoResponse {
-        const [city, cprefix] = this._city ? this._city.getWithPrefixLength(ip) : [{} as CityResponse, 0];
-        const [isp, iprefix] = this._isp ? this._isp.getWithPrefixLength(ip) : [{} as IspResponse, 0];
+        let city: CityResponse | null = null;
+        let isp: IspResponse | null = null;
+        let cprefix = 0;
+        let iprefix = 0;
+
+        if (this._city) {
+            GeoIPService._cityLookupHistogram.record(
+                observe((reader: Reader<CityResponse>) => {
+                    [city, cprefix] = reader.getWithPrefixLength(ip);
+                }, this._city),
+            );
+        }
+
+        if (this._isp) {
+            GeoIPService._ispLookupHistogram.record(
+                observe((reader: Reader<IspResponse>) => {
+                    [isp, iprefix] = reader.getWithPrefixLength(ip);
+                }, this._isp),
+            );
+        }
 
         return {
             cprefix,

test/functional/controller/geoip.test.mts

@@ -1,7 +1,7 @@
 import { dirname, join } from 'node:path';
 import { fileURLToPath } from 'node:url';
 import { expect } from 'chai';
-import { type Express } from 'express';
+import type { Express } from 'express';
 import request from 'supertest';
 import type { ErrorResponse } from '@myrotvorets/express-microservice-middlewares';
 import { configureApp, createApp } from '../../../src/server.mjs';

test/functional/controller/monitoring.test.mts

@@ -1,5 +1,5 @@
 import { expect } from 'chai';
-import { type Express } from 'express';
+import type { Express } from 'express';
 import request from 'supertest';
 import type { HealthChecker } from '@cloudnative/health-connect';
 import { healthChecker, monitoringController } from '../../../src/controllers/monitoring.mjs';

test/unit/lib/utils.test.mts

@@ -0,0 +1,19 @@
+import { expect } from 'chai';
+import { observe } from '../../../src/lib/utils.mjs';
+
+describe('utils', function () {
+    describe('observe', function () {
+        it('should call the provided function with the provided arguments', function () {
+            const args = ['hello', 42, true];
+            let called = false;
+            const fn = (...fnArgs: typeof args): void => {
+                expect(fnArgs).to.deep.equal(args);
+                called = true;
+            };
+
+            const result = observe(fn, ...args);
+            expect(result).to.be.a('number');
+            expect(called).to.be.true;
+        });
+    });
+});

test/unit/services/geoip.test.mts

@@ -1,6 +1,7 @@
 import { TestDouble, func, matchers, replaceEsm, when } from 'testdouble';
 import { expect } from 'chai';
 import type { Reader } from 'maxmind';
+import { metrics } from '@opentelemetry/api';
 import type { GeoIPService } from '../../../src/services/geoip.mjs';
 import {
     cityResponseWithCountry,
@@ -44,7 +45,9 @@ describe('GeoIPService', function () {
         when(readFileSyncMock(matchers.isA(String) as string)).thenReturn(Buffer.from(''));
 
         geoip = await import('../../../src/services/geoip.mjs');
-        service = new geoip.GeoIPService();
+        service = new geoip.GeoIPService({
+            meter: metrics.getMeter('test'),
+        });
     });
 
     describe('setCityDatabase()', function () {