Common - Throttle update + Metrics (#15413)
This commit is contained in:
+47
-28
@@ -31,6 +31,8 @@ import { WorkspacePreQueryHookPayload } from 'src/engine/api/graphql/workspace-q
|
||||
import { WorkspaceQueryHookService } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/workspace-query-hook.service';
|
||||
import { ApiKeyRoleService } from 'src/engine/core-modules/api-key/api-key-role.service';
|
||||
import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type';
|
||||
import { MetricsService } from 'src/engine/core-modules/metrics/metrics.service';
|
||||
import { MetricsKeys } from 'src/engine/core-modules/metrics/types/metrics-keys.type';
|
||||
import { ThrottlerService } from 'src/engine/core-modules/throttler/throttler.service';
|
||||
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
|
||||
import { type PermissionFlagType } from 'src/engine/metadata-modules/permissions/constants/permission-flag-type.constants';
|
||||
@@ -77,6 +79,8 @@ export abstract class CommonBaseQueryRunnerService<
|
||||
protected readonly throttlerService: ThrottlerService;
|
||||
@Inject()
|
||||
protected readonly twentyConfigService: TwentyConfigService;
|
||||
@Inject()
|
||||
protected readonly metricsService: MetricsService;
|
||||
|
||||
protected abstract readonly operationName: CommonQueryNames;
|
||||
|
||||
@@ -94,7 +98,7 @@ export abstract class CommonBaseQueryRunnerService<
|
||||
);
|
||||
}
|
||||
|
||||
await this.throttleQueryExecution(authContext.workspace.id);
|
||||
await this.throttleQueryExecution(authContext);
|
||||
|
||||
await this.validate(args, queryRunnerContext);
|
||||
|
||||
@@ -331,35 +335,50 @@ export abstract class CommonBaseQueryRunnerService<
|
||||
};
|
||||
}
|
||||
|
||||
private async throttleQueryExecution(workspaceId: string) {
|
||||
const shortConfig = {
|
||||
key: `api:throttler:${workspaceId}-short-limit`,
|
||||
maxTokens: this.twentyConfigService.get('API_RATE_LIMITING_SHORT_LIMIT'),
|
||||
timeWindow: this.twentyConfigService.get(
|
||||
'API_RATE_LIMITING_SHORT_TTL_IN_MS',
|
||||
),
|
||||
};
|
||||
private async throttleQueryExecution(authContext: WorkspaceAuthContext) {
|
||||
try {
|
||||
if (!isDefined(authContext.apiKey)) return;
|
||||
|
||||
const longConfig = {
|
||||
key: `api:throttler:${workspaceId}-long-limit`,
|
||||
maxTokens: this.twentyConfigService.get('API_RATE_LIMITING_LONG_LIMIT'),
|
||||
timeWindow: this.twentyConfigService.get(
|
||||
'API_RATE_LIMITING_LONG_TTL_IN_MS',
|
||||
),
|
||||
};
|
||||
const workspaceId = authContext.workspace.id;
|
||||
|
||||
await this.throttlerService.tokenBucketThrottle(
|
||||
shortConfig.key,
|
||||
1,
|
||||
shortConfig.maxTokens,
|
||||
shortConfig.timeWindow,
|
||||
);
|
||||
const shortConfig = {
|
||||
key: `api:throttler:${workspaceId}-short-limit`,
|
||||
maxTokens: this.twentyConfigService.get(
|
||||
'API_RATE_LIMITING_SHORT_LIMIT',
|
||||
),
|
||||
timeWindow: this.twentyConfigService.get(
|
||||
'API_RATE_LIMITING_SHORT_TTL_IN_MS',
|
||||
),
|
||||
};
|
||||
|
||||
await this.throttlerService.tokenBucketThrottle(
|
||||
longConfig.key,
|
||||
1,
|
||||
longConfig.maxTokens,
|
||||
longConfig.timeWindow,
|
||||
);
|
||||
const longConfig = {
|
||||
key: `api:throttler:${workspaceId}-long-limit`,
|
||||
maxTokens: this.twentyConfigService.get('API_RATE_LIMITING_LONG_LIMIT'),
|
||||
timeWindow: this.twentyConfigService.get(
|
||||
'API_RATE_LIMITING_LONG_TTL_IN_MS',
|
||||
),
|
||||
};
|
||||
|
||||
await this.throttlerService.tokenBucketThrottleOrThrow(
|
||||
shortConfig.key,
|
||||
1,
|
||||
shortConfig.maxTokens,
|
||||
shortConfig.timeWindow,
|
||||
);
|
||||
|
||||
await this.throttlerService.tokenBucketThrottleOrThrow(
|
||||
longConfig.key,
|
||||
1,
|
||||
longConfig.maxTokens,
|
||||
longConfig.timeWindow,
|
||||
);
|
||||
} catch (error) {
|
||||
await this.metricsService.incrementCounter({
|
||||
key: MetricsKeys.CommonApiQueryRateLimited,
|
||||
shouldStoreInCache: false,
|
||||
});
|
||||
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -12,6 +12,7 @@ import { WorkspaceQueryHookModule } from 'src/engine/api/graphql/workspace-query
|
||||
import { WorkspaceQueryRunnerModule } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-runner.module';
|
||||
import { ApiKeyModule } from 'src/engine/core-modules/api-key/api-key.module';
|
||||
import { FileModule } from 'src/engine/core-modules/file/file.module';
|
||||
import { MetricsModule } from 'src/engine/core-modules/metrics/metrics.module';
|
||||
import { ThrottlerModule } from 'src/engine/core-modules/throttler/throttler.module';
|
||||
import { PermissionsModule } from 'src/engine/metadata-modules/permissions/permissions.module';
|
||||
import { RoleTargetsEntity } from 'src/engine/metadata-modules/role/role-targets.entity';
|
||||
@@ -35,6 +36,7 @@ import { WorkspacePermissionsCacheModule } from 'src/engine/metadata-modules/wor
|
||||
ViewFilterModule,
|
||||
ViewFilterGroupModule,
|
||||
ThrottlerModule,
|
||||
MetricsModule,
|
||||
],
|
||||
providers: [
|
||||
ProcessNestedRelationsHelper,
|
||||
|
||||
@@ -4,7 +4,6 @@ import GraphQLJSON from 'graphql-type-json';
|
||||
import { NodeEnvironment } from 'src/engine/core-modules/twenty-config/interfaces/node-environment.interface';
|
||||
|
||||
import { useCachedMetadata } from 'src/engine/api/graphql/graphql-config/hooks/use-cached-metadata';
|
||||
import { useThrottler } from 'src/engine/api/graphql/graphql-config/hooks/use-throttler';
|
||||
import { MetadataGraphQLApiModule } from 'src/engine/api/graphql/metadata-graphql-api.module';
|
||||
import { type CacheStorageService } from 'src/engine/core-modules/cache-storage/services/cache-storage.service';
|
||||
import { type ExceptionHandlerService } from 'src/engine/core-modules/exception-handler/exception-handler.service';
|
||||
@@ -31,13 +30,6 @@ export const metadataModuleFactory = async (
|
||||
},
|
||||
resolvers: { JSON: GraphQLJSON },
|
||||
plugins: [
|
||||
useThrottler({
|
||||
ttl: twentyConfigService.get('API_RATE_LIMITING_LONG_TTL_IN_MS') / 1000,
|
||||
limit: twentyConfigService.get('API_RATE_LIMITING_LONG_LIMIT'),
|
||||
identifyFn: (context) => {
|
||||
return context.req.user?.id ?? context.req.ip ?? 'anonymous';
|
||||
},
|
||||
}),
|
||||
useGraphQLErrorHandlerHook({
|
||||
metricsService: metricsService,
|
||||
exceptionHandlerService,
|
||||
|
||||
@@ -29,4 +29,5 @@ export enum MetricsKeys {
|
||||
CronJobDeletedWorkspace = 'cron-job/deleted-workspace',
|
||||
JobWebhookCallCompleted = 'job/webhook-call-completed',
|
||||
SignUpSuccess = 'sign-up/success',
|
||||
CommonApiQueryRateLimited = 'common-api-query/rate-limited',
|
||||
}
|
||||
|
||||
@@ -31,7 +31,7 @@ describe('ThrottlerService', () => {
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
describe('tokenBucketThrottle', () => {
|
||||
describe('tokenBucketThrottleOrThrow', () => {
|
||||
const key = 'test-throttle-key';
|
||||
const maxTokens = 100;
|
||||
const timeWindow = 1000; // 1 second
|
||||
@@ -39,7 +39,7 @@ describe('ThrottlerService', () => {
|
||||
it('should allow request when tokens are available (first request)', async () => {
|
||||
cacheStorageService.get.mockResolvedValue(null);
|
||||
|
||||
await service.tokenBucketThrottle(key, 10, maxTokens, timeWindow);
|
||||
await service.tokenBucketThrottleOrThrow(key, 10, maxTokens, timeWindow);
|
||||
|
||||
expect(cacheStorageService.get).toHaveBeenCalledWith(key);
|
||||
expect(cacheStorageService.set).toHaveBeenCalledWith(
|
||||
@@ -60,7 +60,7 @@ describe('ThrottlerService', () => {
|
||||
lastRefillAt: now - 100,
|
||||
});
|
||||
|
||||
await service.tokenBucketThrottle(key, 10, maxTokens, timeWindow);
|
||||
await service.tokenBucketThrottleOrThrow(key, 10, maxTokens, timeWindow);
|
||||
|
||||
expect(cacheStorageService.get).toHaveBeenCalledWith(key);
|
||||
expect(cacheStorageService.set).toHaveBeenCalledWith(
|
||||
@@ -82,11 +82,11 @@ describe('ThrottlerService', () => {
|
||||
});
|
||||
|
||||
await expect(
|
||||
service.tokenBucketThrottle(key, 10, maxTokens, timeWindow),
|
||||
service.tokenBucketThrottleOrThrow(key, 10, maxTokens, timeWindow),
|
||||
).rejects.toThrow(ThrottlerException);
|
||||
|
||||
await expect(
|
||||
service.tokenBucketThrottle(key, 10, maxTokens, timeWindow),
|
||||
service.tokenBucketThrottleOrThrow(key, 10, maxTokens, timeWindow),
|
||||
).rejects.toThrow('Limit reached');
|
||||
|
||||
expect(cacheStorageService.set).not.toHaveBeenCalled();
|
||||
@@ -105,7 +105,7 @@ describe('ThrottlerService', () => {
|
||||
|
||||
jest.spyOn(Date, 'now').mockReturnValue(now);
|
||||
|
||||
await service.tokenBucketThrottle(key, 10, maxTokens, timeWindow);
|
||||
await service.tokenBucketThrottleOrThrow(key, 10, maxTokens, timeWindow);
|
||||
|
||||
expect(cacheStorageService.set).toHaveBeenCalledWith(
|
||||
key,
|
||||
@@ -130,7 +130,7 @@ describe('ThrottlerService', () => {
|
||||
|
||||
jest.spyOn(Date, 'now').mockReturnValue(now);
|
||||
|
||||
await service.tokenBucketThrottle(key, 10, maxTokens, timeWindow);
|
||||
await service.tokenBucketThrottleOrThrow(key, 10, maxTokens, timeWindow);
|
||||
|
||||
// Available tokens should be capped at maxTokens (100)
|
||||
expect(cacheStorageService.set).toHaveBeenCalledWith(
|
||||
|
||||
@@ -28,7 +28,7 @@ export class ThrottlerService {
|
||||
await this.cacheStorage.set(key, currentCount + 1, ttl);
|
||||
}
|
||||
|
||||
async tokenBucketThrottle(
|
||||
async tokenBucketThrottleOrThrow(
|
||||
key: string,
|
||||
tokensToConsume: number,
|
||||
maxTokens: number,
|
||||
|
||||
@@ -975,7 +975,7 @@ export class ConfigVariables {
|
||||
type: ConfigVariableType.NUMBER,
|
||||
})
|
||||
@CastToPositiveNumber()
|
||||
API_RATE_LIMITING_LONG_LIMIT = 1000;
|
||||
API_RATE_LIMITING_LONG_LIMIT = 100;
|
||||
|
||||
@ConfigVariablesMetadata({
|
||||
group: ConfigVariablesGroup.SSL,
|
||||
|
||||
Reference in New Issue
Block a user