Skip to content

Commit

Permalink
fix: Change format for bulk metrics. (#3229)
Browse files Browse the repository at this point in the history
### What
Change /edge/metrics endpoint to accept list of ClientMetricsEnv

### Rationale
We originally made the assumption that we probably didn't need to keep
splitting from a map of features into ClientMetricsEnv for bulk, instead
the bulk poster could post ClientMetricsEnv directly. However, Unleash
still expected the old client metrics format with a dictionary of
featurename -> metricsForFeature. This PR changes that to now accept the
list of ClientMetricsEnv (preprocessed data from downstream) instead of
expecting metrics to be in the old single application metric format.
  • Loading branch information
Christopher Kolstad committed Mar 1, 2023
1 parent 2fece60 commit f4bdd89
Show file tree
Hide file tree
Showing 8 changed files with 124 additions and 18 deletions.
2 changes: 2 additions & 0 deletions src/lib/openapi/index.ts
Expand Up @@ -140,6 +140,7 @@ import apiVersion from '../util/version';
import { maintenanceSchema } from './spec/maintenance-schema';
import { bulkRegistrationSchema } from './spec/bulk-registration-schema';
import { bulkMetricsSchema } from './spec/bulk-metrics-schema';
import { clientMetricsEnvSchema } from './spec/client-metrics-env-schema';

// All schemas in `openapi/spec` should be listed here.
export const schemas = {
Expand All @@ -160,6 +161,7 @@ export const schemas = {
clientFeaturesQuerySchema,
clientFeaturesSchema,
clientMetricsSchema,
clientMetricsEnvSchema,
cloneFeatureSchema,
constraintSchema,
contextFieldSchema,
Expand Down
7 changes: 3 additions & 4 deletions src/lib/openapi/spec/bulk-metrics-schema.ts
@@ -1,7 +1,7 @@
import { FromSchema } from 'json-schema-to-ts';
import { bulkRegistrationSchema } from './bulk-registration-schema';
import { clientMetricsSchema } from './client-metrics-schema';
import { dateSchema } from './date-schema';
import { clientMetricsEnvSchema } from './client-metrics-env-schema';

export const bulkMetricsSchema = {
$id: '#/components/schemas/bulkMetricsSchema',
Expand All @@ -16,17 +16,16 @@ export const bulkMetricsSchema = {
metrics: {
type: 'array',
items: {
$ref: '#/components/schemas/clientMetricsSchema',
$ref: '#/components/schemas/clientMetricsEnvSchema',
},
},
},
components: {
schemas: {
bulkRegistrationSchema,
dateSchema,
clientMetricsSchema,
clientMetricsEnvSchema,
},
},
} as const;

export type BulkMetricsSchema = FromSchema<typeof bulkMetricsSchema>;
43 changes: 43 additions & 0 deletions src/lib/openapi/spec/client-metrics-env-schema.ts
@@ -0,0 +1,43 @@
import { dateSchema } from './date-schema';
import { FromSchema } from 'json-schema-to-ts';

export const clientMetricsEnvSchema = {
$id: '#/components/schemas/clientMetricsEnvSchema',
type: 'object',
required: ['featureName', 'appName'],
additionalProperties: true,
properties: {
featureName: {
type: 'string',
},
appName: {
type: 'string',
},
environment: {
type: 'string',
},
timestamp: {
$ref: '#/components/schemas/dateSchema',
},
yes: {
type: 'number',
},
no: {
type: 'number',
},
variants: {
type: 'object',
additionalProperties: {
type: 'integer',
minimum: 0,
},
},
},
components: {
schemas: {
dateSchema,
},
},
} as const;

export type ClientMetricsSchema = FromSchema<typeof clientMetricsEnvSchema>;
1 change: 0 additions & 1 deletion src/lib/routes/client-api/feature.ts
Expand Up @@ -230,7 +230,6 @@ export default class FeatureController extends Controller {
const featureQuery = await this.resolveQuery(req);
const q = { ...featureQuery, namePrefix: name };
const toggles = await this.featureToggleServiceV2.getClientFeatures(q);

const toggle = toggles.find((t) => t.name === name);
if (!toggle) {
throw new NotFoundError(`Could not find feature toggle ${name}`);
Expand Down
12 changes: 6 additions & 6 deletions src/lib/routes/edge-api/index.ts
Expand Up @@ -16,6 +16,7 @@ import { OpenApiService } from '../../services/openapi-service';
import { emptyResponse } from '../../openapi/util/standard-responses';
import { BulkMetricsSchema } from '../../openapi/spec/bulk-metrics-schema';
import ClientMetricsServiceV2 from '../../services/client-metrics/metrics-service-v2';
import { clientMetricsEnvBulkSchema } from '../../services/client-metrics/schema';

export default class EdgeController extends Controller {
private readonly logger: Logger;
Expand Down Expand Up @@ -116,12 +117,11 @@ export default class EdgeController extends Controller {
this.clientInstanceService.registerClient(app, clientIp),
);
}
if (metrics) {
for (const metric of metrics) {
promises.push(
this.metricsV2.registerClientMetrics(metric, clientIp),
);
}
if (metrics && metrics.length > 0) {
const data = await clientMetricsEnvBulkSchema.validateAsync(
metrics,
);
promises.push(this.metricsV2.registerBulkMetrics(data));
}
await Promise.all(promises);
res.status(202).end();
Expand Down
15 changes: 9 additions & 6 deletions src/lib/services/client-metrics/metrics-service-v2.ts
Expand Up @@ -60,6 +60,14 @@ export default class ClientMetricsServiceV2 {
);
}

async registerBulkMetrics(metrics: IClientMetricsEnv[]): Promise<void> {
this.unsavedMetrics = collapseHourlyMetrics([
...this.unsavedMetrics,
...metrics,
]);
this.lastSeenService.updateLastSeen(metrics);
}

async registerClientMetrics(
data: ClientMetricsSchema,
clientIp: string,
Expand All @@ -83,12 +91,7 @@ export default class ClientMetricsServiceV2 {
yes: value.bucket.toggles[name].yes,
no: value.bucket.toggles[name].no,
}));

this.unsavedMetrics = collapseHourlyMetrics([
...this.unsavedMetrics,
...clientMetrics,
]);
this.lastSeenService.updateLastSeen(clientMetrics);
await this.registerBulkMetrics(clientMetrics);
this.config.eventBus.emit(CLIENT_METRICS, value);
}

Expand Down
25 changes: 25 additions & 0 deletions src/lib/services/client-metrics/schema.ts
Expand Up @@ -35,6 +35,23 @@ export const clientMetricsSchema = joi
}),
});

