drop nim_runtime_info collector, handle badly behaved collectors

library/logosmetrics.nim

@@ -17,7 +17,9 @@ proc toJson(collector: Collector, metrics: var seq[JsonNode] = @[]) =
     # The logos openmetrics format (https://github.com/logos-co/openmetrics-module)
     # does not include the timestamp, so we don't include it either.
     var labelMap = newJObject()
-    for i in 0 ..< labels.len:
+    # When a label is missing, it's simply not included in the values, so we take
+    # the minimum.
+    for i in 0 ..< min(labelValues.len, labels.len):
       labelMap[labels[i]] = %labelValues[i]
 
     metricsPtr[].add(%*{"name": name, "value": value, "labels": labelMap})
@@ -26,10 +28,17 @@ proc toJson(collector: Collector, metrics: var seq[JsonNode] = @[]) =
 
 # Serializes all collectors in a given registry to a Logos openmetrics-compatible
 # format. Allows including only specific collectors by name.
-proc toJson*(registry: Registry, includeOnly: openArray[string] = []): JsonNode =
+proc toJson*(
+    registry: Registry,
+    exclude: openArray[string] = [],
+    includeOnly: openArray[string] = [],
+): JsonNode =
   var metrics = newSeq[JsonNode]()
   withLock registry.lock:
     for collector in registry.collectors:
+      if exclude.len > 0:
+        if collector.name in exclude:
+          continue
       if includeOnly.len > 0:
         if collector.name notin includeOnly:
           continue

library/storage_thread_requests/requests/node_info_request.nim

@@ -80,4 +80,7 @@ proc process*(
     return res
   of METRICS:
     {.cast(gcsafe).}:
-      return ok($defaultRegistry.toJson())
+      # Excludes nim_runtime_info as dumpHeapInstances seems to be returning
+      # an infinite number of objects.
+      # FIXME figure out what's going on and add this back.
+      return ok($defaultRegistry.toJson(exclude = @["nim_runtime_info"]))

tests/cbindings/storage.c

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

tests/libstorage/logosmetrics.nim

@@ -1,4 +1,5 @@
 import std/json
+import std/times
 
 import pkg/unittest2
 import pkg/metrics
@@ -9,6 +10,26 @@ declareCounter myCounter, "Parts Counter", ["part_type"]
 declareGauge myGauge, "My gauge"
 declareHistogram myHistogram, "My histogram", buckets = [0.0, 1.0, 2.0]
 
+# A badly-behaved collector that emits label/labelValue arrays of differing
+# lengths.
+type BadCollector = ref object of Gauge
+
+method collect(collector: BadCollector, output: MetricHandler) =
+  output(
+    name = "bad_metric",
+    value = 1.0,
+    labels = ["label1", "label2"],
+    labelValues = ["value1"],
+    timestamp = Time(),
+  )
+  output(
+    name = "bad_metric",
+    value = 1.0,
+    labels = ["label1", "label2"],
+    labelValues = ["value1", "value2"],
+    timestamp = Time(),
+  )
+
 suite "Metrics":
   test "should serialize Nim metrics to Logos Metrics format":
     myCounter.inc(labelValues = ["screws"])
@@ -22,7 +43,8 @@ suite "Metrics":
     myHistogram.observe(4)
     myHistogram.observe(5)
 
-    let metrics = defaultRegistry.toJson(@["myCounter", "myGauge", "myHistogram"])
+    let metrics =
+      defaultRegistry.toJson(includeOnly = @["myCounter", "myGauge", "myHistogram"])
 
     # Remove "created" metrics as we can't pin those down.
     var filteredMetrics = %*{"metrics": @[]}
@@ -45,3 +67,20 @@ suite "Metrics":
           {"name": "myHistogram_bucket", "value": 5.0, "labels": {"le": "+Inf"}},
         ]
       }
+
+  test "should drop labels when collector emits fewer values than labels":
+    discard BadCollector.newCollector("badMetric", "Badly behaved collector")
+
+    let metrics = defaultRegistry.toJson(includeOnly = @["badMetric"])
+
+    check metrics ==
+      %*{
+        "metrics": [
+          {"name": "bad_metric", "value": 1.0, "labels": {"label1": "value1"}},
+          {
+            "name": "bad_metric",
+            "value": 1.0,
+            "labels": {"label1": "value1", "label2": "value2"},
+          },
+        ]
+      }