feat: log agent version when client is not known (#6314)

* feat: log agent version when client is not known * chore: add test case for getKnownClientFromAgentVersion
ChainSafe · Jan 19, 2024 · c423687 · c423687
1 parent 3215bdd
commit c423687
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 10 deletions.
diff --git a/packages/beacon-node/src/network/peers/client.ts b/packages/beacon-node/src/network/peers/client.ts
@@ -7,7 +7,13 @@ export enum ClientKind {
   Unknown = "Unknown",
 }
 
-export function clientFromAgentVersion(agentVersion: string): ClientKind {
+/**
+ * Get known client from agent version.
+ * If client is not known, don't return ClientKind.Unknown here.
+ * For metrics it'll have fallback logic to use ClientKind.Unknown
+ * For logs, we want to print out agentVersion instead for debugging purposes.
+ */
+export function getKnownClientFromAgentVersion(agentVersion: string): ClientKind | null {
   const slashIndex = agentVersion.indexOf("/");
   const agent = slashIndex >= 0 ? agentVersion.slice(0, slashIndex) : agentVersion;
   const agentLC = agent.toLowerCase();
@@ -16,5 +22,6 @@ export function clientFromAgentVersion(agentVersion: string): ClientKind {
   if (agentLC === "prysm") return ClientKind.Prysm;
   if (agentLC === "nimbus") return ClientKind.Nimbus;
   if (agentLC === "lodestar" || agentLC === "js-libp2p") return ClientKind.Lodestar;
-  return ClientKind.Unknown;
+
+  return null;
 }
diff --git a/packages/beacon-node/src/network/peers/peerManager.ts b/packages/beacon-node/src/network/peers/peerManager.ts
@@ -19,7 +19,7 @@ import {NetworkCoreMetrics} from "../core/metrics.js";
 import {LodestarDiscv5Opts} from "../discv5/types.js";
 import {PeerDiscovery, SubnetDiscvQueryMs} from "./discover.js";
 import {PeersData, PeerData} from "./peersData.js";
-import {clientFromAgentVersion, ClientKind} from "./client.js";
+import {getKnownClientFromAgentVersion, ClientKind} from "./client.js";
 import {
   getConnectedPeerIds,
   hasSomeConnectedPeer,
@@ -615,7 +615,7 @@ export class PeerManager {
         if (agentVersionBytes) {
           const agentVersion = new TextDecoder().decode(agentVersionBytes) || "N/A";
           peerData.agentVersion = agentVersion;
-          peerData.agentClient = clientFromAgentVersion(agentVersion);
+          peerData.agentClient = getKnownClientFromAgentVersion(agentVersion);
         }
       },
       {retries: 3, retryDelay: 1000}

diff --git a/packages/beacon-node/src/network/peers/peersData.ts b/packages/beacon-node/src/network/peers/peersData.ts
@@ -38,8 +38,8 @@ export class PeersData {
     return this.connectedPeers.get(peerIdStr)?.agentVersion ?? "NA";
   }
 
-  getPeerKind(peerIdStr: string): ClientKind {
-    return this.connectedPeers.get(peerIdStr)?.agentClient ?? ClientKind.Unknown;
+  getPeerKind(peerIdStr: string): ClientKind | null {
+    return this.connectedPeers.get(peerIdStr)?.agentClient ?? null;
   }
 
   getEncodingPreference(peerIdStr: string): Encoding | null {

diff --git a/packages/beacon-node/src/network/reqresp/ReqRespBeaconNode.ts b/packages/beacon-node/src/network/reqresp/ReqRespBeaconNode.ts
@@ -88,7 +88,8 @@ export class ReqRespBeaconNode extends ReqResp {
           metrics?.reqResp.rateLimitErrors.inc({method});
         },
         getPeerLogMetadata(peerId) {
-          return peersData.getPeerKind(peerId);
+          // this logs the whole agent version for unknown client which is good for debugging
+          return peersData.getPeerKind(peerId) ?? peersData.getAgentVersion(peerId);
         },
       }
     );

diff --git a/packages/beacon-node/test/unit/network/peers/client.test.ts b/packages/beacon-node/test/unit/network/peers/client.test.ts
@@ -1,8 +1,8 @@
 import {describe, it, expect} from "vitest";
-import {clientFromAgentVersion, ClientKind} from "../../../../src/network/peers/client.js";
+import {getKnownClientFromAgentVersion, ClientKind} from "../../../../src/network/peers/client.js";
 
 describe("clientFromAgentVersion", () => {
-  const testCases: {name: string; agentVersion: string; client: ClientKind}[] = [
+  const testCases: {name: string; agentVersion: string; client: ClientKind | null}[] = [
     {
       name: "lighthouse",
       agentVersion: "Lighthouse/v2.0.1-fff01b2/x86_64-linux",
@@ -28,11 +28,16 @@ describe("clientFromAgentVersion", () => {
       agentVersion: "js-libp2p/0.32.4",
       client: ClientKind.Lodestar,
     },
+    {
+      name: "unknown client",
+      agentVersion: "strange-client-agent-version",
+      client: null,
+    },
   ];
 
   for (const {name, agentVersion, client} of testCases) {
     it(name, () => {
-      expect(clientFromAgentVersion(agentVersion)).toBe(client);
+      expect(getKnownClientFromAgentVersion(agentVersion)).toBe(client);
     });
   }
 });