refactor(test): use the configurable logger consistently via top leve…

…l interface of crucible (#6871) * Use the logger consistently * Fix types * Fix lint errors * Fix types
ChainSafe · Jun 10, 2024 · ff253a7 · ff253a7 · github-actions · Jun 10, 2024
1 parent 4ef27ac
commit ff253a7
Show file tree

Hide file tree

Showing 12 changed files with 206 additions and 227 deletions.
diff --git a/packages/cli/test/utils/crucible/clients/beacon/lighthouse.ts b/packages/cli/test/utils/crucible/clients/beacon/lighthouse.ts
@@ -92,12 +92,11 @@ export const generateLighthouseBeaconNode: BeaconNodeGenerator<BeaconClient.Ligh
       health: async () => {
         try {
           await got.get(`http://127.0.0.1:${ports.beacon.httpPort}/eth/v1/node/health`);
-          return {ok: true};
         } catch (err) {
           if (err instanceof RequestError && err.code !== "ECONNREFUSED") {
-            return {ok: true};
+            return;
           }
-          return {ok: false, reason: (err as Error).message, checkId: "/eth/v1/node/health query"};
+          throw err;
         }
       },
     },

diff --git a/packages/cli/test/utils/crucible/clients/beacon/lodestar.ts b/packages/cli/test/utils/crucible/clients/beacon/lodestar.ts
@@ -95,12 +95,7 @@ export const generateLodestarBeaconNode: BeaconNodeGenerator<BeaconClient.Lodest
         stdoutFilePath: logFilePath,
       },
       health: async () => {
-        try {
-          await got.get(`http://${address}:${ports.beacon.httpPort}/eth/v1/node/health`);
-          return {ok: true};
-        } catch (err) {
-          return {ok: false, reason: (err as Error).message, checkId: "eth/v1/node/health query"};
-        }
+        await got.get(`http://${address}:${ports.beacon.httpPort}/eth/v1/node/health`);
       },
     },
   ]);

diff --git a/packages/cli/test/utils/crucible/clients/execution/geth.ts b/packages/cli/test/utils/crucible/clients/execution/geth.ts
@@ -154,12 +154,7 @@ export const generateGethNode: ExecutionNodeGenerator<ExecutionClient.Geth> = (o
       stdoutFilePath: logFilePath,
     },
     health: async () => {
-      try {
-        await got.post(ethRpcPublicUrl, {json: {jsonrpc: "2.0", method: "net_version", params: [], id: 67}});
-        return {ok: true};
-      } catch (err) {
-        return {ok: false, reason: (err as Error).message, checkId: "JSON RPC query net_version"};
-      }
+      await got.post(ethRpcPublicUrl, {json: {jsonrpc: "2.0", method: "net_version", params: [], id: 67}});
     },
   };
 

diff --git a/packages/cli/test/utils/crucible/clients/execution/nethermind.ts b/packages/cli/test/utils/crucible/clients/execution/nethermind.ts
@@ -118,12 +118,7 @@ export const generateNethermindNode: ExecutionNodeGenerator<ExecutionClient.Neth
       stdoutFilePath: logFilePath,
     },
     health: async () => {
-      try {
-        await got.post(ethRpcPublicUrl, {json: {jsonrpc: "2.0", method: "net_version", params: [], id: 67}});
-        return {ok: true};
-      } catch (err) {
-        return {ok: false, reason: (err as Error).message, checkId: "JSON RPC query net_version"};
-      }
+      await got.post(ethRpcPublicUrl, {json: {jsonrpc: "2.0", method: "net_version", params: [], id: 67}});
     },
   };
 

diff --git a/packages/cli/test/utils/crucible/clients/validator/lighthouse.ts b/packages/cli/test/utils/crucible/clients/validator/lighthouse.ts
@@ -83,12 +83,11 @@ export const generateLighthouseValidatorNode: ValidatorNodeGenerator<ValidatorCl
       health: async () => {
         try {
           await got.get(`http://127.0.0.1:${ports.validator.keymanagerPort}/lighthouse/health`);
-          return {ok: true};
         } catch (err) {
           if (err instanceof RequestError) {
-            return {ok: true};
+            return;
           }
-          return {ok: false, reason: (err as Error).message, checkId: "/lighthouse/health query"};
+          throw err;
         }
       },
     },

diff --git a/packages/cli/test/utils/crucible/clients/validator/lodestar.ts b/packages/cli/test/utils/crucible/clients/validator/lodestar.ts
@@ -69,12 +69,7 @@ export const generateLodestarValidatorNode: ValidatorNodeGenerator<ValidatorClie
         stdoutFilePath: logFilePath,
       },
       health: async () => {
-        try {
-          await got.get(`http://127.0.0.1:${ports.validator.keymanagerPort}/eth/v1/keystores`);
-          return {ok: true};
-        } catch (err) {
-          return {ok: false, reason: (err as Error).message, checkId: "eth/v1/keystores query"};
-        }
+        await got.get(`http://127.0.0.1:${ports.validator.keymanagerPort}/eth/v1/keystores`);
       },
     },
   ]);

