mirror of
https://github.com/Unleash/unleash.git
synced 2025-07-26 13:48:33 +02:00
chore: emit CLIENT_METRICS event after sifting (#10376)
https://linear.app/unleash/issue/2-3705/only-emit-client-metrics-after-sifting-metrics Only emits the `CLIENT_METRICS` event after metric sifting. This ensures the event is emitted only for valid metrics tied to known flags, instead of all flags included in the metrics payload. See: https://github.com/Unleash/unleash/pull/10375#discussion_r2218974109
This commit is contained in:
parent
8f0c0ccef3
commit
51f8244a5d
@ -113,7 +113,7 @@ test('process metrics properly even when some names are not url friendly, filter
|
||||
);
|
||||
|
||||
// only toggle with a bad name gets filtered out
|
||||
expect(eventBus.emit).toHaveBeenCalledTimes(1);
|
||||
expect(eventBus.emit).not.toHaveBeenCalled();
|
||||
expect(lastSeenService.updateLastSeen).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
@ -251,6 +251,7 @@ export default class ClientMetricsServiceV2 {
|
||||
...siftedMetrics,
|
||||
]);
|
||||
this.lastSeenService.updateLastSeen(siftedMetrics);
|
||||
this.config.eventBus.emit(CLIENT_METRICS, siftedMetrics);
|
||||
}
|
||||
|
||||
async registerImpactMetrics(impactMetrics: Metric[]) {
|
||||
@ -300,7 +301,6 @@ export default class ClientMetricsServiceV2 {
|
||||
|
||||
if (clientMetrics.length) {
|
||||
await this.registerBulkMetrics(clientMetrics);
|
||||
this.config.eventBus.emit(CLIENT_METRICS, clientMetrics);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -23,7 +23,6 @@ import {
|
||||
customMetricsSchema,
|
||||
} from '../shared/schema.js';
|
||||
import type { IClientMetricsEnv } from '../client-metrics/client-metrics-store-v2-type.js';
|
||||
import { CLIENT_METRICS } from '../../../events/index.js';
|
||||
import type { CustomMetricsSchema } from '../../../openapi/spec/custom-metrics-schema.js';
|
||||
import type { StoredCustomMetric } from '../custom/custom-metrics-store.js';
|
||||
import type { CustomMetricsService } from '../custom/custom-metrics-service.js';
|
||||
@ -276,7 +275,6 @@ export default class ClientMetricsController extends Controller {
|
||||
promises.push(
|
||||
this.metricsV2.registerBulkMetrics(filteredData),
|
||||
);
|
||||
this.config.eventBus.emit(CLIENT_METRICS, data);
|
||||
}
|
||||
|
||||
if (
|
||||
|
@ -16,9 +16,12 @@ import dbInit, {
|
||||
import { startOfHour } from 'date-fns';
|
||||
import type TestAgent from 'supertest/lib/agent.d.ts';
|
||||
import type { BulkRegistrationSchema } from '../../../openapi/index.js';
|
||||
import type { EventEmitter } from 'stream';
|
||||
import { CLIENT_METRICS } from '../../../events/index.js';
|
||||
|
||||
let db: ITestDb;
|
||||
let config: IUnleashConfig;
|
||||
let eventBus: EventEmitter;
|
||||
|
||||
async function getSetup(opts?: IUnleashOptions) {
|
||||
config = createTestConfig(opts);
|
||||
@ -26,12 +29,16 @@ async function getSetup(opts?: IUnleashOptions) {
|
||||
|
||||
const services = createServices(db.stores, config, db.rawDatabase);
|
||||
const app = await getApp(config, db.stores, services);
|
||||
|
||||
config.eventBus.emit = vi.fn();
|
||||
|
||||
return {
|
||||
request: supertest(app),
|
||||
stores: db.stores,
|
||||
services,
|
||||
db: db.rawDatabase,
|
||||
destroy: db.destroy,
|
||||
eventBus: config.eventBus,
|
||||
};
|
||||
}
|
||||
|
||||
@ -52,6 +59,7 @@ beforeAll(async () => {
|
||||
stores = setup.stores;
|
||||
destroy = setup.destroy;
|
||||
services = setup.services;
|
||||
eventBus = setup.eventBus;
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
@ -101,13 +109,22 @@ describe('should register unknown flags', () => {
|
||||
appName: 'demo',
|
||||
seenAt: expect.any(Date),
|
||||
});
|
||||
expect(eventBus.emit).toHaveBeenCalledWith(
|
||||
CLIENT_METRICS,
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
featureName: 'existing_flag',
|
||||
yes: 200,
|
||||
}),
|
||||
]),
|
||||
);
|
||||
});
|
||||
|
||||
test('/metrics/bulk endpoint', async () => {
|
||||
// @ts-expect-error - cachedFeatureNames is a private property in ClientMetricsServiceV2
|
||||
services.clientMetricsServiceV2.cachedFeatureNames = vi
|
||||
.fn<() => Promise<string[]>>()
|
||||
.mockResolvedValue(['existing_flag']);
|
||||
.mockResolvedValue(['existing_flag_bulk']);
|
||||
|
||||
const unknownFlag: BulkRegistrationSchema = {
|
||||
appName: 'demo',
|
||||
@ -123,21 +140,21 @@ describe('should register unknown flags', () => {
|
||||
applications: [unknownFlag],
|
||||
metrics: [
|
||||
{
|
||||
featureName: 'existing_flag',
|
||||
featureName: 'existing_flag_bulk',
|
||||
environment: 'development',
|
||||
appName: 'demo',
|
||||
timestamp: startOfHour(new Date()),
|
||||
yes: 200,
|
||||
yes: 1337,
|
||||
no: 0,
|
||||
variants: {},
|
||||
},
|
||||
{
|
||||
featureName: 'unknown_flag',
|
||||
featureName: 'unknown_flag_bulk',
|
||||
environment: 'development',
|
||||
appName: 'demo',
|
||||
timestamp: startOfHour(new Date()),
|
||||
yes: 100,
|
||||
no: 50,
|
||||
yes: 200,
|
||||
no: 100,
|
||||
variants: {},
|
||||
},
|
||||
],
|
||||
@ -149,10 +166,19 @@ describe('should register unknown flags', () => {
|
||||
|
||||
expect(unknownFlags).toHaveLength(1);
|
||||
expect(unknownFlags[0]).toMatchObject({
|
||||
name: 'unknown_flag',
|
||||
name: 'unknown_flag_bulk',
|
||||
environment: 'development',
|
||||
appName: 'demo',
|
||||
seenAt: expect.any(Date),
|
||||
});
|
||||
expect(eventBus.emit).toHaveBeenCalledWith(
|
||||
CLIENT_METRICS,
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
featureName: 'existing_flag_bulk',
|
||||
yes: 1337,
|
||||
}),
|
||||
]),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
Loading…
Reference in New Issue
Block a user