From 906f0daa03bf876170591310ee67d4055851fca2 Mon Sep 17 00:00:00 2001 From: Simon Gurcke Date: Sun, 7 Jul 2024 14:31:34 +1000 Subject: [PATCH] Allow setting consumer name and group (#23) * WIP * Add test * Check for consumer in tests * Fix --- src/common/client.ts | 4 ++ src/common/consumerRegistry.ts | 57 +++++++++++++++++++++++ src/common/types.ts | 9 ++++ src/express/middleware.ts | 21 ++++++--- src/fastify/plugin.ts | 27 +++++++---- src/koa/middleware.ts | 15 ++++-- tests/common/consumerRegistry.test.ts | 66 +++++++++++++++++++++++++++ tests/express/app.test.ts | 1 + tests/express/app.ts | 6 --- tests/fastify/app.test.ts | 1 + tests/fastify/app.ts | 1 - tests/koa/app.test.ts | 1 + tsconfig.json | 2 +- 13 files changed, 184 insertions(+), 27 deletions(-) create mode 100644 src/common/consumerRegistry.ts create mode 100644 tests/common/consumerRegistry.test.ts diff --git a/src/common/client.ts b/src/common/client.ts index db9cdd4..a624e66 100644 --- a/src/common/client.ts +++ b/src/common/client.ts @@ -1,5 +1,6 @@ import { randomUUID } from "crypto"; import fetchRetry from "fetch-retry"; +import ConsumerRegistry from "./consumerRegistry.js"; import { Logger, getLogger } from "./logging.js"; import { isValidClientId, isValidEnv } from "./paramValidation.js"; import RequestCounter from "./requestCounter.js"; @@ -43,6 +44,7 @@ export class ApitallyClient { public requestCounter: RequestCounter; public validationErrorCounter: ValidationErrorCounter; public serverErrorCounter: ServerErrorCounter; + public consumerRegistry: ConsumerRegistry; public logger: Logger; constructor({ clientId, env = "dev", logger }: ApitallyConfig) { @@ -68,6 +70,7 @@ export class ApitallyClient { this.requestCounter = new RequestCounter(); this.validationErrorCounter = new ValidationErrorCounter(); this.serverErrorCounter = new ServerErrorCounter(); + this.consumerRegistry = new ConsumerRegistry(); this.logger = logger || getLogger(); this.startSync(); @@ -190,6 +193,7 @@ export class ApitallyClient { validation_errors: this.validationErrorCounter.getAndResetValidationErrors(), server_errors: this.serverErrorCounter.getAndResetServerErrors(), + consumers: this.consumerRegistry.getAndResetUpdatedConsumers(), }; this.syncDataQueue.push([Date.now(), newPayload]); diff --git a/src/common/consumerRegistry.ts b/src/common/consumerRegistry.ts new file mode 100644 index 0000000..849fd76 --- /dev/null +++ b/src/common/consumerRegistry.ts @@ -0,0 +1,57 @@ +import { ApitallyConsumer } from "./types.js"; + +export const consumerFromStringOrObject = ( + consumer: ApitallyConsumer | string, +) => { + if (typeof consumer === "string") { + consumer = String(consumer).trim().substring(0, 128); + return consumer ? { identifier: consumer } : null; + } else { + consumer.identifier = String(consumer.identifier).trim().substring(0, 128); + consumer.name = consumer.name?.trim().substring(0, 64); + consumer.group = consumer.group?.trim().substring(0, 64); + return consumer.identifier ? consumer : null; + } +}; + +export default class ConsumerRegistry { + private consumers: Map; + private updated: Set; + + constructor() { + this.consumers = new Map(); + this.updated = new Set(); + } + + public addOrUpdateConsumer(consumer?: ApitallyConsumer | null) { + if (!consumer || (!consumer.name && !consumer.group)) { + return; + } + const existing = this.consumers.get(consumer.identifier); + if (!existing) { + this.consumers.set(consumer.identifier, consumer); + this.updated.add(consumer.identifier); + } else { + if (consumer.name && consumer.name !== existing.name) { + existing.name = consumer.name; + this.updated.add(consumer.identifier); + } + if (consumer.group && consumer.group !== existing.group) { + existing.group = consumer.group; + this.updated.add(consumer.identifier); + } + } + } + + public getAndResetUpdatedConsumers() { + const data: Array = []; + this.updated.forEach((identifier) => { + const consumer = this.consumers.get(identifier); + if (consumer) { + data.push(consumer); + } + }); + this.updated.clear(); + return data; + } +} diff --git a/src/common/types.ts b/src/common/types.ts index 07eb54f..2af43d4 100644 --- a/src/common/types.ts +++ b/src/common/types.ts @@ -8,6 +8,12 @@ export type ApitallyConfig = { logger?: Logger; }; +export type ApitallyConsumer = { + identifier: string; + name?: string | null; + group?: string | null; +}; + export type PathInfo = { method: string; path: string; @@ -74,6 +80,8 @@ export type ServerErrorsItem = ConsumerMethodPath & { error_count: number; }; +export type ConsumerItem = ApitallyConsumer; + export type SyncPayload = { time_offset: number; instance_uuid: string; @@ -81,4 +89,5 @@ export type SyncPayload = { requests: Array; validation_errors: Array; server_errors: Array; + consumers: Array; }; diff --git a/src/express/middleware.ts b/src/express/middleware.ts index 45f49ee..3a3bef0 100644 --- a/src/express/middleware.ts +++ b/src/express/middleware.ts @@ -2,9 +2,11 @@ import type { Express, NextFunction, Request, Response } from "express"; import { performance } from "perf_hooks"; import { ApitallyClient } from "../common/client.js"; +import { consumerFromStringOrObject } from "../common/consumerRegistry.js"; import { getPackageVersion } from "../common/packageVersions.js"; import { ApitallyConfig, + ApitallyConsumer, StartupData, ValidationError, } from "../common/types.js"; @@ -12,8 +14,8 @@ import listEndpoints from "./listEndpoints.js"; declare module "express" { interface Request { - apitallyConsumer?: string; - consumerIdentifier?: string; // For backwards compatibility + apitallyConsumer?: ApitallyConsumer | string | null; + consumerIdentifier?: ApitallyConsumer | string | null; // For backwards compatibility } } @@ -56,8 +58,9 @@ const getMiddleware = (app: Express, client: ApitallyClient) => { if (req.route) { const responseTime = performance.now() - startTime; const consumer = getConsumer(req); + client.consumerRegistry.addOrUpdateConsumer(consumer); client.requestCounter.addRequest({ - consumer: consumer, + consumer: consumer?.identifier, method: req.method, path: req.route.path, statusCode: res.statusCode, @@ -87,7 +90,7 @@ const getMiddleware = (app: Express, client: ApitallyClient) => { } validationErrors.forEach((error) => { client.validationErrorCounter.addValidationError({ - consumer: consumer, + consumer: consumer?.identifier, method: req.method, path: req.route.path, ...error, @@ -97,7 +100,7 @@ const getMiddleware = (app: Express, client: ApitallyClient) => { if (res.statusCode === 500 && res.locals.serverError) { const serverError = res.locals.serverError as Error; client.serverErrorCounter.addServerError({ - consumer: consumer, + consumer: consumer?.identifier, method: req.method, path: req.route.path, type: serverError.name, @@ -127,10 +130,14 @@ const getMiddleware = (app: Express, client: ApitallyClient) => { const getConsumer = (req: Request) => { if (req.apitallyConsumer) { - return String(req.apitallyConsumer); + return consumerFromStringOrObject(req.apitallyConsumer); } else if (req.consumerIdentifier) { // For backwards compatibility - return String(req.consumerIdentifier); + process.emitWarning( + "The consumerIdentifier property on the request object is deprecated. Use apitallyConsumer instead.", + "DeprecationWarning", + ); + return consumerFromStringOrObject(req.consumerIdentifier); } return null; }; diff --git a/src/fastify/plugin.ts b/src/fastify/plugin.ts index 7a05f96..d316f9a 100644 --- a/src/fastify/plugin.ts +++ b/src/fastify/plugin.ts @@ -7,8 +7,14 @@ import type { import fp from "fastify-plugin"; import { ApitallyClient } from "../common/client.js"; +import { consumerFromStringOrObject } from "../common/consumerRegistry.js"; import { getPackageVersion } from "../common/packageVersions.js"; -import { ApitallyConfig, PathInfo, ValidationError } from "../common/types.js"; +import { + ApitallyConfig, + ApitallyConsumer, + PathInfo, + ValidationError, +} from "../common/types.js"; declare module "fastify" { interface FastifyReply { @@ -17,8 +23,8 @@ declare module "fastify" { } interface FastifyRequest { - apitallyConsumer?: string; - consumerIdentifier?: string; // For backwards compatibility + apitallyConsumer?: ApitallyConsumer | string | null; + consumerIdentifier?: ApitallyConsumer | string | null; // For backwards compatibility } } @@ -82,8 +88,9 @@ const apitallyPlugin: FastifyPluginAsync = async ( if (Array.isArray(responseSize)) { responseSize = responseSize[0]; } + client.consumerRegistry.addOrUpdateConsumer(consumer); client.requestCounter.addRequest({ - consumer: consumer, + consumer: consumer?.identifier, method: request.method, path: path, statusCode: reply.statusCode, @@ -100,7 +107,7 @@ const apitallyPlugin: FastifyPluginAsync = async ( const validationErrors = extractAjvErrors(reply.payload.message); validationErrors.forEach((error) => { client.validationErrorCounter.addValidationError({ - consumer: consumer, + consumer: consumer?.identifier, method: request.method, path: path, ...error, @@ -109,7 +116,7 @@ const apitallyPlugin: FastifyPluginAsync = async ( } if (reply.statusCode === 500 && reply.serverError) { client.serverErrorCounter.addServerError({ - consumer: consumer, + consumer: consumer?.identifier, method: request.method, path: path, type: reply.serverError.name, @@ -146,10 +153,14 @@ const getAppInfo = (routes: PathInfo[], appVersion?: string) => { const getConsumer = (request: FastifyRequest) => { if (request.apitallyConsumer) { - return String(request.apitallyConsumer); + return consumerFromStringOrObject(request.apitallyConsumer); } else if (request.consumerIdentifier) { // For backwards compatibility - return String(request.consumerIdentifier); + process.emitWarning( + "The consumerIdentifier property on the request object is deprecated. Use apitallyConsumer instead.", + "DeprecationWarning", + ); + return consumerFromStringOrObject(request.consumerIdentifier); } return null; }; diff --git a/src/koa/middleware.ts b/src/koa/middleware.ts index 0be8f1b..44739f6 100644 --- a/src/koa/middleware.ts +++ b/src/koa/middleware.ts @@ -1,6 +1,7 @@ import Koa from "koa"; import { ApitallyClient } from "../common/client.js"; +import { consumerFromStringOrObject } from "../common/consumerRegistry.js"; import { getPackageVersion } from "../common/packageVersions.js"; import { ApitallyConfig, PathInfo, StartupData } from "../common/types.js"; @@ -25,7 +26,7 @@ const getMiddleware = (client: ApitallyClient) => { statusCode = error.statusCode || error.status || 500; if (path && statusCode === 500 && error instanceof Error) { client.serverErrorCounter.addServerError({ - consumer: getConsumer(ctx), + consumer: getConsumer(ctx)?.identifier, method: ctx.request.method, path, type: error.name, @@ -40,8 +41,10 @@ const getMiddleware = (client: ApitallyClient) => { } if (path) { try { + const consumer = getConsumer(ctx); + client.consumerRegistry.addOrUpdateConsumer(consumer); client.requestCounter.addRequest({ - consumer: getConsumer(ctx), + consumer: consumer?.identifier, method: ctx.request.method, path, statusCode: statusCode || ctx.response.status, @@ -66,10 +69,14 @@ const getPath = (ctx: Koa.Context) => { const getConsumer = (ctx: Koa.Context) => { if (ctx.state.apitallyConsumer) { - return String(ctx.state.apitallyConsumer); + return consumerFromStringOrObject(ctx.state.apitallyConsumer); } else if (ctx.state.consumerIdentifier) { // For backwards compatibility - return String(ctx.state.consumerIdentifier); + process.emitWarning( + "The consumerIdentifier property on the ctx.state object is deprecated. Use apitallyConsumer instead.", + "DeprecationWarning", + ); + return consumerFromStringOrObject(ctx.state.consumerIdentifier); } return null; }; diff --git a/tests/common/consumerRegistry.test.ts b/tests/common/consumerRegistry.test.ts new file mode 100644 index 0000000..20fa002 --- /dev/null +++ b/tests/common/consumerRegistry.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from "vitest"; + +import ConsumerRegistry, { + consumerFromStringOrObject, +} from "../../src/common/consumerRegistry.js"; + +describe("Consumer registry", () => { + it("Consumer from string or object", () => { + let consumer = consumerFromStringOrObject(""); + expect(consumer).toBeNull(); + + consumer = consumerFromStringOrObject({ identifier: " " }); + expect(consumer).toBeNull(); + + consumer = consumerFromStringOrObject("test"); + expect(consumer).toEqual({ + identifier: "test", + }); + + consumer = consumerFromStringOrObject({ identifier: "test" }); + expect(consumer).toEqual({ + identifier: "test", + }); + + consumer = consumerFromStringOrObject({ + identifier: "test", + name: "Test ", + group: " Testers ", + }); + expect(consumer).toEqual({ + identifier: "test", + name: "Test", + group: "Testers", + }); + }); + + it("Add or update consumers", () => { + const consumerRegistry = new ConsumerRegistry(); + consumerRegistry.addOrUpdateConsumer(null); + consumerRegistry.addOrUpdateConsumer({ identifier: "test" }); + let data = consumerRegistry.getAndResetUpdatedConsumers(); + expect(data.length).toBe(0); + + const testConsumer = { + identifier: "test", + name: "Test", + group: "Testers", + }; + consumerRegistry.addOrUpdateConsumer(testConsumer); + data = consumerRegistry.getAndResetUpdatedConsumers(); + expect(data.length).toBe(1); + expect(data[0]).toEqual(testConsumer); + + consumerRegistry.addOrUpdateConsumer(testConsumer); + data = consumerRegistry.getAndResetUpdatedConsumers(); + expect(data.length).toBe(0); + + consumerRegistry.addOrUpdateConsumer({ + identifier: "test", + name: "Test 2", + group: "Testers 2", + }); + data = consumerRegistry.getAndResetUpdatedConsumers(); + expect(data.length).toBe(1); + }); +}); diff --git a/tests/express/app.test.ts b/tests/express/app.test.ts index 4a918bd..d3d770b 100644 --- a/tests/express/app.test.ts +++ b/tests/express/app.test.ts @@ -46,6 +46,7 @@ testCases.forEach(({ name, getApp }) => { expect( requests.some( (r) => + r.consumer === "test" && r.method === "GET" && r.path === "/hello" && r.status_code === 200 && diff --git a/tests/express/app.ts b/tests/express/app.ts index 9cd13ca..eac64d4 100644 --- a/tests/express/app.ts +++ b/tests/express/app.ts @@ -6,12 +6,6 @@ import { body, query, validationResult } from "express-validator"; import { useApitally } from "../../src/express/index.js"; import { CLIENT_ID, ENV } from "../utils.js"; -declare module "express" { - interface Request { - apitallyConsumer?: string; - } -} - export const getAppWithCelebrate = () => { const app = express(); diff --git a/tests/fastify/app.test.ts b/tests/fastify/app.test.ts index 4b4f7fc..de8a273 100644 --- a/tests/fastify/app.test.ts +++ b/tests/fastify/app.test.ts @@ -35,6 +35,7 @@ describe("Plugin for Fastify", () => { expect( requests.some( (r) => + r.consumer === "test" && r.method === "GET" && r.path === "/hello" && r.status_code === 200 && diff --git a/tests/fastify/app.ts b/tests/fastify/app.ts index 1957734..05df432 100644 --- a/tests/fastify/app.ts +++ b/tests/fastify/app.ts @@ -6,7 +6,6 @@ import { CLIENT_ID, ENV } from "../utils.js"; export const getApp = async () => { const app = Fastify({ ajv: { customOptions: { allErrors: true } }, - // logger: { level: "error" }, }); await app.register(apitallyPlugin, { diff --git a/tests/koa/app.test.ts b/tests/koa/app.test.ts index cee6549..51f6272 100644 --- a/tests/koa/app.test.ts +++ b/tests/koa/app.test.ts @@ -53,6 +53,7 @@ testCases.forEach(({ name, router, getApp }) => { expect( requests.some( (r) => + r.consumer === "test" && r.method === "GET" && r.path === "/hello" && r.status_code === 200 && diff --git a/tsconfig.json b/tsconfig.json index 29fa225..9104f0a 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,5 +1,5 @@ { - "include": ["src"], + "include": ["src", "tests"], "compilerOptions": { "module": "NodeNext", "esModuleInterop": true,