diff --git a/library/logosmetrics.nim b/library/logosmetrics.nim index a5082abe..cd830c94 100644 --- a/library/logosmetrics.nim +++ b/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 diff --git a/library/storage_thread_requests/requests/node_info_request.nim b/library/storage_thread_requests/requests/node_info_request.nim index 1e2caed4..fea50bbb 100644 --- a/library/storage_thread_requests/requests/node_info_request.nim +++ b/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"])) diff --git a/tests/cbindings/storage.c b/tests/cbindings/storage.c index 5818b830..c27ed4ff 100644 --- a/tests/cbindings/storage.c +++ b/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); diff --git a/tests/libstorage/logosmetrics.nim b/tests/libstorage/logosmetrics.nim index a3e85bc4..e04f2c83 100644 --- a/tests/libstorage/logosmetrics.nim +++ b/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"}, + }, + ] + }