From c4412d8276e4b851dde72b006d35346d8ab8ccc9 Mon Sep 17 00:00:00 2001 From: Jaanus Sellin Date: Wed, 13 Mar 2024 14:28:59 +0200 Subject: [PATCH] fix: measure frontend times only when flag enabled (#6535) Moving to controller level to measure only for flag. Other option would have been to check flag also at service. --- .../frontend-api/frontend-api-controller.ts | 42 +++++++++++++++---- .../frontend-api/frontend-api-service.ts | 14 ------- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/src/lib/features/frontend-api/frontend-api-controller.ts b/src/lib/features/frontend-api/frontend-api-controller.ts index 24f62d2ed5..bfe61dd13a 100644 --- a/src/lib/features/frontend-api/frontend-api-controller.ts +++ b/src/lib/features/frontend-api/frontend-api-controller.ts @@ -23,6 +23,8 @@ import rateLimit from 'express-rate-limit'; import { minutesToMilliseconds } from 'date-fns'; import isEqual from 'lodash.isequal'; import { diff } from 'json-diff'; +import metricsHelper from '../../util/metrics-helper'; +import { FUNCTION_TIME } from '../../metric-events'; interface ApiUserRequest< PARAM = any, @@ -43,11 +45,19 @@ export default class FrontendAPIController extends Controller { private services: Services; + private timer: Function; + constructor(config: IUnleashConfig, services: Services) { super(config); this.logger = config.getLogger('frontend-api-controller.ts'); this.services = services; + this.timer = (functionName) => + metricsHelper.wrapTimer(config.eventBus, FUNCTION_TIME, { + className: 'FrontendAPIController', + functionName, + }); + // Support CORS requests for the frontend endpoints. // Preflight requests are handled in `app.ts`. this.app.use(corsOriginMiddleware(services, config)); @@ -181,14 +191,8 @@ export default class FrontendAPIController extends Controller { if (this.config.flagResolver.isEnabled('globalFrontendApiCache')) { const context = FrontendAPIController.createContext(req); [toggles, newToggles] = await Promise.all([ - this.services.frontendApiService.getFrontendApiFeatures( - req.user, - context, - ), - this.services.frontendApiService.getNewFrontendApiFeatures( - req.user, - context, - ), + this.getTimedFrontendApiFeatures(req.user, context), + this.getTimedNewFrontendApiFeatures(req.user, context), ]); const sortedToggles = toggles.sort((a, b) => a.name.localeCompare(b.name), @@ -233,6 +237,28 @@ export default class FrontendAPIController extends Controller { ); } + private async getTimedFrontendApiFeatures(req, context) { + const stopTimer = this.timer('getFrontendApiFeatures'); + const features = + await this.services.frontendApiService.getFrontendApiFeatures( + req.user, + context, + ); + stopTimer(); + return features; + } + + private async getTimedNewFrontendApiFeatures(req, context) { + const stopTimer = this.timer('getNewFrontendApiFeatures'); + const features = + await this.services.frontendApiService.getNewFrontendApiFeatures( + req.user, + context, + ); + stopTimer(); + return features; + } + private async registerFrontendApiMetrics( req: ApiUserRequest, res: Response, diff --git a/src/lib/features/frontend-api/frontend-api-service.ts b/src/lib/features/frontend-api/frontend-api-service.ts index dc1e19addd..2a43dc2774 100644 --- a/src/lib/features/frontend-api/frontend-api-service.ts +++ b/src/lib/features/frontend-api/frontend-api-service.ts @@ -16,14 +16,12 @@ import { import { validateOrigins } from '../../util'; import { BadDataError, InvalidTokenError } from '../../error'; import { - FUNCTION_TIME, FRONTEND_API_REPOSITORY_CREATED, PROXY_REPOSITORY_CREATED, } from '../../metric-events'; import { FrontendApiRepository } from './frontend-api-repository'; import { GlobalFrontendApiCache } from './global-frontend-api-cache'; import { ProxyRepository } from './proxy-repository'; -import metricsHelper from '../../util/metrics-helper'; export type Config = Pick< IUnleashConfig, @@ -63,8 +61,6 @@ export class FrontendApiService { private cachedFrontendSettings?: FrontendSettings; - private timer: Function; - constructor( config: Config, stores: Stores, @@ -76,19 +72,12 @@ export class FrontendApiService { this.stores = stores; this.services = services; this.globalFrontendApiCache = globalFrontendApiCache; - - this.timer = (functionName) => - metricsHelper.wrapTimer(config.eventBus, FUNCTION_TIME, { - className: 'FrontendApiService', - functionName, - }); } async getFrontendApiFeatures( token: IApiUser, context: Context, ): Promise { - const stopTimer = this.timer('getFrontendApiFeatures'); const client = await this.clientForFrontendApiToken(token); const definitions = client.getFeatureToggleDefinitions() || []; const sessionId = context.sessionId || String(Math.random()); @@ -109,7 +98,6 @@ export class FrontendApiService { }), impressionData: Boolean(feature.impressionData), })); - stopTimer(); return resultDefinitions; } @@ -117,7 +105,6 @@ export class FrontendApiService { token: IApiUser, context: Context, ): Promise { - const stopTimer = this.timer('getNewFrontendApiFeatures'); const client = await this.newClientForFrontendApiToken(token); const definitions = client.getFeatureToggleDefinitions() || []; const sessionId = context.sessionId || String(Math.random()); @@ -139,7 +126,6 @@ export class FrontendApiService { }), impressionData: Boolean(feature.impressionData), })); - stopTimer(); return resultDefinitions; }