From a4171832c37881e7bd5af94d2fd44232463f57da Mon Sep 17 00:00:00 2001
From: Cody Born <cody.born@uniswap.org>
Date: Mon, 2 Dec 2024 01:31:08 -0500
Subject: [PATCH] Fix S3 clock skew and tighten 5xx alerts (#392)

* Tighten 5xx alerts

* Do not use the S3 client in the injector
---
 bin/stacks/api-stack.ts                 |  8 ++++----
 lib/handlers/hard-quote/injector.ts     |  6 +-----
 lib/handlers/quote/injector.ts          |  6 +-----
 lib/providers/circuit-breaker/dynamo.ts | 21 +++++++++++++++------
 4 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/bin/stacks/api-stack.ts b/bin/stacks/api-stack.ts
index 1067d35..cab2eea 100644
--- a/bin/stacks/api-stack.ts
+++ b/bin/stacks/api-stack.ts
@@ -440,10 +440,10 @@ export class APIStack extends cdk.Stack {
       alarmName: 'UniswapXParameterizationAPI-SEV2-5XX',
       metric: api.metricServerError({
         period: Duration.minutes(5),
-        // For this metric 'avg' represents error rate.
-        statistic: 'avg',
+        // For this metric 'sum' represents error count.
+        statistic: 'sum',
       }),
-      threshold: 0.5,
+      threshold: 100,
       // Beta has much less traffic so is more susceptible to transient errors.
       evaluationPeriods: stage == STAGE.BETA ? 5 : 3,
     });
@@ -455,7 +455,7 @@ export class APIStack extends cdk.Stack {
         // For this metric 'sum' represents error count.
         statistic: 'sum',
       }),
-      threshold: 100,
+      threshold: 20,
       // Beta has much less traffic so is more susceptible to transient errors.
       evaluationPeriods: stage == STAGE.BETA ? 5 : 3,
     });
diff --git a/lib/handlers/hard-quote/injector.ts b/lib/handlers/hard-quote/injector.ts
index 43a0197..89240ae 100644
--- a/lib/handlers/hard-quote/injector.ts
+++ b/lib/handlers/hard-quote/injector.ts
@@ -42,11 +42,7 @@ export class QuoteInjector extends ApiInjector<ContainerInjected, RequestInjecte
     const orderServiceUrl = checkDefined(process.env.ORDER_SERVICE_URL, 'ORDER_SERVICE_URL is not defined');
 
     const webhookProvider = new S3WebhookConfigurationProvider(log, `${WEBHOOK_CONFIG_BUCKET}-${stage}-1`, s3Key);
-    await webhookProvider.fetchEndpoints();
-    const circuitBreakerProvider = new DynamoCircuitBreakerConfigurationProvider(
-      log,
-      webhookProvider.fillerEndpoints()
-    );
+    const circuitBreakerProvider = new DynamoCircuitBreakerConfigurationProvider(log, webhookProvider);
 
     const orderServiceProvider = new UniswapXServiceProvider(log, orderServiceUrl);
 
diff --git a/lib/handlers/quote/injector.ts b/lib/handlers/quote/injector.ts
index be65c45..5dc19a1 100644
--- a/lib/handlers/quote/injector.ts
+++ b/lib/handlers/quote/injector.ts
@@ -45,11 +45,7 @@ export class QuoteInjector extends ApiInjector<ContainerInjected, RequestInjecte
     const s3Key = stage === STAGE.BETA ? BETA_S3_KEY : PRODUCTION_S3_KEY;
 
     const webhookProvider = new S3WebhookConfigurationProvider(log, `${WEBHOOK_CONFIG_BUCKET}-${stage}-1`, s3Key);
-    await webhookProvider.fetchEndpoints();
-    const circuitBreakerProvider = new DynamoCircuitBreakerConfigurationProvider(
-      log,
-      webhookProvider.fillerEndpoints()
-    );
+    const circuitBreakerProvider = new DynamoCircuitBreakerConfigurationProvider(log, webhookProvider);
 
     const complianceKey = stage === STAGE.BETA ? BETA_COMPLIANCE_S3_KEY : PROD_COMPLIANCE_S3_KEY;
     const fillerComplianceProvider = new S3FillerComplianceConfigurationProvider(
diff --git a/lib/providers/circuit-breaker/dynamo.ts b/lib/providers/circuit-breaker/dynamo.ts
index d512591..b56e540 100644
--- a/lib/providers/circuit-breaker/dynamo.ts
+++ b/lib/providers/circuit-breaker/dynamo.ts
@@ -4,11 +4,12 @@ import Logger from 'bunyan';
 
 import { CircuitBreakerConfigurationProvider, EndpointStatuses } from '.';
 import { BaseTimestampRepository, FillerTimestampMap, TimestampRepository } from '../../repositories';
-import { WebhookConfiguration } from '../webhook';
+import { S3WebhookConfigurationProvider, WebhookConfiguration } from '../webhook';
 
 export class DynamoCircuitBreakerConfigurationProvider implements CircuitBreakerConfigurationProvider {
   private log: Logger;
-  private fillerEndpoints: string[];
+  private webhookProvider: S3WebhookConfigurationProvider;
+  private fillerEndpoints: string[] = [];
   private lastUpdatedTimestamp: number;
   private timestampDB: BaseTimestampRepository;
   private timestamps: FillerTimestampMap = new Map();
@@ -16,9 +17,9 @@ export class DynamoCircuitBreakerConfigurationProvider implements CircuitBreaker
   // try to refetch endpoints every 30 seconds
   private static UPDATE_PERIOD_MS = 1 * 30000;
 
-  constructor(_log: Logger, _fillerEndpoints: string[] = []) {
+  constructor(_log: Logger, _webhookProvider: S3WebhookConfigurationProvider) {
     this.log = _log.child({ quoter: 'CircuitBreakerConfigurationProvider' });
-    this.fillerEndpoints = _fillerEndpoints;
+    this.webhookProvider = _webhookProvider;
     this.lastUpdatedTimestamp = Date.now();
     const documentClient = DynamoDBDocumentClient.from(new DynamoDBClient({}), {
       marshallOptions: {
@@ -31,9 +32,17 @@ export class DynamoCircuitBreakerConfigurationProvider implements CircuitBreaker
     this.timestampDB = TimestampRepository.create(documentClient);
   }
 
+  private async getFillerEndpoints(): Promise<string[]> {
+    if (this.fillerEndpoints.length === 0) {
+      this.fillerEndpoints = this.webhookProvider.fillerEndpoints();
+      this.lastUpdatedTimestamp = Date.now();
+    }
+    return this.fillerEndpoints;
+  }
+
   async getConfigurations(): Promise<FillerTimestampMap> {
     if (
-      this.fillerEndpoints.length === 0 ||
+      (await this.getFillerEndpoints()).length === 0 ||
       Date.now() - this.lastUpdatedTimestamp > DynamoCircuitBreakerConfigurationProvider.UPDATE_PERIOD_MS
     ) {
       await this.fetchConfigurations();
@@ -44,7 +53,7 @@ export class DynamoCircuitBreakerConfigurationProvider implements CircuitBreaker
   }
 
   async fetchConfigurations(): Promise<void> {
-    this.timestamps = await this.timestampDB.getFillerTimestampsMap(this.fillerEndpoints);
+    this.timestamps = await this.timestampDB.getFillerTimestampsMap(await this.getFillerEndpoints());
   }
 
   /* add filler to `enabled` array if it's not blocked until a future timestamp;