Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

7.0.x backport: Fix stats missing for non workers #10742

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions etc/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@
"icmp_type": {
"type": "integer"
},
"in_iface": {
"type": "string"
},
"log_level": {
"type": "string"
},
Expand Down Expand Up @@ -3697,6 +3700,20 @@
"uptime": {
"type": "integer"
},
"capture": {
"type": "object",
"properties": {
"kernel_packets": {
"type": "integer"
},
"kernel_drops": {
"type": "integer"
},
"kernel_ifdrops": {
"type": "integer"
}
}
},
"memcap_pressure": {
"type": "integer"
},
Expand Down
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 } };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason for this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing for convention with the terms/names used in OutputJsonStatsTest02

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);
}
Loading