From 50d4f8a7edf2af9848bdc72f95d56a339c1ec01d Mon Sep 17 00:00:00 2001 From: Nick Anderson Date: Mon, 20 Jul 2026 04:53:51 -0500 Subject: [PATCH] ENT-6511: Increase cf-monitord measurement slots from 100 to 300 Out of the box ~80 of 100 measurement slots are used by default measurements, leaving little room for custom measurements. This change increases the maximum number of slots to 300. Changes are synchronized across: - libpromises/cf3.defs.h (main constant) - cf-check/observables.h (cf-check copy) - cf-check/db_structs.h (LMDB struct layout) - libpromises/constants.c (OBSERVABLES name table) - tests/unit/mon_*_test.c (unit test buffers) The OBSERVABLES name table must have an entry per slot: cf-monitord's GetObservable() falls back to OBSERVABLES[i] for any slot without a registered measurement, so leaving the table at 100 entries while raising CF_OBSERVABLES makes cf-monitord read past its end (NULL) and crash on the first sampling pass, whether or not any custom measurements are configured. The table is extended to 300 entries to match. cf-check dump must tolerate monitoring state written before the upgrade: a pre-upgrade record is shorter than the enlarged Averages struct, and the pre-upgrade ts_key has fewer than CF_OBSERVABLES entries. dump.c now accepts a record up to sizeof(Averages) (copied into a zeroed struct) and GetObservableNames() fills any missing names with a spare[] fallback instead of leaving the array partly uninitialized, so `cf-check dump` does not crash on pre-upgrade state in the brief window before cf-monitord rewrites it. Ticket: ENT-6511 Changelog: Increased the maximum number of cf-monitord measurement slots (CF_OBSERVABLES) from 100 to 300 --- cf-check/db_structs.h | 2 +- cf-check/dump.c | 13 +- cf-check/observables.c | 21 +++- cf-check/observables.h | 2 +- libpromises/cf3.defs.h | 2 +- libpromises/constants.c | 204 ++++++++++++++++++++++++++++++++ tests/unit/mon_cpu_test.c | 2 +- tests/unit/mon_load_test.c | 2 +- tests/unit/mon_processes_test.c | 2 +- 9 files changed, 236 insertions(+), 14 deletions(-) diff --git a/cf-check/db_structs.h b/cf-check/db_structs.h index 17ba14e73b..8a92eed4c7 100644 --- a/cf-check/db_structs.h +++ b/cf-check/db_structs.h @@ -125,7 +125,7 @@ static const char *const observable_strings[] = }; // Not the actual count, just the room we set aside in struct (and LMDB): -#define CF_OBSERVABLES 100 +#define CF_OBSERVABLES 300 typedef struct Averages { diff --git a/cf-check/dump.c b/cf-check/dump.c index 6ded3c8bbd..e8af8eb8a3 100644 --- a/cf-check/dump.c +++ b/cf-check/dump.c @@ -162,10 +162,14 @@ static void print_struct_lock_data( static void print_struct_averages( const MDB_val value, const bool strip_strings, const char *tskey_filename) { - assert(sizeof(Averages) == value.mv_size); - if (sizeof(Averages) != value.mv_size) + // A record written before an increase of CF_OBSERVABLES is shorter than the + // current struct (e.g. after a package upgrade, until cf-monitord rewrites + // it). Accept any size up to the full struct and copy into a zeroed struct + // so the un-stored trailing slots read back as zero. + assert(value.mv_size <= sizeof(Averages)); + if (value.mv_size > sizeof(Averages)) { - // Fall back to simple printing in release builds: + // Unexpected: larger than the struct. Fall back to simple printing. print_json_string(value.mv_data, value.mv_size, strip_strings); } else @@ -173,7 +177,8 @@ static void print_struct_averages( // TODO: clean up Averages char **obnames = NULL; Averages averages; - memcpy(&averages, value.mv_data, sizeof(averages)); + memset(&averages, 0, sizeof(averages)); + memcpy(&averages, value.mv_data, value.mv_size); const time_t last_seen = averages.last_seen; obnames = GetObservableNames(tskey_filename); diff --git a/cf-check/observables.c b/cf-check/observables.c index 857c6a192d..2619ad1dce 100644 --- a/cf-check/observables.c +++ b/cf-check/observables.c @@ -67,10 +67,23 @@ char **GetObservableNames(const char *ts_key_path) if (fgets(line, CF_MAXVARSIZE, f) == NULL) { - Log(LOG_LEVEL_ERR, - "Error trying to read ts_key from file '%s'. (fgets: %s)", - filename, - GetErrorStr()); + if (ferror(f)) + { + Log(LOG_LEVEL_ERR, + "Error trying to read ts_key from file '%s'. (fgets: %s)", + filename, + GetErrorStr()); + } + /* The ts_key has fewer entries than CF_OBSERVABLES -- e.g. it + * was written by an older agent with a smaller CF_OBSERVABLES. + * Fill the remaining names so the returned array keeps its + * documented non-NULL guarantee (callers index all + * CF_OBSERVABLES entries). */ + for (int j = i; j < CF_OBSERVABLES; ++j) + { + snprintf(buf, CF_MAXVARSIZE, "spare[%d]", j); + temp[j] = xstrdup(buf); + } break; } diff --git a/cf-check/observables.h b/cf-check/observables.h index 1c8d932b96..dd33be3d56 100644 --- a/cf-check/observables.h +++ b/cf-check/observables.h @@ -4,7 +4,7 @@ #include // copy of libpromises/cf3.defs.h, TODO refactor -#define CF_OBSERVABLES 100 +#define CF_OBSERVABLES 300 char **GetObservableNames(const char *ts_key_path); diff --git a/libpromises/cf3.defs.h b/libpromises/cf3.defs.h index fd53bf8a68..c2e060c055 100644 --- a/libpromises/cf3.defs.h +++ b/libpromises/cf3.defs.h @@ -148,7 +148,7 @@ typedef enum #define CF_MEASURE_INTERVAL (5.0*60.0) #define CF_SHIFT_INTERVAL (6*3600) -#define CF_OBSERVABLES 100 +#define CF_OBSERVABLES 300 /* Special exit codes */ #define EC_EVAL_ABORTED 6 /* like SIGABRT, but signal exit codes are 120+SIG */ diff --git a/libpromises/constants.c b/libpromises/constants.c index 7562b5aa64..8ade1ced25 100644 --- a/libpromises/constants.c +++ b/libpromises/constants.c @@ -179,4 +179,208 @@ const char *const OBSERVABLES[CF_OBSERVABLES][2] = {"spare", "unused"}, {"spare", "unused"}, {"spare", "unused"}, + /* ENT-6511: slots 100-299, added when CF_OBSERVABLES was raised + 100->300. Every slot must have a name entry: cf-monitord's + GetObservable() falls back to OBSERVABLES[i] for unregistered + slots, so a table shorter than CF_OBSERVABLES => NULL deref. */ + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, + {"spare", "unused"}, }; diff --git a/tests/unit/mon_cpu_test.c b/tests/unit/mon_cpu_test.c index d8eafadd74..3399fb08c8 100644 --- a/tests/unit/mon_cpu_test.c +++ b/tests/unit/mon_cpu_test.c @@ -52,7 +52,7 @@ static double GetCpuStat() void test_cpu_monitor(void) { - double cf_this[100]; + double cf_this[300]; double dq1 = GetCpuStat(); if (dq1 == -1.0) { diff --git a/tests/unit/mon_load_test.c b/tests/unit/mon_load_test.c index c25789f9ac..de143e0690 100644 --- a/tests/unit/mon_load_test.c +++ b/tests/unit/mon_load_test.c @@ -6,7 +6,7 @@ void test_load_monitor(void) { - double cf_this[100]; + double cf_this[300]; MonLoadGatherData(cf_this); double load1[2] = {0,0}; double load2[2] = {0,0}; diff --git a/tests/unit/mon_processes_test.c b/tests/unit/mon_processes_test.c index 50b59fc620..fff48dd224 100644 --- a/tests/unit/mon_processes_test.c +++ b/tests/unit/mon_processes_test.c @@ -100,7 +100,7 @@ static bool GetSysUsers( int *userListSz, int *numRootProcs, int *numOtherProcs) void test_processes_monitor(void) { - double cf_this[100] = { 0.0 }; + double cf_this[300] = { 0.0 }; MonProcessesGatherData(cf_this); MonProcessesGatherData(cf_this); MonProcessesGatherData(cf_this);