From e7642c02aa93c39d0eb43d0a5cbace9e9fc99cd1 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Wed, 10 Jan 2024 14:52:34 +0100 Subject: [PATCH] feat: Added bulk metrics support under /api/client/metrics/bulk path (#5779) This adds a bulk endpoint under `/api/client/metrics`. Accessible under `/api/client/metrics/bulk`. This allows us to piggyback on the need for an API user with access. This PR mostly copies the behaviour from our `/edge/metrics` endpoint, but it filters metrics to only include the environment that the token has access to. So a client token that has access to the `production` will not be allowed to report metrics for the `development` environment. More importantly, a `development` token will not be allowed to post metrics for the `production` environment. --- src/lib/routes/client-api/metrics.test.ts | 153 +++++++++++++++++- src/lib/routes/client-api/metrics.ts | 64 ++++++++ .../client-metrics/metrics-service-v2.ts | 7 + 3 files changed, 221 insertions(+), 3 deletions(-) diff --git a/src/lib/routes/client-api/metrics.test.ts b/src/lib/routes/client-api/metrics.test.ts index 9bcb968252..4e3e7fa4ff 100644 --- a/src/lib/routes/client-api/metrics.test.ts +++ b/src/lib/routes/client-api/metrics.test.ts @@ -2,9 +2,18 @@ import supertest from 'supertest'; import getApp from '../../app'; import { createTestConfig } from '../../../test/config/test-config'; import { clientMetricsSchema } from '../../services/client-metrics/schema'; -import { createServices } from '../../services'; -import { IUnleashOptions, IUnleashServices, IUnleashStores } from '../../types'; +import { ApiTokenService, createServices } from '../../services'; +import { + CLIENT, + IAuthType, + IUnleashOptions, + IUnleashServices, + IUnleashStores, +} from '../../types'; import dbInit from '../../../test/e2e/helpers/database-init'; +import { addDays, subMinutes } from 'date-fns'; +import ApiUser from '../../types/api-user'; +import { ALL, ApiTokenType } from '../../types/models/api-token'; let db; @@ -14,11 +23,11 @@ async function getSetup(opts?: IUnleashOptions) { const services = createServices(db.stores, config, db.rawDatabase); const app = await getApp(config, db.stores, services); - return { request: supertest(app), stores: db.stores, services, + db: db.rawDatabase, destroy: db.destroy, }; } @@ -260,3 +269,141 @@ test('should return 204 if metrics are disabled by feature flag', async () => { }) .expect(204); }); + +describe('bulk metrics', () => { + test('filters out metrics for environments we do not have access for. No auth setup so we can only access default env', async () => { + const timer = new Date().valueOf(); + await request + .post('/api/client/metrics/bulk') + .send({ + applications: [], + metrics: [ + { + featureName: 'test_feature_one', + appName: 'test_application', + environment: 'default', + timestamp: subMinutes(Date.now(), 3), + yes: 1000, + no: 800, + variants: {}, + }, + { + featureName: 'test_feature_two', + appName: 'test_application', + environment: 'development', + timestamp: subMinutes(Date.now(), 3), + yes: 1000, + no: 800, + variants: {}, + }, + ], + }) + .expect(202); + console.log( + `Posting happened ${new Date().valueOf() - timer} ms after`, + ); + await services.clientMetricsServiceV2.bulkAdd(); // Force bulk collection. + console.log( + `Bulk add happened ${new Date().valueOf() - timer} ms after`, + ); + const developmentReport = + await services.clientMetricsServiceV2.getClientMetricsForToggle( + 'test_feature_two', + 1, + ); + console.log( + `Getting for toggle two ${new Date().valueOf() - timer} ms after`, + ); + const defaultReport = + await services.clientMetricsServiceV2.getClientMetricsForToggle( + 'test_feature_one', + 1, + ); + console.log( + `Getting for toggle one ${new Date().valueOf() - timer} ms after`, + ); + expect(developmentReport).toHaveLength(0); + expect(defaultReport).toHaveLength(1); + expect(defaultReport[0].yes).toBe(1000); + }); + + test('should accept empty bulk metrics', async () => { + await request + .post('/api/client/metrics/bulk') + .send({ + applications: [], + metrics: [], + }) + .expect(202); + }); + + test('should validate bulk metrics data', async () => { + await request + .post('/api/client/metrics/bulk') + .send({ randomData: 'blurb' }) + .expect(400); + }); + + test('bulk metrics should return 204 if metrics are disabled', async () => { + const { request: localRequest } = await getSetup({ + experimental: { + flags: { + disableMetrics: true, + }, + }, + }); + + await localRequest + .post('/api/client/metrics/bulk') + .send({ + applications: [], + metrics: [], + }) + .expect(204); + }); + + test('bulk metrics requires a valid client token to accept metrics', async () => { + const authed = await getSetup({ + authentication: { + type: IAuthType.DEMO, + enableApiToken: true, + }, + }); + await authed.db('environments').insert({ + name: 'development', + sort_order: 5000, + type: 'development', + enabled: true, + }); + const clientToken = + await authed.services.apiTokenService.createApiTokenWithProjects({ + tokenName: 'bulk-metrics-test', + type: ApiTokenType.CLIENT, + environment: 'development', + projects: ['*'], + }); + const frontendToken = + await authed.services.apiTokenService.createApiTokenWithProjects({ + tokenName: 'frontend-bulk-metrics-test', + type: ApiTokenType.FRONTEND, + environment: 'development', + projects: ['*'], + }); + + await authed.request + .post('/api/client/metrics/bulk') + .send({ applications: [], metrics: [] }) + .expect(401); + await authed.request + .post('/api/client/metrics/bulk') + .set('Authorization', frontendToken.secret) + .send({ applications: [], metrics: [] }) + .expect(403); + await authed.request + .post('/api/client/metrics/bulk') + .set('Authorization', clientToken.secret) + .send({ applications: [], metrics: [] }) + .expect(202); + await authed.destroy(); + }); +}); diff --git a/src/lib/routes/client-api/metrics.ts b/src/lib/routes/client-api/metrics.ts index 509cf6fb51..2eeeedc848 100644 --- a/src/lib/routes/client-api/metrics.ts +++ b/src/lib/routes/client-api/metrics.ts @@ -14,6 +14,10 @@ import { } from '../../openapi/util/standard-responses'; import rateLimit from 'express-rate-limit'; import { minutesToMilliseconds } from 'date-fns'; +import { BulkMetricsSchema } from '../../openapi/spec/bulk-metrics-schema'; +import { clientMetricsEnvBulkSchema } from '../../services/client-metrics/schema'; +import { IClientMetricsEnv } from '../../types/stores/client-metrics-store-v2'; +import ApiUser from '../../types/api-user'; export default class ClientMetricsController extends Controller { logger: Logger; @@ -75,6 +79,26 @@ export default class ClientMetricsController extends Controller { }), ], }); + + this.route({ + method: 'post', + path: '/bulk', + handler: this.bulkMetrics, + permission: NONE, + middleware: [ + this.openApiService.validPath({ + tags: ['Edge'], + summary: 'Send metrics in bulk', + description: `This operation accepts batched metrics from any client. Metrics will be inserted into Unleash's metrics storage`, + operationId: 'clientBulkMetrics', + requestBody: createRequestSchema('bulkMetricsSchema'), + responses: { + 202: emptyResponse, + ...getStandardResponses(400, 413, 415), + }, + }), + ], + }); } async registerMetrics(req: IAuthRequest, res: Response): Promise { @@ -104,4 +128,44 @@ export default class ClientMetricsController extends Controller { } } } + + async bulkMetrics( + req: IAuthRequest, + res: Response, + ): Promise { + if (this.config.flagResolver.isEnabled('disableMetrics')) { + res.status(204).end(); + } else { + const { body, ip: clientIp } = req; + const { metrics, applications } = body; + try { + const promises: Promise[] = []; + for (const app of applications) { + promises.push( + this.clientInstanceService.registerClient( + app, + clientIp, + ), + ); + } + if (metrics && metrics.length > 0) { + const data: IClientMetricsEnv[] = + await clientMetricsEnvBulkSchema.validateAsync(metrics); + const { user } = req; + const acceptedEnvironment = + this.metricsV2.resolveUserEnvironment(user); + const filteredData = data.filter( + (metric) => metric.environment === acceptedEnvironment, + ); + promises.push( + this.metricsV2.registerBulkMetrics(filteredData), + ); + } + await Promise.all(promises); + res.status(202).end(); + } catch (e) { + res.status(400).end(); + } + } + } } diff --git a/src/lib/services/client-metrics/metrics-service-v2.ts b/src/lib/services/client-metrics/metrics-service-v2.ts index 5288ba0ebb..bf2a854cb1 100644 --- a/src/lib/services/client-metrics/metrics-service-v2.ts +++ b/src/lib/services/client-metrics/metrics-service-v2.ts @@ -245,4 +245,11 @@ export default class ClientMetricsServiceV2 { } return 'default'; } + + resolveUserEnvironment(user: IUser | IApiUser): string { + if (user instanceof ApiUser && user.environment !== ALL) { + return user.environment; + } + return 'default'; + } }