diff --git a/packages/cli/test/utils/crucible/interfaces.ts b/packages/cli/test/utils/crucible/interfaces.ts
@@ -7,7 +7,7 @@ import {ApiClient as KeyManagerApi} from "@lodestar/api/keymanager";
 import {ChainForkConfig} from "@lodestar/config";
 import {ForkName} from "@lodestar/params";
 import {Slot, allForks, Epoch} from "@lodestar/types";
-import {Logger} from "@lodestar/logger";
+import {LogLevel, Logger} from "@lodestar/logger";
 import {BeaconArgs} from "../../../src/cmds/beacon/options.js";
 import {IValidatorCliArgs} from "../../../src/cmds/validator/options.js";
 import {GlobalArgs} from "../../../src/options/index.js";
@@ -29,6 +29,7 @@ export type SimulationOptions = {
   controller: AbortController;
   genesisTime: number;
   trustedSetup?: boolean;
+  logLevel?: LogLevel;
 };
 
 export enum BeaconClient {
@@ -234,8 +235,6 @@ export type ExecutionNodeGenerator<E extends ExecutionClient> = (
   runner: IRunner
 ) => ExecutionNode;
 
-export type HealthStatus = {ok: true} | {ok: false; reason: string; checkId: string};
-
 export type JobOptions<T extends RunnerType = RunnerType.ChildProcess | RunnerType.Docker> = {
   readonly id: string;
 
@@ -257,7 +256,7 @@ export type JobOptions<T extends RunnerType = RunnerType.ChildProcess | RunnerTy
 
   // Will be called frequently to check the health of job startup
   // If not present then wait for the job to exit
-  health?(): Promise<HealthStatus>;
+  health?(): Promise<void>;
 
   // Called once before the `job.start` is called
   bootstrap?(): Promise<void>;

diff --git a/packages/cli/test/utils/crucible/runner/childProcessRunner.ts b/packages/cli/test/utils/crucible/runner/childProcessRunner.ts
@@ -1,15 +1,15 @@
 import {ChildProcess} from "node:child_process";
-import {
-  spawnChildProcess,
-  stopChildProcess,
-  ChildProcessHealthStatus,
-  SpawnChildProcessOptions,
-  ChildProcessResolve,
-} from "@lodestar/test-utils";
+import {spawnChildProcess, stopChildProcess, SpawnChildProcessOptions, ChildProcessResolve} from "@lodestar/test-utils";
+import {Logger} from "@lodestar/logger";
 import {Job, JobOptions, RunnerEnv, RunnerType} from "../interfaces.js";
 
 export class ChildProcessRunner implements RunnerEnv<RunnerType.ChildProcess> {
   type = RunnerType.ChildProcess as const;
+  private logger: Logger;
+
+  constructor(opts: {logger: Logger}) {
+    this.logger = opts.logger;
+  }
 
   create(jobOption: Omit<JobOptions<RunnerType.ChildProcess>, "children">): Job {
     let childProcess: ChildProcess;
@@ -24,14 +24,7 @@ export class ChildProcessRunner implements RunnerEnv<RunnerType.ChildProcess> {
 
     if (health) {
       spawnOpts.healthTimeoutMs = 30000;
-      spawnOpts.health = async (): Promise<ChildProcessHealthStatus> =>
-        health()
-          .then((status) => {
-            return status.ok ? {healthy: true} : {healthy: false};
-          })
-          .catch((error) => {
-            return {healthy: false, message: (error as Error).message};
-          });
+      spawnOpts.health = health;
     } else {
       spawnOpts.resolveOn = ChildProcessResolve.Completion;
     }

diff --git a/packages/cli/test/utils/crucible/runner/dockerRunner.ts b/packages/cli/test/utils/crucible/runner/dockerRunner.ts
@@ -1,32 +1,26 @@
 import {ChildProcess} from "node:child_process";
+import {Logger} from "@lodestar/logger";
 import {sleep} from "@lodestar/utils";
-import {
-  ChildProcessHealthStatus,
-  SpawnChildProcessOptions,
-  execChildProcess,
-  spawnChildProcess,
-  ChildProcessResolve,
-} from "@lodestar/test-utils";
+import {SpawnChildProcessOptions, execChildProcess, spawnChildProcess, ChildProcessResolve} from "@lodestar/test-utils";
 import {Job, JobOptions, RunnerEnv, RunnerType} from "../interfaces.js";
 
 const dockerNetworkIpRange = "192.168.0";
 const dockerNetworkName = "sim-env-net";
 
 export class DockerRunner implements RunnerEnv<RunnerType.Docker> {
   type = RunnerType.Docker as const;
+  private logger: Logger;
 
   private ipIndex = 2;
-  private logFilePath: string;
 
-  constructor(logFilePath: string) {
-    this.logFilePath = logFilePath;
+  constructor(opts: {logger: Logger}) {
+    this.logger = opts.logger;
   }
 
   async start(): Promise<void> {
     try {
       await execChildProcess(`docker network create --subnet ${dockerNetworkIpRange}.0/24 ${dockerNetworkName}`, {
-        logPrefix: "docker-runner",
-        pipeStdioToFile: this.logFilePath,
+        logger: this.logger,
       });
     } catch {
       // During multiple sim tests files the network might already exist
@@ -38,8 +32,7 @@ export class DockerRunner implements RunnerEnv<RunnerType.Docker> {
     for (let i = 0; i < 5; i++) {
       try {
         await execChildProcess(`docker network rm ${dockerNetworkName}`, {
-          logPrefix: "docker-runner",
-          pipeStdioToFile: this.logFilePath,
+          logger: this.logger,
         });
         return;
       } catch {
@@ -97,14 +90,7 @@ export class DockerRunner implements RunnerEnv<RunnerType.Docker> {
 
     if (health) {
       spawnOpts.healthTimeoutMs = 30000;
-      spawnOpts.health = async (): Promise<ChildProcessHealthStatus> =>
-        health()
-          .then((status) => {
-            return status.ok ? {healthy: true} : {healthy: false};
-          })
-          .catch((error) => {
-            return {healthy: false, message: (error as Error).message};
-          });
+      spawnOpts.health = health;
     } else {
       spawnOpts.resolveOn = ChildProcessResolve.Completion;
     }
@@ -120,7 +106,7 @@ export class DockerRunner implements RunnerEnv<RunnerType.Docker> {
         }
         // TODO: Debug why stopping the process was not killing the container
         // await stopChildProcess(childProcess);
-        await execChildProcess(`docker stop ${jobOption.id} --time 2 || true`, {pipeStdioToParent: true});
+        await execChildProcess(`docker stop ${jobOption.id} --time 2 || true`, {logger: this.logger});
       },
     };
   }

diff --git a/packages/cli/test/utils/crucible/runner/index.ts b/packages/cli/test/utils/crucible/runner/index.ts
@@ -1,20 +1,19 @@
 import {EventEmitter} from "node:events";
-import path from "node:path";
-import {Logger} from "@lodestar/logger";
+import {LoggerNode} from "@lodestar/logger/node";
 import {IRunner, Job, JobOptions, RunnerEvent, RunnerType} from "../interfaces.js";
 import {ChildProcessRunner} from "./childProcessRunner.js";
 import {DockerRunner} from "./dockerRunner.js";
 
 export class Runner implements IRunner {
-  readonly logger: Logger;
+  readonly logger: LoggerNode;
   private emitter = new EventEmitter({captureRejections: true});
   private runners: {[RunnerType.ChildProcess]: ChildProcessRunner; [RunnerType.Docker]: DockerRunner};
 
-  constructor({logsDir, logger}: {logsDir: string; logger: Logger}) {
+  constructor({logger}: {logger: LoggerNode}) {
     this.logger = logger;
     this.runners = {
-      [RunnerType.ChildProcess]: new ChildProcessRunner(),
-      [RunnerType.Docker]: new DockerRunner(path.join(logsDir, "docker_runner.log")),
+      [RunnerType.ChildProcess]: new ChildProcessRunner({logger: this.logger.child({module: "cp"})}),
+      [RunnerType.Docker]: new DockerRunner({logger: this.logger.child({module: "docker"})}),
     };
   }
 
@@ -59,7 +58,7 @@ export class Runner implements IRunner {
         await job.start();
 
         this.emitter.emit("started", jobOption.id);
-        this.logger.info(`Started "${jobOption.id}" logFile=${jobOption.logs.stdoutFilePath}...`);
+        this.logger.debug(`Started "${jobOption.id}" logFile=${jobOption.logs.stdoutFilePath}...`);
 
         if (childrenJob) await childrenJob.start();
       });
@@ -71,7 +70,7 @@ export class Runner implements IRunner {
         if (childrenJob) await childrenJob.stop();
         await job.stop();
         this.emitter.emit("stopped", jobOption.id);
-        this.logger.info(`Stopped "${jobOption.id}"...`);
+        this.logger.debug(`Stopped "${jobOption.id}"...`);
       });
     }
 

diff --git a/packages/cli/test/utils/crucible/simulation.ts b/packages/cli/test/utils/crucible/simulation.ts
@@ -64,7 +64,10 @@ export class Simulation {
       timestampFormat: {
         format: TimestampFormatCode.DateRegular,
       },
-      file: {level: LogLevel.debug, filepath: path.join(options.logsDir, `simulation-${this.options.id}.log`)},
+      file: {
+        level: options.logLevel ?? LogLevel.debug,
+        filepath: path.join(options.logsDir, `simulation-${this.options.id}.log`),
+      },
     });
     this.clock = new EpochClock({
       genesisTime: this.options.genesisTime + this.forkConfig.GENESIS_DELAY,
@@ -74,7 +77,7 @@ export class Simulation {
     });
 
     this.externalSigner = new ExternalSignerServer([]);
-    this.runner = new Runner({logsDir: this.options.logsDir, logger: this.logger.child({module: "runner"})});
+    this.runner = new Runner({logger: this.logger});
     this.tracker = SimulationTracker.initWithDefaults({
       logsDir: options.logsDir,
       logger: this.logger,