From 8d872dbb8482276a341adabf570099d08640bbf4 Mon Sep 17 00:00:00 2001
From: alecps <alec@cLabs.co>
Date: Tue, 17 Oct 2023 13:51:38 -0400
Subject: [PATCH] add tail latency metric

---
 .../src/common/crypto-clients/crypto-client.ts       | 12 ++++++++++--
 .../combiner/src/common/handlers.ts                  |  5 ++---
 .../combiner/src/common/metrics.ts                   |  6 ++++++
 .../combiner/src/domain/endpoints/sign/action.ts     |  2 +-
 .../combiner/src/pnp/endpoints/sign/action.ts        |  2 +-
 5 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/packages/phone-number-privacy/combiner/src/common/crypto-clients/crypto-client.ts b/packages/phone-number-privacy/combiner/src/common/crypto-clients/crypto-client.ts
index c18aecd1d..a34705939 100644
--- a/packages/phone-number-privacy/combiner/src/common/crypto-clients/crypto-client.ts
+++ b/packages/phone-number-privacy/combiner/src/common/crypto-clients/crypto-client.ts
@@ -9,6 +9,7 @@ export interface ServicePartialSignature {
 
 export abstract class CryptoClient {
   protected unverifiedSignatures: ServicePartialSignature[] = []
+  private tailLatencyTimer: () => void = () => {}
 
   constructor(protected readonly keyVersionInfo: KeyVersionInfo) {}
 
@@ -19,7 +20,11 @@ export abstract class CryptoClient {
     return this.allSignaturesLength >= this.keyVersionInfo.threshold
   }
 
-  public addSignature(serviceResponse: ServicePartialSignature): void {
+  public addSignature(serviceResponse: ServicePartialSignature, ctx: Context): void {
+    if (!this.allSignaturesLength) {
+      // start timer when first signer responds
+      this.tailLatencyTimer = Histograms.signerTailLatency.labels(ctx.url).startTimer()
+    }
     this.unverifiedSignatures.push(serviceResponse)
   }
 
@@ -40,7 +45,10 @@ export abstract class CryptoClient {
       )
     }
 
-    const timer = Histograms.signatureAggregationLatency.startTimer()
+    // Once we reach this point, we've received a quorum of signer responses
+    this.tailLatencyTimer()
+
+    const timer = Histograms.signatureAggregationLatency.labels(ctx.url).startTimer()
     const combinedSignature = this._combineBlindedSignatureShares(blindedMessage, ctx)
     timer()
 
diff --git a/packages/phone-number-privacy/combiner/src/common/handlers.ts b/packages/phone-number-privacy/combiner/src/common/handlers.ts
index 34ec2d143..f02e2a0ba 100644
--- a/packages/phone-number-privacy/combiner/src/common/handlers.ts
+++ b/packages/phone-number-privacy/combiner/src/common/handlers.ts
@@ -102,10 +102,9 @@ export function meteringHandler<R extends OdisRequest>(
       logger.info({ req: req.body }, 'Request received')
       Counters.requests.labels(req.url).inc()
 
-      const eventLoopLagMeasurementStart = Date.now()
+      const eventLoopLagTimer = Histograms.eventLoopLag.labels(req.url).startTimer()
       setTimeout(() => {
-        const eventLoopLag = Date.now() - eventLoopLagMeasurementStart
-        Histograms.eventLoopLag.labels(req.url).observe(eventLoopLag)
+        eventLoopLagTimer()
       })
 
       await handler(req, res)
diff --git a/packages/phone-number-privacy/combiner/src/common/metrics.ts b/packages/phone-number-privacy/combiner/src/common/metrics.ts
index 0d95a6668..b46e045c6 100644
--- a/packages/phone-number-privacy/combiner/src/common/metrics.ts
+++ b/packages/phone-number-privacy/combiner/src/common/metrics.ts
@@ -120,6 +120,12 @@ export const Histograms = {
     labelNames: ['endpoint'],
     buckets,
   }),
+  signerTailLatency: new Histogram({
+    name: 'signer_tail_latency',
+    help: 'Histogram of latency discrepencies between the fastest and slowest signer in a quorum',
+    labelNames: ['endpoint'],
+    buckets,
+  }),
 }
 
 export function newMeter(
diff --git a/packages/phone-number-privacy/combiner/src/domain/endpoints/sign/action.ts b/packages/phone-number-privacy/combiner/src/domain/endpoints/sign/action.ts
index 3fda28ada..776416d9c 100644
--- a/packages/phone-number-privacy/combiner/src/domain/endpoints/sign/action.ts
+++ b/packages/phone-number-privacy/combiner/src/domain/endpoints/sign/action.ts
@@ -57,7 +57,7 @@ export function domainSign(
     ): Promise<boolean> => {
       assert(res.success)
       // TODO remove the need to pass url here
-      crypto.addSignature({ url: request.url, signature: res.signature })
+      crypto.addSignature({ url: request.url, signature: res.signature }, ctx)
 
       // Send response immediately once we cross threshold
       // BLS threshold signatures can be combined without all partial signatures
diff --git a/packages/phone-number-privacy/combiner/src/pnp/endpoints/sign/action.ts b/packages/phone-number-privacy/combiner/src/pnp/endpoints/sign/action.ts
index f500fa52c..59b180bfa 100644
--- a/packages/phone-number-privacy/combiner/src/pnp/endpoints/sign/action.ts
+++ b/packages/phone-number-privacy/combiner/src/pnp/endpoints/sign/action.ts
@@ -73,7 +73,7 @@ export function pnpSign(
 
     const processResult = async (result: OdisResponse<SignMessageRequest>): Promise<boolean> => {
       assert(result.success)
-      crypto.addSignature({ url: request.url, signature: result.signature })
+      crypto.addSignature({ url: request.url, signature: result.signature }, ctx)
 
       // Send response immediately once we cross threshold
       // BLS threshold signatures can be combined without all partial signatures