stats: Fix non-worker stats missing

Commit b8b8aa69b49ac0dd222446c28d00a50f9fd7d716 used tm_name of the
first StatsRecord of a thread block as key for the "threads" object.
However, depending on the type of thread, tm_name can be NULL and would
result in no entry being included for that thread at all. This caused
non-worker metrics to vanish from the "threads" object in the
dump-counters output.

This patch fixes this by remembering the first occurrence of a valid
tm_name within the per-thread block and adds another unittest to
cover this scenario.

diff --git a/src/output-json-stats.c b/src/output-json-stats.c
index 33f98afa2dda91853fb1b59180f056d754eb7437..7cc880727dcea3f433252c33d325aa7c2ca020a0 100644 (file)
--- a/src/output-json-stats.c
+++ b/src/output-json-stats.c
@@ -265,23 +265,30 @@ json_t *StatsToJSON(const StatsTable *st, uint8_t flags)
         uint32_t x;
         for (x = 0; x < st->ntstats; x++) {
             uint32_t offset = x * st->nstats;
-
-            // Stats for for this thread.
-            json_t *thread = json_object();
-            if (unlikely(thread == NULL)) {
-                json_decref(js_stats);
-                json_decref(threads);
-                return NULL;
-            }
+            const char *tm_name = NULL;
+            json_t *thread = NULL;
 
             /* for each counter */
             for (u = offset; u < (offset + st->nstats); u++) {
                 if (st->tstats[u].name == NULL)
                     continue;
 
-                // Seems this holds, but assert in debug builds.
-                DEBUG_VALIDATE_BUG_ON(
-                        strcmp(st->tstats[offset].tm_name, st->tstats[u].tm_name) != 0);
+                DEBUG_VALIDATE_BUG_ON(st->tstats[u].tm_name == NULL);
+
+                if (tm_name == NULL) {
+                    // First time we see a set tm_name. Remember it
+                    // and allocate the stats object for this thread.
+                    tm_name = st->tstats[u].tm_name;
+                    thread = json_object();
+                    if (unlikely(thread == NULL)) {
+                        json_decref(js_stats);
+                        json_decref(threads);
+                        return NULL;
+                    }
+                } else {
+                    DEBUG_VALIDATE_BUG_ON(strcmp(tm_name, st->tstats[u].tm_name) != 0);
+                    DEBUG_VALIDATE_BUG_ON(thread == NULL);
+                }
 
                 json_t *js_type = NULL;
                 const char *stat_name = st->tstats[u].short_name;
@@ -303,7 +310,10 @@ json_t *StatsToJSON(const StatsTable *st, uint8_t flags)
                     }
                 }
             }
-            json_object_set_new(threads, st->tstats[offset].tm_name, thread);
+            if (tm_name != NULL) {
+                DEBUG_VALIDATE_BUG_ON(thread == NULL);
+                json_object_set_new(threads, tm_name, thread);
+            }
         }
         json_object_set_new(js_stats, "threads", threads);
     }

diff --git a/src/tests/output-json-stats.c b/src/tests/output-json-stats.c
index ac1336eff898822ec37c413207975881c45501e6..332a819fee861010c5e2977b954514aad0325fd0 100644 (file)
--- a/src/tests/output-json-stats.c
+++ b/src/tests/output-json-stats.c
@@ -23,7 +23,7 @@
 
 static int OutputJsonStatsTest01(void)
 {
-    StatsRecord global_records[] = { { 0 }, { 0 } };
+    StatsRecord total_records[] = { { 0 }, { 0 } };
     StatsRecord thread_records[2];
     thread_records[0].name = "capture.kernel_packets";
     thread_records[0].short_name = "kernel_packets";
@@ -36,7 +36,7 @@ static int OutputJsonStatsTest01(void)
 
     StatsTable table = {
         .nstats = 2,
-        .stats = &global_records[0],
+        .stats = &total_records[0],
         .ntstats = 1,
         .tstats = &thread_records[0],
     };
@@ -64,7 +64,72 @@ static int OutputJsonStatsTest01(void)
     return cmp_result == 0;
 }
 
+static int OutputJsonStatsTest02(void)
+{
+    StatsRecord total_records[4] = { 0 };
+    StatsRecord thread_records[8] = { 0 };
+
+    // Totals
+    total_records[0].name = "tcp.syn";
+    total_records[0].short_name = "syn";
+    total_records[0].tm_name = NULL;
+    total_records[0].value = 1234;
+
+    // Worker
+    // thread_records[0] is a global counter
+    thread_records[1].name = "capture.kernel_packets";
+    thread_records[1].short_name = "kernel_packets";
+    thread_records[1].tm_name = "W#01-bond0.30";
+    thread_records[1].value = 42;
+    thread_records[2].name = "capture.kernel_drops";
+    thread_records[2].short_name = "kernel_drops";
+    thread_records[2].tm_name = "W#01-bond0.30";
+    thread_records[2].value = 4711;
+    // thread_records[3] is a FM specific counter
+
+    // Flow manager
+    // thread_records[4] is a global counter
+    // thread_records[5] is a worker specific counter
+    // thread_records[6] is a worker specific counter
+    thread_records[7].name = "flow.mgr.full_hash_passes";
+    thread_records[7].short_name = "full_hash_passes";
+    thread_records[7].tm_name = "FM#01";
+    thread_records[7].value = 10;
+
+    StatsTable table = {
+        .nstats = 4,
+        .stats = &total_records[0],
+        .ntstats = 2,
+        .tstats = &thread_records[0],
+    };
+
+    json_t *r = StatsToJSON(&table, JSON_STATS_TOTALS | JSON_STATS_THREADS);
+    if (!r)
+        return 0;
+
+    // Remove variable content
+    json_object_del(r, "uptime");
+
+    char *serialized = json_dumps(r, 0);
+
+    // Cheesy comparison
+    const char *expected = "{\"tcp\": {\"syn\": 1234}, \"threads\": {\"W#01-bond0.30\": "
+                           "{\"capture\": {\"kernel_packets\": "
+                           "42, \"kernel_drops\": 4711}}, \"FM#01\": {\"flow\": {\"mgr\": "
+                           "{\"full_hash_passes\": 10}}}}}";
+
+    int cmp_result = strcmp(expected, serialized);
+    if (cmp_result != 0)
+        printf("unexpected result\nexpected=%s\ngot=%s\n", expected, serialized);
+
+    free(serialized);
+    json_decref(r);
+
+    return cmp_result == 0;
+}
+
 void OutputJsonStatsRegisterTests(void)
 {
     UtRegisterTest("OutputJsonStatsTest01", OutputJsonStatsTest01);
+    UtRegisterTest("OutputJsonStatsTest02", OutputJsonStatsTest02);
 }