From f15bcdc2a66707684bac0280032e6c52642eca1c Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Thu, 18 Jul 2024 13:35:45 +0200 Subject: [PATCH] chore: send prometheus metrics when someone tries to exceed resource limits (#7617) This PR adds prometheus metrics for when users attempt to exceed the limits for a given resource. The implementation sets up a second function exported from the ExceedsLimitError file that records metrics and then throws the error. This could also be a static method on the class, but I'm not sure that'd be better. --- src/lib/error/exceeds-limit-error.test.ts | 54 +++++++++++++++++++ src/lib/error/exceeds-limit-error.ts | 19 +++++++ .../feature-toggle/feature-toggle-service.ts | 26 ++++++--- .../feature-toggle-service.limit.test.ts | 2 +- .../project/project-service.limit.test.ts | 3 ++ src/lib/features/project/project-service.ts | 11 +++- .../segment/segment-service.limit.test.ts | 3 ++ src/lib/features/segment/segment-service.ts | 7 ++- src/lib/metric-events.ts | 2 + src/lib/metrics.test.ts | 21 +++++++- src/lib/metrics.ts | 18 +++++++ src/lib/services/api-token-service.ts | 12 ++++- 12 files changed, 162 insertions(+), 16 deletions(-) create mode 100644 src/lib/error/exceeds-limit-error.test.ts diff --git a/src/lib/error/exceeds-limit-error.test.ts b/src/lib/error/exceeds-limit-error.test.ts new file mode 100644 index 000000000000..be4e9adeab48 --- /dev/null +++ b/src/lib/error/exceeds-limit-error.test.ts @@ -0,0 +1,54 @@ +import type EventEmitter from 'events'; +import { EXCEEDS_LIMIT } from '../metric-events'; +import { + ExceedsLimitError, + throwExceedsLimitError, +} from './exceeds-limit-error'; + +it('emits events event when created through the external function', () => { + const emitEvent = jest.fn(); + const resource = 'some-resource'; + const limit = 10; + + expect(() => + throwExceedsLimitError( + { + emit: emitEvent, + } as unknown as EventEmitter, + { + resource, + limit, + }, + ), + ).toThrow(ExceedsLimitError); + + expect(emitEvent).toHaveBeenCalledWith(EXCEEDS_LIMIT, { + resource, + limit, + }); +}); + +it('emits uses the resourceNameOverride for the event when provided, but uses the resource for the error', () => { + const emitEvent = jest.fn(); + const resource = 'not this'; + const resourceNameOverride = 'but this!'; + const limit = 10; + + expect(() => + throwExceedsLimitError( + { + emit: emitEvent, + } as unknown as EventEmitter, + { + resource, + resourceNameOverride, + limit, + }, + ), + ).toThrow(new ExceedsLimitError(resource, limit)); + + expect(emitEvent).toHaveBeenCalledWith(EXCEEDS_LIMIT, { + resource: resourceNameOverride, + limit, + }); +}); diff --git a/src/lib/error/exceeds-limit-error.ts b/src/lib/error/exceeds-limit-error.ts index bb5a24c41ff9..b5a31244dc0f 100644 --- a/src/lib/error/exceeds-limit-error.ts +++ b/src/lib/error/exceeds-limit-error.ts @@ -1,4 +1,6 @@ import { GenericUnleashError } from './unleash-error'; +import { EXCEEDS_LIMIT } from '../metric-events'; +import type EventEmitter from 'events'; export class ExceedsLimitError extends GenericUnleashError { constructor(resource: string, limit: number) { @@ -9,3 +11,20 @@ export class ExceedsLimitError extends GenericUnleashError { }); } } + +type ExceedsLimitErrorData = { + resource: string; + limit: number; + resourceNameOverride?: string; +}; + +export const throwExceedsLimitError = ( + eventBus: EventEmitter, + { resource, limit, resourceNameOverride }: ExceedsLimitErrorData, +) => { + eventBus.emit(EXCEEDS_LIMIT, { + resource: resourceNameOverride ?? resource, + limit, + }); + throw new ExceedsLimitError(resource, limit); +}; diff --git a/src/lib/features/feature-toggle/feature-toggle-service.ts b/src/lib/features/feature-toggle/feature-toggle-service.ts index cd4b137ceb7d..fe5d2d304ea3 100644 --- a/src/lib/features/feature-toggle/feature-toggle-service.ts +++ b/src/lib/features/feature-toggle/feature-toggle-service.ts @@ -109,7 +109,7 @@ import { allSettledWithRejection } from '../../util/allSettledWithRejection'; import type EventEmitter from 'node:events'; import type { IFeatureLifecycleReadModel } from '../feature-lifecycle/feature-lifecycle-read-model-type'; import type { ResourceLimitsSchema } from '../../openapi'; -import { ExceedsLimitError } from '../../error/exceeds-limit-error'; +import { throwExceedsLimitError } from '../../error/exceeds-limit-error'; interface IFeatureContext { featureName: string; @@ -383,7 +383,10 @@ class FeatureToggleService { ) ).length; if (existingCount >= limit) { - throw new ExceedsLimitError('strategy', limit); + throwExceedsLimitError(this.eventBus, { + resource: 'strategy', + limit, + }); } } @@ -392,7 +395,10 @@ class FeatureToggleService { const constraintsLimit = this.resourceLimits.constraints; if (updatedConstrains.length > constraintsLimit) { - throw new ExceedsLimitError(`constraints`, constraintsLimit); + throwExceedsLimitError(this.eventBus, { + resource: `constraints`, + limit: constraintsLimit, + }); } const constraintValuesLimit = this.resourceLimits.constraintValues; @@ -402,10 +408,11 @@ class FeatureToggleService { constraint.values?.length > constraintValuesLimit, ); if (constraintOverLimit) { - throw new ExceedsLimitError( - `content values for ${constraintOverLimit.contextName}`, - constraintValuesLimit, - ); + throwExceedsLimitError(this.eventBus, { + resource: `constraint values for ${constraintOverLimit.contextName}`, + limit: constraintValuesLimit, + resourceNameOverride: 'constraint values', + }); } } @@ -1181,7 +1188,10 @@ class FeatureToggleService { const currentFlagCount = await this.featureToggleStore.count(); const limit = this.resourceLimits.featureFlags; if (currentFlagCount >= limit) { - throw new ExceedsLimitError('feature flag', limit); + throwExceedsLimitError(this.eventBus, { + resource: 'feature flag', + limit, + }); } } } diff --git a/src/lib/features/feature-toggle/tests/feature-toggle-service.limit.test.ts b/src/lib/features/feature-toggle/tests/feature-toggle-service.limit.test.ts index cb0a572da7b3..a3ee0a3f3786 100644 --- a/src/lib/features/feature-toggle/tests/feature-toggle-service.limit.test.ts +++ b/src/lib/features/feature-toggle/tests/feature-toggle-service.limit.test.ts @@ -125,7 +125,7 @@ describe('Strategy limits', () => { }, ]), ).rejects.toThrow( - "Failed to create content values for userId. You can't create more than the established limit of 3", + "Failed to create constraint values for userId. You can't create more than the established limit of 3", ); }); }); diff --git a/src/lib/features/project/project-service.limit.test.ts b/src/lib/features/project/project-service.limit.test.ts index 186af10bb860..d9bf0d7af023 100644 --- a/src/lib/features/project/project-service.limit.test.ts +++ b/src/lib/features/project/project-service.limit.test.ts @@ -17,6 +17,9 @@ test('Should not allow to exceed project limit', async () => { resourceLimits: { projects: LIMIT, }, + eventBus: { + emit: () => {}, + }, } as unknown as IUnleashConfig); const createProject = (name: string) => diff --git a/src/lib/features/project/project-service.ts b/src/lib/features/project/project-service.ts index 46ba74576bfc..10b05e7c4fe9 100644 --- a/src/lib/features/project/project-service.ts +++ b/src/lib/features/project/project-service.ts @@ -84,7 +84,8 @@ import type { IProjectQuery, } from './project-store-type'; import type { IProjectFlagCreatorsReadModel } from './project-flag-creators-read-model.type'; -import { ExceedsLimitError } from '../../error/exceeds-limit-error'; +import { throwExceedsLimitError } from '../../error/exceeds-limit-error'; +import type EventEmitter from 'events'; type Days = number; type Count = number; @@ -159,6 +160,8 @@ export default class ProjectService { private resourceLimits: ResourceLimitsSchema; + private eventBus: EventEmitter; + constructor( { projectStore, @@ -215,6 +218,7 @@ export default class ProjectService { this.flagResolver = config.flagResolver; this.isEnterprise = config.isEnterprise; this.resourceLimits = config.resourceLimits; + this.eventBus = config.eventBus; } async getProjects( @@ -325,7 +329,10 @@ export default class ProjectService { const projectCount = await this.projectStore.count(); if (projectCount >= limit) { - throw new ExceedsLimitError('project', limit); + throwExceedsLimitError(this.eventBus, { + resource: 'project', + limit, + }); } } diff --git a/src/lib/features/segment/segment-service.limit.test.ts b/src/lib/features/segment/segment-service.limit.test.ts index fb40777677f8..f2e2947e800a 100644 --- a/src/lib/features/segment/segment-service.limit.test.ts +++ b/src/lib/features/segment/segment-service.limit.test.ts @@ -16,6 +16,9 @@ test('Should not allow to exceed segment limit', async () => { resourceLimits: { segments: LIMIT, }, + eventBus: { + emit: () => {}, + }, } as unknown as IUnleashConfig); const createSegment = (name: string) => diff --git a/src/lib/features/segment/segment-service.ts b/src/lib/features/segment/segment-service.ts index eb7fe93d8731..e1c83ed59b70 100644 --- a/src/lib/features/segment/segment-service.ts +++ b/src/lib/features/segment/segment-service.ts @@ -26,7 +26,7 @@ import type { IPrivateProjectChecker } from '../private-project/privateProjectCh import type EventService from '../events/event-service'; import type { IChangeRequestSegmentUsageReadModel } from '../change-request-segment-usage-service/change-request-segment-usage-read-model'; import type { ResourceLimitsSchema } from '../../openapi'; -import { ExceedsLimitError } from '../../error/exceeds-limit-error'; +import { throwExceedsLimitError } from '../../error/exceeds-limit-error'; export class SegmentService implements ISegmentService { private logger: Logger; @@ -136,7 +136,10 @@ export class SegmentService implements ISegmentService { const segmentCount = await this.segmentStore.count(); if (segmentCount >= limit) { - throw new ExceedsLimitError('segment', limit); + throwExceedsLimitError(this.config.eventBus, { + resource: 'segment', + limit, + }); } } diff --git a/src/lib/metric-events.ts b/src/lib/metric-events.ts index d92d06d9a2f2..413f47a06c97 100644 --- a/src/lib/metric-events.ts +++ b/src/lib/metric-events.ts @@ -8,6 +8,7 @@ const FRONTEND_API_REPOSITORY_CREATED = 'frontend_api_repository_created'; const PROXY_REPOSITORY_CREATED = 'proxy_repository_created'; const PROXY_FEATURES_FOR_TOKEN_TIME = 'proxy_features_for_token_time'; const STAGE_ENTERED = 'stage-entered' as const; +const EXCEEDS_LIMIT = 'exceeds-limit' as const; export { REQUEST_TIME, @@ -20,4 +21,5 @@ export { PROXY_REPOSITORY_CREATED, PROXY_FEATURES_FOR_TOKEN_TIME, STAGE_ENTERED, + EXCEEDS_LIMIT, }; diff --git a/src/lib/metrics.test.ts b/src/lib/metrics.test.ts index 4b0e4bbcf4be..20a46ab34354 100644 --- a/src/lib/metrics.test.ts +++ b/src/lib/metrics.test.ts @@ -2,7 +2,12 @@ import { register } from 'prom-client'; import EventEmitter from 'events'; import type { IEventStore } from './types/stores/event-store'; import { createTestConfig } from '../test/config/test-config'; -import { DB_TIME, FUNCTION_TIME, REQUEST_TIME } from './metric-events'; +import { + DB_TIME, + EXCEEDS_LIMIT, + FUNCTION_TIME, + REQUEST_TIME, +} from './metric-events'; import { CLIENT_METRICS, CLIENT_REGISTER, @@ -330,3 +335,17 @@ test('should collect metrics for lifecycle', async () => { expect(metrics).toMatch(/feature_lifecycle_stage_count_by_project/); expect(metrics).toMatch(/feature_lifecycle_stage_entered/); }); + +test('should collect limit exceeded metrics', async () => { + eventBus.emit(EXCEEDS_LIMIT, { + resource: 'feature flags', + limit: '5000', + }); + + const recordedMetric = await prometheusRegister.getSingleMetricAsString( + 'exceeds_limit_error', + ); + expect(recordedMetric).toMatch( + /exceeds_limit_error{resource=\"feature flags\",limit=\"5000\"} 1/, + ); +}); diff --git a/src/lib/metrics.ts b/src/lib/metrics.ts index 7aa939d14372..92137835905d 100644 --- a/src/lib/metrics.ts +++ b/src/lib/metrics.ts @@ -341,6 +341,12 @@ export default class MetricsMonitor { help: 'Number of API tokens with v1 format, last seen within 3 months', }); + const exceedsLimitErrorCounter = createCounter({ + name: 'exceeds_limit_error', + help: 'The number of exceeds limit errors registered by this instance.', + labelNames: ['resource', 'limit'], + }); + async function collectStaticCounters() { try { const stats = await instanceStatsService.getStats(); @@ -400,6 +406,18 @@ export default class MetricsMonitor { }, ); + eventBus.on( + events.EXCEEDS_LIMIT, + ({ + resource, + limit, + }: { resource: string; limit: number }) => { + exceedsLimitErrorCounter + .labels({ resource, limit }) + .inc(); + }, + ); + featureLifecycleStageCountByProject.reset(); stageCountByProjectResult.forEach((stageResult) => featureLifecycleStageCountByProject diff --git a/src/lib/services/api-token-service.ts b/src/lib/services/api-token-service.ts index 6d3d76dd4169..1c4e0d2f7f44 100644 --- a/src/lib/services/api-token-service.ts +++ b/src/lib/services/api-token-service.ts @@ -34,7 +34,8 @@ import { addMinutes, isPast } from 'date-fns'; import metricsHelper from '../util/metrics-helper'; import { FUNCTION_TIME } from '../metric-events'; import type { ResourceLimitsSchema } from '../openapi'; -import { ExceedsLimitError } from '../error/exceeds-limit-error'; +import { throwExceedsLimitError } from '../error/exceeds-limit-error'; +import type EventEmitter from 'events'; const resolveTokenPermissions = (tokenType: string) => { if (tokenType === ApiTokenType.ADMIN) { @@ -73,6 +74,8 @@ export class ApiTokenService { private resourceLimits: ResourceLimitsSchema; + private eventBus: EventEmitter; + constructor( { apiTokenStore, @@ -109,6 +112,8 @@ export class ApiTokenService { className: 'ApiTokenService', functionName, }); + + this.eventBus = config.eventBus; } /** @@ -307,7 +312,10 @@ export class ApiTokenService { const currentTokenCount = await this.store.count(); const limit = this.resourceLimits.apiTokens; if (currentTokenCount >= limit) { - throw new ExceedsLimitError('api token', limit); + throwExceedsLimitError(this.eventBus, { + resource: 'api token', + limit, + }); } } }