Fix client monitoring rate limit errors

Ensure next interval only starts after previous request has finished
else we might send next request too early and run into rate limit errors.

docs/usage/client-monitoring.md

@@ -40,9 +40,7 @@ It is also possible to print out the data sent to the remote service by enabling
 ### Monitoring interval
 
 It is possible to adjust the interval between sending client stats to the remote service by setting the `--monitoring.interval` flag.
-It takes an integer value in milliseconds, the default is `62000` which means data is sent approximately once a minute.
-The extra 2 seconds are added to avoid rate limit errors when using *beaconcha.in* as they are currently really strict
-and minor delays in the previous request can cause the following request to fail.
+It takes an integer value in milliseconds, the default is `60000` which means data is sent once a minute.
 
 For example, setting an interval of `300000` would mean the data is only sent every 5 minutes.
 

packages/beacon-node/src/monitoring/options.ts

@@ -13,9 +13,7 @@ export type MonitoringOptions = {
 
 export const defaultMonitoringOptions: Required<MonitoringOptions> = {
   endpoint: "",
-  // default interval should be once a minute
-  // but need to add 2 seconds to avoid rate limit errors
-  interval: 62_000,
+  interval: 60_000,
   initialDelay: 30_000,
   requestTimeout: 10_000,
   collectSystemStats: true,

packages/beacon-node/src/monitoring/service.ts

@@ -43,7 +43,7 @@ export class MonitoringService {
 
   private status = Status.Stopped;
   private initialDelayTimeout?: NodeJS.Timeout;
-  private monitoringInterval?: NodeJS.Timer;
+  private monitoringInterval?: NodeJS.Timeout;
   private fetchAbortController?: AbortController;
   private pendingRequest?: Promise<void>;
 
@@ -84,10 +84,7 @@ export class MonitoringService {
 
     this.initialDelayTimeout = setTimeout(async () => {
       await this.send();
-
-      this.monitoringInterval = setInterval(async () => {
-        await this.send();
-      }, interval);
+      this.nextMonitoringInterval();
     }, initialDelay);
 
     this.logger.info("Started monitoring service", {
@@ -109,7 +106,7 @@ export class MonitoringService {
       clearTimeout(this.initialDelayTimeout);
     }
     if (this.monitoringInterval) {
-      clearInterval(this.monitoringInterval);
+      clearTimeout(this.monitoringInterval);
     }
     if (this.pendingRequest) {
       this.fetchAbortController?.abort(FetchAbortReason.Stop);
@@ -211,6 +208,15 @@ export class MonitoringService {
     }
   }
 
+  private nextMonitoringInterval(): void {
+    // ensure next interval only starts after previous request has finished
+    // else we might send next request too early and run into rate limit errors
+    this.monitoringInterval = setTimeout(async () => {
+      await this.send();
+      this.nextMonitoringInterval();
+    }, this.options.interval);
+  }
+
   private parseMonitoringEndpoint(endpoint: string): URL {
     if (!endpoint) {
       throw new Error("Monitoring endpoint must be provided");

packages/beacon-node/test/unit/monitoring/service.test.ts

@@ -1,5 +1,5 @@
 import {expect} from "chai";
-import sinon from "sinon";
+import sinon, {SinonSpy} from "sinon";
 import {ErrorAborted, Logger, TimeoutError} from "@lodestar/utils";
 import {RegistryMetricCreator} from "../../../src/index.js";
 import {HistogramExtra} from "../../../src/metrics/utils/histogram.js";
@@ -73,11 +73,12 @@ describe("monitoring / service", () => {
     });
 
     it("should set interval to continuously send client stats", async () => {
-      const setInterval = sandbox.spy(global, "setInterval");
+      const setTimeout = sandbox.spy(global, "setTimeout");
+      const interval = 1000;
 
-      const service = await startedMonitoringService();
+      const service = await startedMonitoringService({interval});
 
-      expect(setInterval).to.have.been.calledOnce;
+      expect(setTimeout).to.have.been.calledWithMatch({}, interval);
       expect(service["monitoringInterval"]).to.be.an("object");
     });
 
@@ -116,6 +117,12 @@ describe("monitoring / service", () => {
   });
 
   describe("MonitoringService - stop", () => {
+    let clearTimeout: SinonSpy;
+
+    before(() => {
+      clearTimeout = sandbox.spy(global, "clearTimeout");
+    });
+
     it("should set the status to stopped", async () => {
       const service = await startedMonitoringService();
 
@@ -125,23 +132,19 @@ describe("monitoring / service", () => {
     });
 
     it("should clear the monitoring interval", async () => {
-      const clearInterval = sandbox.spy(global, "clearInterval");
-
       const service = await startedMonitoringService();
 
       service.stop();
 
-      expect(clearInterval).to.have.been.calledOnceWith(service["monitoringInterval"]);
+      expect(clearTimeout).to.have.been.calledWith(service["monitoringInterval"]);
     });
 
     it("should clear the initial delay timeout", async () => {
-      const clearTimeout = sandbox.spy(global, "clearTimeout");
-
       const service = await startedMonitoringService({initialDelay: 1000});
 
       service.stop();
 
-      expect(clearTimeout).to.have.been.calledOnceWith(service["initialDelayTimeout"]);
+      expect(clearTimeout).to.have.been.calledWith(service["initialDelayTimeout"]);
     });
 
     it("should abort pending requests", async () => {

packages/cli/src/cmds/validator/options.ts

@@ -18,7 +18,7 @@ export const validatorMetricsDefaultOptions = {
 };
 
 export const validatorMonitoringDefaultOptions = {
-  interval: 62_000,
+  interval: 60_000,
   initialDelay: 30_000,
   requestTimeout: 10_000,
   collectSystemStats: false,

packages/cli/test/unit/options/beaconNodeOptions.test.ts

@@ -54,7 +54,7 @@ describe("options / beaconNodeOptions", () => {
       "metrics.address": "0.0.0.0",
 
       "monitoring.endpoint": "https://beaconcha.in/api/v1/client/metrics?apikey=secretKey&machine=machine1",
-      "monitoring.interval": 62000,
+      "monitoring.interval": 60000,
       "monitoring.initialDelay": 30000,
       "monitoring.requestTimeout": 10000,
       "monitoring.collectSystemStats": true,
@@ -142,7 +142,7 @@ describe("options / beaconNodeOptions", () => {
       },
       monitoring: {
         endpoint: "https://beaconcha.in/api/v1/client/metrics?apikey=secretKey&machine=machine1",
-        interval: 62000,
+        interval: 60000,
         initialDelay: 30000,
         requestTimeout: 10000,
         collectSystemStats: true,