refactor: type safe metric labels (#6201)

* refactor: type safe metric labels * Update metrics after merging unstable * Remove collect method from GaugeExtra * Remove startTimer method from Gauge interface * Default to NoLabels to fix type issues * Allow to partially set labels in startTimer * Fix type compatibility with prom-client Histogram * Sort metric types * chore: update prom-client to v15.1.0 (#6230) * Add metric type tests
ChainSafe · Jan 2, 2024 · b7925f6 · b7925f6
1 parent 42baaeb
commit b7925f6
Show file tree

Hide file tree

Showing 55 changed files with 646 additions and 686 deletions.
diff --git a/packages/api/src/utils/client/metrics.ts b/packages/api/src/utils/client/metrics.ts
@@ -1,49 +1,9 @@
+import {Gauge, GaugeExtra, Histogram} from "@lodestar/utils";
+
 export type Metrics = {
-  requestTime: Histogram<"routeId">;
-  streamTime: Histogram<"routeId">;
-  requestErrors: Gauge<"routeId">;
-  requestToFallbacks: Gauge<"routeId">;
-  urlsScore: Gauge<"urlIndex">;
+  requestTime: Histogram<{routeId: string}>;
+  streamTime: Histogram<{routeId: string}>;
+  requestErrors: Gauge<{routeId: string}>;
+  requestToFallbacks: Gauge<{routeId: string}>;
+  urlsScore: GaugeExtra<{urlIndex: number}>;
 };
-
-type LabelValues<T extends string> = Partial<Record<T, string | number>>;
-type CollectFn<T extends string> = (metric: Gauge<T>) => void;
-
-export interface Gauge<T extends string> {
-  /**
-   * Increment gauge for given labels
-   * @param labels Object with label keys and values
-   * @param value The value to increment with
-   */
-  inc(labels: LabelValues<T>, value?: number): void;
-
-  /**
-   * Increment gauge
-   * @param value The value to increment with
-   */
-  inc(value?: number): void;
-
-  /**
-   * Set gauge value for labels
-   * @param labels Object with label keys and values
-   * @param value The value to set
-   */
-  set(labels: LabelValues<T>, value: number): void;
-
-  /**
-   * Set gauge value
-   * @param value The value to set
-   */
-  set(value: number): void;
-
-  addCollect(collectFn: CollectFn<T>): void;
-}
-
-export interface Histogram<T extends string> {
-  /**
-   * Start a timer where the value in seconds will observed
-   * @param labels Object with label keys and values
-   * @return Function to invoke when timer should be stopped
-   */
-  startTimer(labels?: LabelValues<T>): (labels?: LabelValues<T>) => number;
-}
diff --git a/packages/beacon-node/package.json b/packages/beacon-node/package.json
@@ -145,7 +145,7 @@
     "jwt-simple": "0.5.6",
     "libp2p": "0.46.12",
     "multiformats": "^11.0.1",
-    "prom-client": "^14.2.0",
+    "prom-client": "^15.1.0",
     "qs": "^6.11.1",
     "snappyjs": "^0.7.0",
     "strict-event-emitter-types": "^2.0.0",

diff --git a/packages/beacon-node/src/api/rest/activeSockets.ts b/packages/beacon-node/src/api/rest/activeSockets.ts
@@ -1,12 +1,11 @@
 import http, {Server} from "node:http";
 import {Socket} from "node:net";
-import {waitFor} from "@lodestar/utils";
-import {IGauge} from "../../metrics/index.js";
+import {Gauge, GaugeExtra, waitFor} from "@lodestar/utils";
 
 export type SocketMetrics = {
-  activeSockets: IGauge;
-  socketsBytesRead: IGauge;
-  socketsBytesWritten: IGauge;
+  activeSockets: GaugeExtra;
+  socketsBytesRead: Gauge;
+  socketsBytesWritten: Gauge;
 };
 
 // Use relatively short timeout to speed up shutdown

diff --git a/packages/beacon-node/src/api/rest/base.ts b/packages/beacon-node/src/api/rest/base.ts
@@ -3,9 +3,8 @@ import fastify, {FastifyInstance} from "fastify";
 import fastifyCors from "@fastify/cors";
 import bearerAuthPlugin from "@fastify/bearer-auth";
 import {RouteConfig} from "@lodestar/api/beacon/server";
-import {ErrorAborted, Logger} from "@lodestar/utils";
+import {ErrorAborted, Gauge, Histogram, Logger} from "@lodestar/utils";
 import {isLocalhostIP} from "../../util/ip.js";
-import {IGauge, IHistogram} from "../../metrics/index.js";
 import {ApiError, NodeIsSyncing} from "../impl/errors.js";
 import {HttpActiveSocketsTracker, SocketMetrics} from "./activeSockets.js";
 
@@ -25,9 +24,9 @@ export type RestApiServerModules = {
 };
 
 export type RestApiServerMetrics = SocketMetrics & {
-  requests: IGauge<"operationId">;
-  responseTime: IHistogram<"operationId">;
-  errors: IGauge<"operationId">;
+  requests: Gauge<{operationId: string}>;
+  responseTime: Histogram<{operationId: string}>;
+  errors: Gauge<{operationId: string}>;
 };
 
 /**

diff --git a/packages/beacon-node/src/chain/blocks/verifyBlocksStateTransitionOnly.ts b/packages/beacon-node/src/chain/blocks/verifyBlocksStateTransitionOnly.ts
@@ -3,6 +3,7 @@ import {
   stateTransition,
   ExecutionPayloadStatus,
   DataAvailableStatus,
+  StateHashTreeRootSource,
 } from "@lodestar/state-transition";
 import {ErrorAborted, Logger, sleep} from "@lodestar/utils";
 import {Metrics} from "../../metrics/index.js";
@@ -57,7 +58,9 @@ export async function verifyBlocksStateTransitionOnly(
       metrics
     );
 
-    const hashTreeRootTimer = metrics?.stateHashTreeRootTime.startTimer({source: "block_transition"});
+    const hashTreeRootTimer = metrics?.stateHashTreeRootTime.startTimer({
+      source: StateHashTreeRootSource.blockTransition,
+    });
     const stateRoot = postState.hashTreeRoot();
     hashTreeRootTimer?.();
 

diff --git a/packages/beacon-node/src/chain/bls/index.ts b/packages/beacon-node/src/chain/bls/index.ts
@@ -1,4 +1,4 @@
 export type {IBlsVerifier} from "./interface.js";
-export type {BlsMultiThreadWorkerPoolModules} from "./multithread/index.js";
+export type {BlsMultiThreadWorkerPoolModules, JobQueueItemType} from "./multithread/index.js";
 export {BlsMultiThreadWorkerPool} from "./multithread/index.js";
 export {BlsSingleThreadVerifier} from "./singleThread.js";
diff --git a/packages/beacon-node/src/chain/bls/multithread/index.ts b/packages/beacon-node/src/chain/bls/multithread/index.ts
@@ -41,6 +41,8 @@ export type BlsMultiThreadWorkerPoolOptions = {
   blsVerifyAllMultiThread?: boolean;
 };
 
+export type {JobQueueItemType};
+
 // 1 worker for the main thread
 const blsPoolSize = Math.max(defaultPoolSize - 1, 1);
 

diff --git a/packages/beacon-node/src/chain/opPools/opPool.ts b/packages/beacon-node/src/chain/opPools/opPool.ts
@@ -19,6 +19,7 @@ import {IBeaconDb} from "../../db/index.js";
 import {SignedBLSToExecutionChangeVersioned} from "../../util/types.js";
 import {BlockType} from "../interface.js";
 import {Metrics} from "../../metrics/metrics.js";
+import {BlockProductionStep} from "../produceBlock/produceBlockBody.js";
 import {isValidBlsToExecutionChangeForBlockInclusion} from "./utils.js";
 
 type HexRoot = string;
@@ -201,7 +202,7 @@ export class OpPool {
       }
     }
     endProposerSlashing?.({
-      step: "proposerSlashing",
+      step: BlockProductionStep.proposerSlashing,
     });
 
     const endAttesterSlashings = stepsMetrics?.startTimer();
@@ -235,7 +236,7 @@ export class OpPool {
       }
     }
     endAttesterSlashings?.({
-      step: "attesterSlashings",
+      step: BlockProductionStep.attesterSlashings,
     });
 
     const endVoluntaryExits = stepsMetrics?.startTimer();
@@ -256,7 +257,7 @@ export class OpPool {
       }
     }
     endVoluntaryExits?.({
-      step: "voluntaryExits",
+      step: BlockProductionStep.voluntaryExits,
     });
 
     const endBlsToExecutionChanges = stepsMetrics?.startTimer();
@@ -270,7 +271,7 @@ export class OpPool {
       }
     }
     endBlsToExecutionChanges?.({
-      step: "blsToExecutionChanges",
+      step: BlockProductionStep.blsToExecutionChanges,
     });
 
     return [attesterSlashings, proposerSlashings, voluntaryExits, blsToExecutionChanges];

diff --git a/packages/beacon-node/src/chain/prepareNextSlot.ts b/packages/beacon-node/src/chain/prepareNextSlot.ts
@@ -1,4 +1,9 @@
-import {computeEpochAtSlot, isExecutionStateType, computeTimeAtSlot} from "@lodestar/state-transition";
+import {
+  computeEpochAtSlot,
+  isExecutionStateType,
+  computeTimeAtSlot,
+  StateHashTreeRootSource,
+} from "@lodestar/state-transition";
 import {ChainForkConfig} from "@lodestar/config";
 import {ForkSeq, SLOTS_PER_EPOCH, ForkExecution} from "@lodestar/params";
 import {Slot} from "@lodestar/types";
@@ -106,7 +111,9 @@ export class PrepareNextSlotScheduler {
 
       // cache HashObjects for faster hashTreeRoot() later, especially for computeNewStateRoot() if we need to produce a block at slot 0 of epoch
       // see https://github.com/ChainSafe/lodestar/issues/6194
-      const hashTreeRootTimer = this.metrics?.stateHashTreeRootTime.startTimer({source: "prepare_next_slot"});
+      const hashTreeRootTimer = this.metrics?.stateHashTreeRootTime.startTimer({
+        source: StateHashTreeRootSource.prepareNextSlot,
+      });
       prepareState.hashTreeRoot();
       hashTreeRootTimer?.();
 

diff --git a/packages/beacon-node/src/chain/produceBlock/computeNewStateRoot.ts b/packages/beacon-node/src/chain/produceBlock/computeNewStateRoot.ts
@@ -2,6 +2,7 @@ import {
   CachedBeaconStateAllForks,
   DataAvailableStatus,
   ExecutionPayloadStatus,
+  StateHashTreeRootSource,
   stateTransition,
 } from "@lodestar/state-transition";
 import {allForks, Gwei, Root} from "@lodestar/types";
@@ -44,7 +45,9 @@ export function computeNewStateRoot(
   const {attestations, syncAggregate, slashing} = postState.proposerRewards;
   const proposerReward = BigInt(attestations + syncAggregate + slashing);
 
-  const hashTreeRootTimer = metrics?.stateHashTreeRootTime.startTimer({source: "compute_new_state_root"});
+  const hashTreeRootTimer = metrics?.stateHashTreeRootTime.startTimer({
+    source: StateHashTreeRootSource.computeNewStateRoot,
+  });
   const newStateRoot = postState.hashTreeRoot();
   hashTreeRootTimer?.();
 

diff --git a/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts b/packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts
@@ -39,10 +39,26 @@ import {validateBlobsAndKzgCommitments} from "./validateBlobsAndKzgCommitments.j
 
 // Time to provide the EL to generate a payload from new payload id
 const PAYLOAD_GENERATION_TIME_MS = 500;
-enum PayloadPreparationType {
+
+export enum PayloadPreparationType {
   Fresh = "Fresh",
   Cached = "Cached",
   Reorged = "Reorged",
+  Blinded = "Blinded",
+}
+
+/**
+ * Block production steps tracked in metrics
+ */
+export enum BlockProductionStep {
+  proposerSlashing = "proposerSlashing",
+  attesterSlashings = "attesterSlashings",
+  voluntaryExits = "voluntaryExits",
+  blsToExecutionChanges = "blsToExecutionChanges",
+  attestations = "attestations",
+  eth1DataAndDeposits = "eth1DataAndDeposits",
+  syncAggregate = "syncAggregate",
+  executionPayload = "executionPayload",
 }
 
 export type BlockAttributes = {
@@ -131,13 +147,13 @@ export async function produceBlockBody<T extends BlockType>(
   const endAttestations = stepsMetrics?.startTimer();
   const attestations = this.aggregatedAttestationPool.getAttestationsForBlock(this.forkChoice, currentState);
   endAttestations?.({
-    step: "attestations",
+    step: BlockProductionStep.attestations,
   });
 
   const endEth1DataAndDeposits = stepsMetrics?.startTimer();
   const {eth1Data, deposits} = await this.eth1.getEth1DataAndDeposits(currentState);
   endEth1DataAndDeposits?.({
-    step: "eth1DataAndDeposits",
+    step: BlockProductionStep.eth1DataAndDeposits,
   });
 
   const blockBody: phase0.BeaconBlockBody = {
@@ -162,7 +178,7 @@ export async function produceBlockBody<T extends BlockType>(
     (blockBody as altair.BeaconBlockBody).syncAggregate = syncAggregate;
   }
   endSyncAggregate?.({
-    step: "syncAggregate",
+    step: BlockProductionStep.syncAggregate,
   });
 
   Object.assign(logMeta, {
@@ -218,7 +234,7 @@ export async function produceBlockBody<T extends BlockType>(
       executionPayloadValue = builderRes.executionPayloadValue;
 
       const fetchedTime = Date.now() / 1000 - computeTimeAtSlot(this.config, blockSlot, this.genesisTime);
-      const prepType = "blinded";
+      const prepType = PayloadPreparationType.Blinded;
       this.metrics?.blockPayload.payloadFetchedTime.observe({prepType}, fetchedTime);
       this.logger.verbose("Fetched execution payload header from builder", {
         slot: blockSlot,
@@ -343,7 +359,7 @@ export async function produceBlockBody<T extends BlockType>(
     executionPayloadValue = BigInt(0);
   }
   endExecutionPayload?.({
-    step: "executionPayload",
+    step: BlockProductionStep.executionPayload,
   });
 
   if (ForkSeq[fork] >= ForkSeq.capella) {

diff --git a/packages/beacon-node/src/chain/regen/queued.ts b/packages/beacon-node/src/chain/regen/queued.ts
@@ -221,7 +221,7 @@ export class QueuedStateRegenerator implements IStateRegenerator {
   private jobQueueProcessor = async (regenRequest: RegenRequest): Promise<CachedBeaconStateAllForks> => {
     const metricsLabels = {
       caller: regenRequest.args[regenRequest.args.length - 1] as RegenCaller,
-      entrypoint: regenRequest.key,
+      entrypoint: regenRequest.key as RegenFnName,
     };
     let timer;
     try {

diff --git a/packages/beacon-node/src/chain/reprocess.ts b/packages/beacon-node/src/chain/reprocess.ts
@@ -11,7 +11,7 @@ export const REPROCESS_MIN_TIME_TO_NEXT_SLOT_SEC = 2;
 /**
  * Reprocess status for metrics
  */
-enum ReprocessStatus {
+export enum ReprocessStatus {
   /**
    * There are too many attestations that have unknown block root.
    */
@@ -140,7 +140,10 @@ export class ReprocessController {
         for (const awaitingPromise of awaitingPromisesByRoot.values()) {
           const {resolve, addedTimeMs} = awaitingPromise;
           resolve(false);
-          this.metrics?.reprocessApiAttestations.waitSecBeforeReject.set((now - addedTimeMs) / 1000);
+          this.metrics?.reprocessApiAttestations.waitSecBeforeReject.set(
+            {reason: ReprocessStatus.expired},
+            (now - addedTimeMs) / 1000
+          );
           this.metrics?.reprocessApiAttestations.reject.inc({reason: ReprocessStatus.expired});
         }
 

diff --git a/packages/beacon-node/src/chain/seenCache/seenAttestationData.ts b/packages/beacon-node/src/chain/seenCache/seenAttestationData.ts
@@ -17,7 +17,7 @@ export type AttestationDataCacheEntry = {
   subnet: number;
 };
 
-enum RejectReason {
+export enum RejectReason {
   // attestation data reaches MAX_CACHE_SIZE_PER_SLOT
   reached_limit = "reached_limit",
   // attestation data is too old

diff --git a/packages/beacon-node/src/chain/stateCache/mapMetrics.ts b/packages/beacon-node/src/chain/stateCache/mapMetrics.ts
@@ -1,8 +1,8 @@
-import {IAvgMinMax} from "../../metrics/index.js";
+import {AvgMinMax} from "@lodestar/utils";
 
 type MapTrackerMetrics = {
-  reads: IAvgMinMax;
-  secondsSinceLastRead: IAvgMinMax;
+  reads: AvgMinMax;
+  secondsSinceLastRead: AvgMinMax;
 };
 
 export class MapTracker<K, V> extends Map<K, V> {

diff --git a/packages/beacon-node/src/chain/validation/attestation.ts b/packages/beacon-node/src/chain/validation/attestation.ts
@@ -541,7 +541,7 @@ export function verifyHeadBlockAndTargetRoot(
   targetRoot: Root,
   attestationSlot: Slot,
   attestationEpoch: Epoch,
-  caller: string,
+  caller: RegenCaller,
   maxSkipSlots?: number
 ): ProtoBlock {
   const headBlock = verifyHeadBlockIsKnown(chain, beaconBlockRoot);

diff --git a/packages/beacon-node/src/eth1/provider/jsonRpcHttpClient.ts b/packages/beacon-node/src/eth1/provider/jsonRpcHttpClient.ts
@@ -1,8 +1,7 @@
 import {EventEmitter} from "events";
 import StrictEventEmitter from "strict-event-emitter-types";
 import {fetch} from "@lodestar/api";
-import {ErrorAborted, TimeoutError, isValidHttpUrl, retry} from "@lodestar/utils";
-import {IGauge, IHistogram} from "../../metrics/interface.js";
+import {ErrorAborted, Gauge, Histogram, TimeoutError, isValidHttpUrl, retry} from "@lodestar/utils";
 import {IJson, RpcPayload} from "../interface.js";
 import {JwtClaim, encodeJwtToken} from "./jwt.js";
 
@@ -58,13 +57,13 @@ export type ReqOpts = {
 };
 
 export type JsonRpcHttpClientMetrics = {
-  requestTime: IHistogram<"routeId">;
-  streamTime: IHistogram<"routeId">;
-  requestErrors: IGauge<"routeId">;
-  requestUsedFallbackUrl: IGauge<"routeId">;
-  activeRequests: IGauge<"routeId">;
-  configUrlsCount: IGauge;
-  retryCount: IGauge<"routeId">;
+  requestTime: Histogram<{routeId: string}>;
+  streamTime: Histogram<{routeId: string}>;
+  requestErrors: Gauge<{routeId: string}>;
+  requestUsedFallbackUrl: Gauge<{routeId: string}>;
+  activeRequests: Gauge<{routeId: string}>;
+  configUrlsCount: Gauge;
+  retryCount: Gauge<{routeId: string}>;
 };
 
 export interface IJsonRpcHttpClient {

diff --git a/packages/beacon-node/src/metrics/index.ts b/packages/beacon-node/src/metrics/index.ts
@@ -1,5 +1,4 @@
 export * from "./metrics.js";
 export * from "./server/index.js";
-export * from "./interface.js";
 export * from "./nodeJsMetrics.js";
 export {RegistryMetricCreator} from "./utils/registryMetricCreator.js";