Skip to content

Commit

Permalink
stats: Fix non-worker stats missing
Browse files Browse the repository at this point in the history
Commit b8b8aa6 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.
  • Loading branch information
awelzel authored and victorjulien committed Mar 7, 2024
1 parent 1d3a156 commit f172041
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 14 deletions.
34 changes: 22 additions & 12 deletions src/output-json-stats.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down
69 changes: 67 additions & 2 deletions src/tests/output-json-stats.c
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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],
};
Expand Down Expand Up @@ -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);
}

0 comments on commit f172041

Please sign in to comment.