ENT-14329: Store only in-use measurement slots

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
 {

cf-check/dump.c

@@ -162,18 +162,24 @@ 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)
+    // cf-monitord stores only the in-use measurement slots (see
+    // AveragesUsedSize), so a record is normally shorter than the full struct;
+    // a record written before CF_OBSERVABLES was raised is likewise short.
+    // 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
     {
         // 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);
@@ -288,7 +294,15 @@ static void print_struct_or_string(
 {
     if (structs)
     {
-        if (StringContains(file, "cf_lastseen.lmdb")
+        if ((StringContains(file, "cf_observations.lmdb")
+             || StringContains(file, "history.lmdb"))
+            && StringEqual(key.mv_data, "version"))
+        {
+            // Migration version scalar (dbm_migration_observations.c), a short
+            // string rather than an Averages struct.
+            print_json_string(value.mv_data, value.mv_size, strip_strings);
+        }
+        else if (StringContains(file, "cf_lastseen.lmdb")
             && StringStartsWith(key.mv_data, "q"))
         {
             print_struct_lastseen_quality(value, strip_strings);

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

cf-check/observables.h

@@ -4,7 +4,7 @@
 #include <sequence.h>
 
 // copy of libpromises/cf3.defs.h, TODO refactor
-#define CF_OBSERVABLES 100
+#define CF_OBSERVABLES 300
 
 char **GetObservableNames(const char *ts_key_path);
 

cf-monitord/env_monitor.c

@@ -50,6 +50,7 @@
 #include <probes.h>                      /* MonOtherInit,MonOtherGatherData */
 #include <history.h>                     /* HistoryUpdate */
 #include <monitoring.h>                  /* GetObservable */
+#include <monitoring_read.h>             /* AveragesUsedSize */
 #include <cleanup.h>
 
 
@@ -773,7 +774,7 @@ static void UpdateAverages(EvalContext *ctx, char *timekey, const Averages *cons
 
     Log(LOG_LEVEL_INFO, "Updated averages at '%s'", timekey);
 
-    WriteDB(dbp, timekey, newvals, sizeof(Averages));
+    WriteDB(dbp, timekey, newvals, AveragesUsedSize());
     WriteDB(dbp, "DATABASE_AGE", &AGE, sizeof(double));
 
     CloseDB(dbp);

cf-monitord/history.c

@@ -26,6 +26,7 @@
 #include <history.h>
 
 #include <monitoring.h>                                      /* MakeTimekey */
+#include <monitoring_read.h>                                 /* AveragesUsedSize */
 #include <actuator.h>
 #include <promises.h>
 #include <ornaments.h>
@@ -71,7 +72,7 @@ static void PutRecordForTime(CF_DB *db, time_t time, const Averages *values)
 
     MakeTimekey(time, timekey);
 
-    WriteDB(db, timekey, values, sizeof(Averages));
+    WriteDB(db, timekey, values, AveragesUsedSize());
 }
 
 static void Nova_SaveFilePosition(const char *handle, const char *name, long fileptr)

cf-monitord/monitoring.c

@@ -165,8 +165,11 @@ void GetObservable(int i, char *name, size_t name_size, char *desc, size_t desc_
         }
         else
         {
-            strncpy(name, OBSERVABLES[i][0], name_size - 1);
-            strncpy(desc, OBSERVABLES[i][1], desc_size - 1);
+            /* OBSERVABLES only holds the named observables (indices < ob_spare);
+             * a slot at or above ob_spare with no registered measurement is
+             * spare. Do not index OBSERVABLES here. */
+            strncpy(name, "spare", name_size - 1);
+            strncpy(desc, "unused", desc_size - 1);
         }
     }
 }

libpromises/Makefile.am

@@ -84,6 +84,7 @@ libpromises_la_SOURCES = \
 	dbm_api.c dbm_api.h dbm_api_types.h dbm_priv.h \
 	dbm_migration.c dbm_migration.h \
 	dbm_migration_lastseen.c \
+	dbm_migration_observations.c \
 	dbm_lmdb.c \
 	dbm_quick.c \
 	dbm_tokyocab.c \

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 */

libpromises/constants.c

@@ -151,32 +151,6 @@ const char *const OBSERVABLES[CF_OBSERVABLES][2] =
     {"postgres_out", "PostgreSQL database client sessions (out)"},
     {"ipp_in", "Internet Printer Protocol (in)"},
     {"ipp_out", "Internet Printer Protocol (out)"},
-    {"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"},
+    /* Indices at/after ob_spare are custom or unused slots; they are
+       not in this table -- GetObservable() reports them as "spare". */
 };

libpromises/dbm_migration.c

@@ -29,6 +29,7 @@
 #include <backup.h>
 
 extern const DBMigrationFunction dbm_migration_plan_lastseen[];
+extern const DBMigrationFunction dbm_migration_plan_observations[];
 
 
 #ifndef LMDB
@@ -51,7 +52,11 @@ static size_t DBVersion(DBHandle *db)
 }
 
 static const DBMigrationFunction *const dbm_migration_plans[dbid_max] = {
-    [dbid_lastseen] = dbm_migration_plan_lastseen
+    [dbid_lastseen] = dbm_migration_plan_lastseen,
+    /* Both DBs store the same fixed-size Averages records, so they share the
+     * plan that expands them when CF_OBSERVABLES grows. */
+    [dbid_observations] = dbm_migration_plan_observations,
+    [dbid_history] = dbm_migration_plan_observations
 };
 
 bool DBMigrate(DBHandle *db, dbid id)

libpromises/dbm_migration_observations.c

@@ -0,0 +1,113 @@
+/*
+  Copyright 2024 Northern.tech AS
+
+  This file is part of CFEngine 3 - written and maintained by Northern.tech AS.
+
+  This program is free software; you can redistribute it and/or modify it
+  under the terms of the GNU General Public License as published by the
+  Free Software Foundation; version 3.
+
+  This program is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+  GNU General Public License for more details.
+
+  You should have received a copy of the GNU General Public License
+  along with this program; if not, write to the Free Software
+  Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA
+
+  To the extent this program is licensed as part of the Enterprise
+  versions of CFEngine, the applicable Commercial Open Source License
+  (COSL) may apply to this file if you as a licensee so wish it. See
+  included file COSL.txt.
+*/
+
+#include <dbm_migration.h>
+
+#include <dbm_api.h>
+#include <logging.h>
+
+/* Number of measurement slots (CF_OBSERVABLES) before ENT-6511 raised it from
+ * 100 to 300. A record written by an agent from before that change is this many
+ * slots long. */
+#define CF_OBSERVABLES_BEFORE_ENT_6511 100
+
+typedef struct
+{
+    time_t last_seen;
+    QPoint Q[CF_OBSERVABLES_BEFORE_ENT_6511];
+} AveragesBeforeEnt6511;
+
+/*
+ * The observations (cf_observations.lmdb) and history (history.lmdb) databases
+ * store fixed-size Averages records keyed by time. Raising CF_OBSERVABLES grows
+ * that struct, so a record written by an older agent is shorter than the current
+ * struct. cf-monitord's own read path zero-extends short records, and it
+ * overwrites the observations records on its next cycle, but the history records
+ * are never rewritten. Migrate every old-size record to the current size,
+ * zero-filling the added slots, so all records on disk share one layout.
+ */
+static bool MeasurementsMigrationVersion0(DBHandle *db)
+{
+    DBCursor *cursor;
+    if (!NewDBCursor(db, &cursor))
+    {
+        Log(LOG_LEVEL_ERR,
+            "Unable to create database cursor during measurement DB migration");
+        return false;
+    }
+
+    char *key;
+    void *value;
+    int key_size, value_size;
+
+    while (NextDB(cursor, &key, &key_size, &value, &value_size))
+    {
+        /* Only fixed-size Averages records written before CF_OBSERVABLES was
+         * raised need expanding. Scalar bookkeeping keys (e.g. "DATABASE_AGE"
+         * and "version") are a different size and are left untouched. */
+        if (value_size != (int) sizeof(AveragesBeforeEnt6511))
+        {
+            continue;
+        }
+
+        /* Copy the old, shorter record into a full-size, zeroed struct so the
+         * slots added by the larger CF_OBSERVABLES read back as zero. */
+        Averages expanded;
+        memset(&expanded, 0, sizeof(expanded));
+        memcpy(&expanded, value, sizeof(AveragesBeforeEnt6511));
+
+        // This will overwrite the entry
+        if (!DBCursorWriteEntry(cursor, &expanded, sizeof(expanded)))
+        {
+            Log(LOG_LEVEL_ERR,
+                "Unable to expand measurement record for key '%s' during migration",
+                key);
+            DeleteDBCursor(cursor);
+            return false;
+        }
+    }
+
+    if (!DeleteDBCursor(cursor))
+    {
+        Log(LOG_LEVEL_ERR,
+            "Unable to close cursor during measurement DB migration");
+        return false;
+    }
+
+    if (!WriteDB(db, "version", "1", sizeof("1")))
+    {
+        Log(LOG_LEVEL_ERR,
+            "Failed to update version number during measurement DB migration");
+        return false;
+    }
+
+    Log(LOG_LEVEL_INFO, "Migrated measurement database to version 1");
+    return true;
+}
+
+const DBMigrationFunction dbm_migration_plan_observations[] =
+{
+    MeasurementsMigrationVersion0,
+    NULL
+};

libpromises/monitoring_read.c

@@ -25,6 +25,7 @@
 
 #include <monitoring_read.h>
 
+#include <stddef.h>                                       /* offsetof */
 #include <file_lib.h>                                     /* FILE_SEPARATOR */
 #include <known_dirs.h>
 
@@ -225,6 +226,31 @@ bool NovaHasSlot(int idx)
     return idx < ob_spare || SLOTS[idx - ob_spare];
 }
 
+/*
+ * The number of bytes of an Averages record that are actually in use. The
+ * built-in observables (indices 0..ob_spare-1) are always present; the custom
+ * slots above them are used only where a measurement is registered. cf-monitord
+ * writes only this many bytes per timekey, so unused trailing slots (up to 200
+ * once CF_OBSERVABLES is 300) cost nothing on disk. Readers zero their buffer
+ * and DBPrivRead clamps the copy, so a short record reads back with its unused
+ * slots as zero.
+ */
+size_t AveragesUsedSize(void)
+{
+    Nova_LoadSlots();
+
+    int highest = ob_spare - 1;
+    for (int i = CF_OBSERVABLES - 1; i >= ob_spare; i--)
+    {
+        if (SLOTS[i - ob_spare] != NULL)
+        {
+            highest = i;
+            break;
+        }
+    }
+    return offsetof(Averages, Q) + (size_t) (highest + 1) * sizeof(QPoint);
+}
+
 const char *NovaGetSlotName(int idx)
 {
     Nova_LoadSlots();
@@ -324,6 +350,10 @@ bool GetRecordForTime(CF_DB *db, time_t time, Averages *result)
 
     MakeTimekey(time, timekey);
 
+    /* Records may be shorter than sizeof(Averages) (see AveragesUsedSize), so
+     * zero the buffer first: DBPrivRead clamps the copy to the stored size and
+     * the unused trailing slots must read back as zero. */
+    memset(result, 0, sizeof(Averages));
     return ReadDB(db, timekey, result, sizeof(Averages));
 }
 

libpromises/monitoring_read.h

@@ -40,6 +40,7 @@ MonitoringSlot *Nova_MakeSlot(const char *name, const char *description,
                               bool consolidable);
 void Nova_LoadSlots(void);
 bool NovaHasSlot(int idx);
+size_t AveragesUsedSize(void);
 const char *NovaGetSlotName(int idx);
 const char *NovaGetSlotDescription(int index);
 const char *NovaGetSlotUnits(int index);