feat: add logos_storage_ prefix to metrics collected over libstorage (#1504)

library/logosmetrics.nim

@@ -17,7 +17,30 @@ proc jsonType(collector: Collector): string =
 
   return "unknown"
 
-proc toJson(collector: Collector, metrics: var seq[JsonNode]) =
+const RedundantFragments = ["logos", "storage", "_"]
+
+func addMetricsPrefix*(name: string): string =
+  ## Adds the standard Logos metrics prefix (`logos_storage_`) to a metric name.
+  ## Drops repeat prefix fragments (`logos`, `storage`) before adding the prefix.
+  var slice_idx: int
+  while true:
+    var hasFrag = false
+    for fragment in RedundantFragments:
+      if name.continuesWith(fragment, slice_idx):
+        slice_idx += fragment.len
+        hasFrag = true
+        break
+    if not hasFrag:
+      break
+
+  return
+    if slice_idx == name.len:
+      name
+    else:
+      # This is the only part where we should be doing copies.
+      "logos_storage_" & name[slice_idx .. ^1]
+
+proc toJson(collector: Collector, metrics: var seq[JsonNode], prefix: bool = false) =
   # We know the closure won't outlive `metrics` so this is
   # an acceptable hack.
   let metricsPtr = addr metrics
@@ -40,18 +63,34 @@ proc toJson(collector: Collector, metrics: var seq[JsonNode]) =
       labelMap[labels[i]] = %labelValues[i]
 
     metricsPtr[].add(
-      %*{"name": name, "type": typ, "help": help, "value": value, "labels": labelMap}
+      %*{
+        "name":
+          if prefix:
+            addMetricsPrefix(name)
+          else:
+            name,
+        "type": typ,
+        "help": help,
+        "value": value,
+        "labels": labelMap,
+      }
     )
 
   collector.collect(serializeMetric)
 
-# Serializes all collectors in a given registry to a Logos openmetrics-compatible
-# format. Allows including only specific collectors by name.
 proc toJson*(
     registry: Registry,
     exclude: openArray[string] = [],
     includeOnly: openArray[string] = [],
+    prefix: bool = false,
 ): JsonNode =
+  ## Serializes all collectors in a given registry to a Logos openmetrics-compatible
+  ## format. Allows including only specific collectors by name. Optionally, adds a
+  ## standardized prefix to all metrics on output.
+  ##
+  ## See also:
+  ##   - `addMetricsPrefix`
+  ##
   var metrics = newSeq[JsonNode]()
   withLock registry.lock:
     for collector in registry.collectors:
@@ -61,6 +100,6 @@ proc toJson*(
       if includeOnly.len > 0:
         if collector.name notin includeOnly:
           continue
-      collector.toJson(metrics)
+      collector.toJson(metrics, prefix)
 
   result = %*{"metrics": metrics}

library/storage_thread_requests/requests/node_info_request.nim

@@ -98,4 +98,4 @@ proc process*(
       # an infinite number of objects (could be related to some assumption
       # failure above).
       # FIXME figure out what's going on and add this back.
-      return ok($defaultRegistry.toJson(exclude = @["nim_runtime_info"]))
+      return ok($defaultRegistry.toJson(exclude = @["nim_runtime_info"], prefix = true))

tests/cbindings/storage.c

@@ -916,7 +916,7 @@ int check_get_metrics(void *storage_ctx)
     }
 
     // Checks that response contains a metric we are SURE must exist
-    if (res == NULL || strstr(res, "libp2p_successful_dials_total") == NULL)
+    if (res == NULL || strstr(res, "logos_storage_libp2p_successful_dials_total") == NULL)
     {
         fprintf(stderr, "get_metrics missing expected metric\n");
         free(res);

tests/libstorage/logosmetrics.nim

@@ -1,5 +1,7 @@
 import std/json
 import std/times
+import std/sequtils
+import std/sets
 
 import pkg/unittest2
 import pkg/metrics
@@ -31,6 +33,21 @@ method collect(collector: BadCollector, output: MetricHandler) =
   )
 
 suite "Metrics":
+  test "should add prefix to unprefixed metrics":
+    check "logos_storage_count" == addMetricsPrefix("count")
+
+  test "should drop leading underscores":
+    check "logos_storage_count" == addMetricsPrefix("_count")
+
+  test "should drop repeated prefixes":
+    check "logos_storage_count" == addMetricsPrefix("logos_storage_count")
+    check "logos_storage_count" == addMetricsPrefix("storage_count")
+    check "logos_storage_count" == addMetricsPrefix("logos_count")
+
+  test "should not modify name if it is made of prefix fragments alone":
+    check "logos_storage_" == addMetricsPrefix("logos_storage_")
+    check "storage" == addMetricsPrefix("storage")
+
   test "should serialize Nim metrics to Logos Metrics format":
     myCounter.inc(labelValues = ["screws"])
     myCounter.inc(labelValues = ["washers"])
@@ -146,3 +163,18 @@ suite "Metrics":
           },
         ]
       }
+
+  test "should prefix metrics when requested":
+    let
+      metrics = defaultRegistry.toJson(
+        includeOnly = @["myCounter", "myGauge", "myHistogram"], prefix = true
+      )
+      names = metrics["metrics"].mapIt(it["name"].getStr()).toHashSet()
+
+    check names ==
+      [
+        "logos_storage_myCounter_total", "logos_storage_myCounter_created",
+        "logos_storage_myGauge", "logos_storage_myHistogram_sum",
+        "logos_storage_myHistogram_count", "logos_storage_myHistogram_bucket",
+        "logos_storage_myHistogram_created",
+      ].toHashSet()