Skip to content

Commit

Permalink
feat: Added bulk metrics support under /api/client/metrics/bulk path (#…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
Christopher Kolstad committed Jan 10, 2024
1 parent dcf539f commit e7642c0
Show file tree
Hide file tree
Showing 3 changed files with 221 additions and 3 deletions.
153 changes: 150 additions & 3 deletions src/lib/routes/client-api/metrics.test.ts
Expand Up @@ -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;

Expand All @@ -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,
};
}
Expand Down Expand Up @@ -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();
});
});
64 changes: 64 additions & 0 deletions src/lib/routes/client-api/metrics.ts
Expand Up @@ -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;
Expand Down Expand Up @@ -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<void> {
Expand Down Expand Up @@ -104,4 +128,44 @@ export default class ClientMetricsController extends Controller {
}
}
}

async bulkMetrics(
req: IAuthRequest<void, void, BulkMetricsSchema>,
res: Response<void>,
): Promise<void> {
if (this.config.flagResolver.isEnabled('disableMetrics')) {
res.status(204).end();
} else {
const { body, ip: clientIp } = req;
const { metrics, applications } = body;
try {
const promises: Promise<void>[] = [];
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();
}
}
}
}
7 changes: 7 additions & 0 deletions src/lib/services/client-metrics/metrics-service-v2.ts
Expand Up @@ -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';
}
}

0 comments on commit e7642c0

Please sign in to comment.