export const clientMetricsEnvSchema = joi
.object()
.options({ stripUnknown: true })
.keys({
featureName: joi.string().required(),
environment: joi.string().required(),
appName: joi.string().required(),
yes: joi.number().default(0),
no: joi.number().default(0),
timestamp: joi.date(),
variants: joi.object().pattern(joi.string(), joi.number().min(0)),
});
export const clientMetricsEnvBulkSchema = joi
.array()
.items(clientMetricsEnvSchema)
.empty();

export const applicationSchema = joi
.object()
.options({ stripUnknown: false })
Expand All @@ -52,6 +69,14 @@ export const applicationSchema = joi
announced: joi.boolean().optional().default(false),
});

export const batchMetricsSchema = joi
.object()
.options({ stripUnknown: true })
.keys({
applications: joi.array().items(applicationSchema),
metrics: joi.array().items(clientMetricsEnvSchema),
});

export const clientRegisterSchema = joi
.object()
.options({ stripUnknown: true })
Expand Down
37 changes: 36 additions & 1 deletion src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap
Expand Up @@ -338,7 +338,7 @@ exports[`should serve the OpenAPI spec 1`] = `
},
"metrics": {
"items": {
"$ref": "#/components/schemas/clientMetricsSchema",
"$ref": "#/components/schemas/clientMetricsEnvSchema",
},
"type": "array",
},
Expand Down Expand Up @@ -568,6 +568,41 @@ exports[`should serve the OpenAPI spec 1`] = `
],
"type": "object",
},
"clientMetricsEnvSchema": {
"additionalProperties": true,
"properties": {
"appName": {
"type": "string",
},
"environment": {
"type": "string",
},
"featureName": {
"type": "string",
},
"no": {
"type": "number",
},
"timestamp": {
"$ref": "#/components/schemas/dateSchema",
},
"variants": {
"additionalProperties": {
"minimum": 0,
"type": "integer",
},
"type": "object",
},
"yes": {
"type": "number",
},
},
"required": [
"featureName",
"appName",
],
"type": "object",
},
"clientMetricsSchema": {
"properties": {
"appName": {
Expand Down

0 comments on commit f4bdd89

Please sign in to comment.