Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow setting consumer name and group #23

Merged
merged 5 commits into from
Jul 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/common/client.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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) {
Expand All @@ -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();
Expand Down Expand Up @@ -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]);

Expand Down
57 changes: 57 additions & 0 deletions src/common/consumerRegistry.ts
Original file line number Diff line number Diff line change
@@ -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<string, ApitallyConsumer>;
private updated: Set<string>;

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<ApitallyConsumer> = [];
this.updated.forEach((identifier) => {
const consumer = this.consumers.get(identifier);
if (consumer) {
data.push(consumer);
}
});
this.updated.clear();
return data;
}
}
9 changes: 9 additions & 0 deletions src/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -74,11 +80,14 @@ export type ServerErrorsItem = ConsumerMethodPath & {
error_count: number;
};

export type ConsumerItem = ApitallyConsumer;

export type SyncPayload = {
time_offset: number;
instance_uuid: string;
message_uuid: string;
requests: Array<RequestsItem>;
validation_errors: Array<ValidationErrorsItem>;
server_errors: Array<ServerErrorsItem>;
consumers: Array<ConsumerItem>;
};
21 changes: 14 additions & 7 deletions src/express/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,20 @@
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";
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
}
}

Expand Down Expand Up @@ -56,8 +58,9 @@
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,
Expand Down Expand Up @@ -87,7 +90,7 @@
}
validationErrors.forEach((error) => {
client.validationErrorCounter.addValidationError({
consumer: consumer,
consumer: consumer?.identifier,
method: req.method,
path: req.route.path,
...error,
Expand All @@ -97,7 +100,7 @@
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,
Expand Down Expand Up @@ -127,10 +130,14 @@

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);

Check warning on line 140 in src/express/middleware.ts

View check run for this annotation

Codecov / codecov/patch

src/express/middleware.ts#L136-L140

Added lines #L136 - L140 were not covered by tests
}
return null;
};
Expand Down
27 changes: 19 additions & 8 deletions src/fastify/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,14 @@
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 {
Expand All @@ -17,8 +23,8 @@
}

interface FastifyRequest {
apitallyConsumer?: string;
consumerIdentifier?: string; // For backwards compatibility
apitallyConsumer?: ApitallyConsumer | string | null;
consumerIdentifier?: ApitallyConsumer | string | null; // For backwards compatibility
}
}

Expand Down Expand Up @@ -82,8 +88,9 @@
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,
Expand All @@ -100,7 +107,7 @@
const validationErrors = extractAjvErrors(reply.payload.message);
validationErrors.forEach((error) => {
client.validationErrorCounter.addValidationError({
consumer: consumer,
consumer: consumer?.identifier,
method: request.method,
path: path,
...error,
Expand All @@ -109,7 +116,7 @@
}
if (reply.statusCode === 500 && reply.serverError) {
client.serverErrorCounter.addServerError({
consumer: consumer,
consumer: consumer?.identifier,
method: request.method,
path: path,
type: reply.serverError.name,
Expand Down Expand Up @@ -146,10 +153,14 @@

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);

Check warning on line 163 in src/fastify/plugin.ts

View check run for this annotation

Codecov / codecov/patch

src/fastify/plugin.ts#L159-L163

Added lines #L159 - L163 were not covered by tests
}
return null;
};
Expand Down
15 changes: 11 additions & 4 deletions src/koa/middleware.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand All @@ -25,7 +26,7 @@
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,
Expand All @@ -40,8 +41,10 @@
}
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,
Expand All @@ -66,10 +69,14 @@

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);

Check warning on line 79 in src/koa/middleware.ts

View check run for this annotation

Codecov / codecov/patch

src/koa/middleware.ts#L75-L79

Added lines #L75 - L79 were not covered by tests
}
return null;
};
Expand Down
66 changes: 66 additions & 0 deletions tests/common/consumerRegistry.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
1 change: 1 addition & 0 deletions tests/express/app.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down
6 changes: 0 additions & 6 deletions tests/express/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
Loading