Skip to content

Commit

Permalink
fix: measure frontend times only when flag enabled (#6535)
Browse files Browse the repository at this point in the history
Moving to controller level to measure only for flag. Other option would
have been to check flag also at service.
  • Loading branch information
sjaanus authored Mar 13, 2024
1 parent 1aca597 commit c4412d8
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 22 deletions.
42 changes: 34 additions & 8 deletions src/lib/features/frontend-api/frontend-api-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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));
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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<unknown, unknown, ClientMetricsSchema>,
res: Response,
Expand Down
14 changes: 0 additions & 14 deletions src/lib/features/frontend-api/frontend-api-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -63,8 +61,6 @@ export class FrontendApiService {

private cachedFrontendSettings?: FrontendSettings;

private timer: Function;

constructor(
config: Config,
stores: Stores,
Expand All @@ -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<FrontendApiFeatureSchema[]> {
const stopTimer = this.timer('getFrontendApiFeatures');
const client = await this.clientForFrontendApiToken(token);
const definitions = client.getFeatureToggleDefinitions() || [];
const sessionId = context.sessionId || String(Math.random());
Expand All @@ -109,15 +98,13 @@ export class FrontendApiService {
}),
impressionData: Boolean(feature.impressionData),
}));
stopTimer();
return resultDefinitions;
}

async getNewFrontendApiFeatures(
token: IApiUser,
context: Context,
): Promise<FrontendApiFeatureSchema[]> {
const stopTimer = this.timer('getNewFrontendApiFeatures');
const client = await this.newClientForFrontendApiToken(token);
const definitions = client.getFeatureToggleDefinitions() || [];
const sessionId = context.sessionId || String(Math.random());
Expand All @@ -139,7 +126,6 @@ export class FrontendApiService {
}),
impressionData: Boolean(feature.impressionData),
}));
stopTimer();
return resultDefinitions;
}

Expand Down

0 comments on commit c4412d8

Please sign in to comment.