From 4a110f65ff5dc73a422655f45077a1fab1d6bfc3 Mon Sep 17 00:00:00 2001
From: Ivan FB <128452529+Ivansete-status@users.noreply.github.com>
Date: Tue, 30 Apr 2024 15:07:17 +0200
Subject: [PATCH] refactor: metrics server. Simplify app.nim module (#2650)

---
 apps/wakunode2/wakunode2.nim |  3 ++-
 tests/wakunode2/test_app.nim |  4 +--
 waku/factory/app.nim         | 50 +++---------------------------------
 waku/node/waku_metrics.nim   | 35 ++++++++++++++++++++++++-
 4 files changed, 42 insertions(+), 50 deletions(-)

diff --git a/apps/wakunode2/wakunode2.nim b/apps/wakunode2/wakunode2.nim
index f14a1f97f..0e4ffc964 100644
--- a/apps/wakunode2/wakunode2.nim
+++ b/apps/wakunode2/wakunode2.nim
@@ -20,6 +20,7 @@ import
   ../../waku/factory/networks_config,
   ../../waku/factory/app,
   ../../waku/node/health_monitor,
+  ../../waku/node/waku_metrics,
   ../../waku/waku_api/rest/builder as rest_server_builder
 
 logScope:
@@ -152,7 +153,7 @@ when isMainModule:
       error "Starting protocols support REST server failed.", error = $error
       quit(QuitFailure)
 
-    wakunode2.startMetricsServerAndLogging().isOkOr:
+    wakunode2.metricsServer = waku_metrics.startMetricsServerAndLogging(conf).valueOr:
       error "Starting monitoring and external interfaces failed", error = error
       quit(QuitFailure)
 
diff --git a/tests/wakunode2/test_app.nim b/tests/wakunode2/test_app.nim
index 14ae0d74a..f96dea3f6 100644
--- a/tests/wakunode2/test_app.nim
+++ b/tests/wakunode2/test_app.nim
@@ -51,8 +51,8 @@ suite "Wakunode2 - App initialization":
     wakunode2.startApp().isOkOr:
       raiseAssert error
 
-    let mountRes = wakunode2.startMetricsServerAndLogging()
-    assert mountRes.isOk(), mountRes.error
+    wakunode2.metricsServer = waku_metrics.startMetricsServerAndLogging(conf).valueOr:
+      raiseAssert error
 
     ## Then
     let node = wakunode2.node
diff --git a/waku/factory/app.nim b/waku/factory/app.nim
index 3900cbb99..1ef3b0393 100644
--- a/waku/factory/app.nim
+++ b/waku/factory/app.nim
@@ -65,7 +65,7 @@ type
     node: WakuNode
 
     restServer*: WakuRestServerRef
-    metricsServer: Option[MetricsHttpServerRef]
+    metricsServer*: MetricsHttpServerRef
 
   AppResult*[T] = Result[T, string]
 
@@ -273,56 +273,14 @@ proc startApp*(app: var App): AppResult[void] =
 
   return ok()
 
-proc startMetricsServer(
-    serverIp: IpAddress, serverPort: Port
-): AppResult[MetricsHttpServerRef] =
-  info "Starting metrics HTTP server", serverIp = $serverIp, serverPort = $serverPort
-
-  let metricsServerRes = MetricsHttpServerRef.new($serverIp, serverPort)
-  if metricsServerRes.isErr():
-    return err("metrics HTTP server start failed: " & $metricsServerRes.error)
-
-  let server = metricsServerRes.value
-  try:
-    waitFor server.start()
-  except CatchableError:
-    return err("metrics HTTP server start failed: " & getCurrentExceptionMsg())
-
-  info "Metrics HTTP server started", serverIp = $serverIp, serverPort = $serverPort
-  ok(server)
-
-proc startMetricsLogging(): AppResult[void] =
-  startMetricsLog()
-  ok()
-
-proc startMetricsServerAndLogging*(app: var App): AppResult[void] =
-  if app.conf.metricsServer:
-    let startMetricsServerRes = startMetricsServer(
-      app.conf.metricsServerAddress,
-      Port(app.conf.metricsServerPort + app.conf.portsShift),
-    )
-    if startMetricsServerRes.isErr():
-      error "Starting metrics server failed. Continuing in current state.",
-        error = startMetricsServerRes.error
-    else:
-      app.metricsServer = some(startMetricsServerRes.value)
-
-  if app.conf.metricsLogging:
-    let startMetricsLoggingRes = startMetricsLogging()
-    if startMetricsLoggingRes.isErr():
-      error "Starting metrics console logging failed. Continuing in current state.",
-        error = startMetricsLoggingRes.error
-
-  ok()
-
 # App shutdown
 
 proc stop*(app: App): Future[void] {.async: (raises: [Exception]).} =
-  if app.conf.rest:
+  if not app.restServer.isNil():
     await app.restServer.stop()
 
-  if app.metricsServer.isSome():
-    await app.metricsServer.get().stop()
+  if not app.metricsServer.isNil():
+    await app.metricsServer.stop()
 
   if app.wakuDiscv5.isSome():
     await app.wakuDiscv5.get().stop()
diff --git a/waku/node/waku_metrics.nim b/waku/node/waku_metrics.nim
index 776eab365..9ee19c9d3 100644
--- a/waku/node/waku_metrics.nim
+++ b/waku/node/waku_metrics.nim
@@ -8,7 +8,8 @@ import
   ../waku_rln_relay/protocol_metrics as rln_metrics,
   ../utils/collector,
   ./peer_manager,
-  ./waku_node
+  ./waku_node,
+  ../factory/external_config
 
 const LogInterval = 10.minutes
 
@@ -54,3 +55,35 @@ proc startMetricsLog*() =
   )
 
   discard setTimer(Moment.fromNow(LogInterval), logMetrics)
+
+proc startMetricsServer(
+    serverIp: IpAddress, serverPort: Port
+): Result[MetricsHttpServerRef, string] =
+  info "Starting metrics HTTP server", serverIp = $serverIp, serverPort = $serverPort
+
+  let server = MetricsHttpServerRef.new($serverIp, serverPort).valueOr:
+    return err("metrics HTTP server start failed: " & $error)
+
+  try:
+    waitFor server.start()
+  except CatchableError:
+    return err("metrics HTTP server start failed: " & getCurrentExceptionMsg())
+
+  info "Metrics HTTP server started", serverIp = $serverIp, serverPort = $serverPort
+  return ok(server)
+
+proc startMetricsServerAndLogging*(
+    conf: WakuNodeConf
+): Result[MetricsHttpServerRef, string] =
+  var metricsServer: MetricsHttpServerRef
+  if conf.metricsServer:
+    metricsServer = startMetricsServer(
+      conf.metricsServerAddress, Port(conf.metricsServerPort + conf.portsShift)
+    ).valueOr:
+      return
+        err("Starting metrics server failed. Continuing in current state:" & $error)
+
+  if conf.metricsLogging:
+    startMetricsLog()
+
+  return ok(metricsServer)