From b0223e38ef18f2810d34b5ea405b2c581792f2af Mon Sep 17 00:00:00 2001 From: Mateusz Kwasniewski Date: Thu, 1 May 2025 15:43:03 +0200 Subject: [PATCH] refactor: stabilize frontend apps reporting (#9880) --- .../frontend-api/createFrontendApiService.ts | 23 +++----- .../frontend-api/frontend-api-controller.ts | 38 +++----------- .../frontend-api/frontend-api-service.test.ts | 1 - .../frontend-api/frontend-api-service.ts | 52 +++++++++++++------ .../middleware/cors-origin-middleware.test.ts | 10 ++-- src/lib/services/index.ts | 2 + .../e2e/api/admin/applications.e2e.test.ts | 36 +++++++++++++ 7 files changed, 94 insertions(+), 68 deletions(-) diff --git a/src/lib/features/frontend-api/createFrontendApiService.ts b/src/lib/features/frontend-api/createFrontendApiService.ts index 93d9e959e2..2d04e64564 100644 --- a/src/lib/features/frontend-api/createFrontendApiService.ts +++ b/src/lib/features/frontend-api/createFrontendApiService.ts @@ -3,12 +3,7 @@ import { SegmentReadModel } from '../segment/segment-read-model'; import type ClientMetricsServiceV2 from '../metrics/client-metrics/metrics-service-v2'; import SettingService from '../../services/setting-service'; import SettingStore from '../../db/setting-store'; -import { - createEventsService, - createFakeEventsService, - createFakeFeatureToggleService, - createFeatureToggleService, -} from '../index'; +import { createEventsService, createFakeEventsService } from '../index'; import type ConfigurationRevisionService from '../feature-toggle/configuration-revision-service'; import { GlobalFrontendApiCache } from './global-frontend-api-cache'; import ClientFeatureToggleReadModel from './client-feature-toggle-read-model'; @@ -17,6 +12,7 @@ import FakeSettingStore from '../../../test/fixtures/fake-setting-store'; import FakeClientFeatureToggleReadModel from './fake-client-feature-toggle-read-model'; import type { IUnleashConfig } from '../../types'; import type { Db } from '../../db/db'; +import type ClientInstanceService from '../metrics/instance/instance-service'; export const createFrontendApiService = ( db: Db, @@ -24,6 +20,7 @@ export const createFrontendApiService = ( // client metrics service needs to be shared because it uses in-memory cache clientMetricsServiceV2: ClientMetricsServiceV2, configurationRevisionService: ConfigurationRevisionService, + clientInstanceService: ClientInstanceService, ): FrontendApiService => { const segmentReadModel = new SegmentReadModel(db); const settingStore = new SettingStore(db, config.getLogger); @@ -33,8 +30,6 @@ export const createFrontendApiService = ( config, eventService, ); - // TODO: remove this dependency after we migrate frontend API - const featureToggleService = createFeatureToggleService(db, config); const clientFeatureToggleReadModel = new ClientFeatureToggleReadModel( db, config.eventBus, @@ -47,12 +42,10 @@ export const createFrontendApiService = ( ); return new FrontendApiService( config, - { segmentReadModel }, { - featureToggleService, clientMetricsServiceV2, settingService, - configurationRevisionService, + clientInstanceService, }, globalFrontendApiCache, ); @@ -62,6 +55,7 @@ export const createFakeFrontendApiService = ( config: IUnleashConfig, clientMetricsServiceV2: ClientMetricsServiceV2, configurationRevisionService: ConfigurationRevisionService, + clientInstanceService: ClientInstanceService, ): FrontendApiService => { const segmentReadModel = new FakeSegmentReadModel(); const settingStore = new FakeSettingStore(); @@ -71,9 +65,6 @@ export const createFakeFrontendApiService = ( config, eventService, ); - // TODO: remove this dependency after we migrate frontend API - const featureToggleService = - createFakeFeatureToggleService(config).featureToggleService; const clientFeatureToggleReadModel = new FakeClientFeatureToggleReadModel(); const globalFrontendApiCache = new GlobalFrontendApiCache( config, @@ -83,12 +74,10 @@ export const createFakeFrontendApiService = ( ); return new FrontendApiService( config, - { segmentReadModel }, { - featureToggleService, clientMetricsServiceV2, settingService, - configurationRevisionService, + clientInstanceService, }, globalFrontendApiCache, ); diff --git a/src/lib/features/frontend-api/frontend-api-controller.ts b/src/lib/features/frontend-api/frontend-api-controller.ts index fa90ed5f33..09579dcfd6 100644 --- a/src/lib/features/frontend-api/frontend-api-controller.ts +++ b/src/lib/features/frontend-api/frontend-api-controller.ts @@ -4,11 +4,10 @@ import { type IFlagResolver, type IUnleashConfig, type IUnleashServices, - type IUser, NONE, } from '../../types'; import type { Logger } from '../../logger'; -import ApiUser, { type IApiUser } from '../../types/api-user'; +import type { IApiUser } from '../../types/api-user'; import { type ClientMetricsSchema, createRequestSchema, @@ -228,13 +227,6 @@ export default class FrontendAPIController extends Controller { ); } - private resolveProject(user: IUser | IApiUser) { - if (user instanceof ApiUser) { - return user.projects; - } - return ['default']; - } - private async registerFrontendApiMetrics( req: ApiUserRequest, res: Response, @@ -248,28 +240,12 @@ export default class FrontendAPIController extends Controller { return; } - const environment = - await this.services.frontendApiService.registerFrontendApiMetrics( - req.user, - req.body, - req.ip, - ); - - if ( - req.body.instanceId && - req.headers['unleash-sdk'] && - this.flagResolver.isEnabled('registerFrontendClient') - ) { - const client = { - appName: req.body.appName, - instanceId: req.body.instanceId, - sdkVersion: req.headers['unleash-sdk'] as string, - sdkType: 'frontend' as const, - environment: environment, - projects: this.resolveProject(req.user), - }; - this.services.clientInstanceService.registerFrontendClient(client); - } + await this.services.frontendApiService.registerFrontendApiMetrics( + req.user, + req.body, + req.ip, + req.headers['unleash-sdk'], + ); res.sendStatus(200); } diff --git a/src/lib/features/frontend-api/frontend-api-service.test.ts b/src/lib/features/frontend-api/frontend-api-service.test.ts index 1cba0ba19d..18ad6a8fdd 100644 --- a/src/lib/features/frontend-api/frontend-api-service.test.ts +++ b/src/lib/features/frontend-api/frontend-api-service.test.ts @@ -48,7 +48,6 @@ test('frontend api service fetching features from global cache', async () => { const frontendApiService = new FrontendApiService( { getLogger: noLogger, eventBus } as unknown as Config, irrelevant, - irrelevant, globalFrontendApiCache, ); diff --git a/src/lib/features/frontend-api/frontend-api-service.ts b/src/lib/features/frontend-api/frontend-api-service.ts index 82ae233d28..56d266037d 100644 --- a/src/lib/features/frontend-api/frontend-api-service.ts +++ b/src/lib/features/frontend-api/frontend-api-service.ts @@ -1,17 +1,17 @@ import crypto from 'node:crypto'; import type { IAuditUser, + IFlagResolver, IUnleashConfig, IUnleashServices, - IUnleashStores, + IUser, } from '../../types'; import type { Logger } from '../../logger'; import type { ClientMetricsSchema, FrontendApiFeatureSchema, } from '../../openapi'; -import type ApiUser from '../../types/api-user'; -import type { IApiUser } from '../../types/api-user'; +import ApiUser, { type IApiUser } from '../../types/api-user'; import { type Context, InMemStorageProvider, @@ -31,17 +31,16 @@ import type { GlobalFrontendApiCache } from './global-frontend-api-cache'; export type Config = Pick< IUnleashConfig, - 'getLogger' | 'frontendApi' | 'frontendApiOrigins' | 'eventBus' + | 'getLogger' + | 'frontendApi' + | 'frontendApiOrigins' + | 'eventBus' + | 'flagResolver' >; -export type Stores = Pick; - export type Services = Pick< IUnleashServices, - | 'featureToggleService' - | 'clientMetricsServiceV2' - | 'settingService' - | 'configurationRevisionService' + 'clientMetricsServiceV2' | 'settingService' | 'clientInstanceService' >; export class FrontendApiService { @@ -49,10 +48,10 @@ export class FrontendApiService { private readonly logger: Logger; - private readonly stores: Stores; - private readonly services: Services; + private flagResolver: IFlagResolver; + private readonly globalFrontendApiCache: GlobalFrontendApiCache; /** @@ -67,14 +66,13 @@ export class FrontendApiService { constructor( config: Config, - stores: Stores, services: Services, globalFrontendApiCache: GlobalFrontendApiCache, ) { this.config = config; this.logger = config.getLogger('services/frontend-api-service.ts'); - this.stores = stores; this.services = services; + this.flagResolver = config.flagResolver; this.globalFrontendApiCache = globalFrontendApiCache; } @@ -107,11 +105,19 @@ export class FrontendApiService { return resultDefinitions; } + private resolveProject(user: IUser | IApiUser) { + if (user instanceof ApiUser) { + return user.projects; + } + return ['default']; + } + async registerFrontendApiMetrics( token: IApiUser, metrics: ClientMetricsSchema, ip: string, - ): Promise { + sdkVersion?: string | string[], + ): Promise { FrontendApiService.assertExpectedTokenType(token); const environment = @@ -128,7 +134,21 @@ export class FrontendApiService { ip, ); - return environment; + if ( + metrics.instanceId && + typeof sdkVersion === 'string' && + this.flagResolver.isEnabled('registerFrontendClient') + ) { + const client = { + appName: metrics.appName, + instanceId: metrics.instanceId, + sdkVersion: sdkVersion, + sdkType: 'frontend' as const, + environment: environment, + projects: this.resolveProject(token), + }; + this.services.clientInstanceService.registerFrontendClient(client); + } } private async clientForFrontendApiToken(token: IApiUser): Promise { diff --git a/src/lib/middleware/cors-origin-middleware.test.ts b/src/lib/middleware/cors-origin-middleware.test.ts index 2e1d97c046..6e1d27b4cb 100644 --- a/src/lib/middleware/cors-origin-middleware.test.ts +++ b/src/lib/middleware/cors-origin-middleware.test.ts @@ -9,8 +9,9 @@ import { type ISettingStore, TEST_AUDIT_USER } from '../../lib/types'; import { frontendSettingsKey } from '../../lib/types/settings/frontend-settings'; import FakeFeatureTagStore from '../../test/fixtures/fake-feature-tag-store'; import { createFakeEventsService } from '../features'; +import type { GlobalFrontendApiCache } from '../features/frontend-api/global-frontend-api-cache'; +import type { Services } from '../features/frontend-api/frontend-api-service'; -const TEST_USER_ID = -9999; const createSettingService = ( frontendApiOrigins: string[], ): { frontendApiService: FrontendApiService; settingStore: ISettingStore } => { @@ -30,8 +31,11 @@ const createSettingService = ( }; return { - //@ts-ignore - frontendApiService: new FrontendApiService(config, stores, services), + frontendApiService: new FrontendApiService( + config, + services as Services, + {} as GlobalFrontendApiCache, + ), settingStore: stores.settingStore, }; }; diff --git a/src/lib/services/index.ts b/src/lib/services/index.ts index 9326b20645..b541e3954a 100644 --- a/src/lib/services/index.ts +++ b/src/lib/services/index.ts @@ -358,11 +358,13 @@ export const createServices = ( config, clientMetricsServiceV2, configurationRevisionService, + clientInstanceService, ) : createFakeFrontendApiService( config, clientMetricsServiceV2, configurationRevisionService, + clientInstanceService, ); const edgeService = new EdgeService({ apiTokenService }, config); diff --git a/src/test/e2e/api/admin/applications.e2e.test.ts b/src/test/e2e/api/admin/applications.e2e.test.ts index b1b191fa19..f8f95f1cff 100644 --- a/src/test/e2e/api/admin/applications.e2e.test.ts +++ b/src/test/e2e/api/admin/applications.e2e.test.ts @@ -12,6 +12,7 @@ import { let app: IUnleashTest; let db: ITestDb; let defaultToken: IApiToken; +let frontendToken: IApiToken; const metrics = { appName: 'appName', @@ -56,6 +57,7 @@ beforeAll(async () => { experimental: { flags: { strictSchemaValidation: true, + registerFrontendClient: true, }, }, }, @@ -69,6 +71,14 @@ beforeAll(async () => { environment: 'default', tokenName: 'tester', }); + + frontendToken = + await app.services.apiTokenService.createApiTokenWithProjects({ + type: ApiTokenType.FRONTEND, + projects: ['default'], + environment: 'default', + tokenName: 'tester', + }); }); afterEach(async () => { @@ -180,6 +190,32 @@ test('should show correct application metrics', async () => { }); }); +test('should report frontend application instances', async () => { + await app.request + .post('/api/frontend/client/metrics') + .set('Authorization', frontendToken.secret) + .set('Unleash-Sdk', 'unleash-client-js:1.0.0') + .send(metrics) + .expect(200); + await app.services.clientInstanceService.bulkAdd(); + + const { body } = await app.request + .get( + `/api/admin/metrics/instances/${metrics.appName}/environment/default`, + ) + .expect(200); + + expect(body).toMatchObject({ + instances: [ + { + instanceId: metrics.instanceId, + clientIp: null, + sdkVersion: 'unleash-client-js:1.0.0', + }, + ], + }); +}); + test('should show missing features and strategies', async () => { await Promise.all([ app.createFeature('toggle-name-1'),