[PATCH] initial pass at fixing zbxpcp.so for Zabbix v3

[Nathan Scott <nathans@xxxxxxxxxx>]

Hi Marko,

This should fix the problematic interactions between zbxpcp and the
new version of Zabbix with the ABI/API break.  I noticed the problem
structure (AGENT_RESULT) can actually be determined at runtime, so
we can (at least in theory) solve this via a single .so as we first
hoped.  We don't ever use zbx_log_t (which also changed), and it is
only ever referenced via pointer, so I made that opaque and dropped
its definition entirely.

This switches between using v2/v3 interface based on presence or lack
of a named file.  This is a bit suboptimal as it still involves some
manual user interaction, but its better than having multiple shared
libraries I think.  If we could find some reliable way of detecting a
Zabbix 3 vs 2 install (?) at runtime, we'd be laughing.  Something in
the environment, or a file we could test for?  A symbol we could look
for with dlsym, even?  Anyway, this will probably do for now.

I don't know whether we need to set the new AGENT_RESULT fields, i.e.
lastlogsize and mtime, in the zbxpcp.so v3 callback code - any idea?

cheers.


diff --git a/src/zabbix-agent/src/module.h b/src/zabbix-agent/src/module.h
index aaccc61..91c32d1 100644
--- a/src/zabbix-agent/src/module.h
+++ b/src/zabbix-agent/src/module.h
@@ -1,5 +1,5 @@
 /*
-** Copyright (C) 2001-2015 Zabbix SIA
+** Copyright (C) 2001-2016 Zabbix SIA
 **
 ** 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
@@ -22,6 +22,8 @@ typedef __uint64_t zbx_uint64_t;
 
 #define ZBX_MODULE_API_VERSION_ONE     1
 
+#define get_rkey(request)              (request)->key
+#define get_rparams_num(request)       (request)->nparam
 #define get_rparam(request, num)       ((request)->nparam > num ? 
(request)->params[num] : NULL)
 
 /* flags for command */
@@ -49,17 +51,17 @@ typedef struct
 }
 AGENT_REQUEST;
 
-typedef struct
-{
-       char            *value;
-       char            *source;
-       zbx_uint64_t    lastlogsize;
-       int             timestamp;
-       int             severity;
-       int             logeventid;
-       int             mtime;
-}
-zbx_log_t;
+struct zbx_log;
+typedef struct zbx_log zbx_log_t;
+
+/* agent result types */
+#define AR_UINT64      0x01
+#define AR_DOUBLE      0x02
+#define AR_STRING      0x04
+#define AR_TEXT                0x08
+#define AR_LOG         0x10
+#define AR_MESSAGE     0x20
+#define AR_META                0x40
 
 /* agent return structure */
 typedef struct
@@ -74,15 +76,21 @@ typedef struct
        /* null-terminated list of pointers */
        zbx_log_t       **logs;
 }
-AGENT_RESULT;
+AGENT_RESULT_V2;
 
-/* agent result types */
-#define AR_UINT64      0x01
-#define AR_DOUBLE      0x02
-#define AR_STRING      0x04
-#define AR_TEXT                0x08
-#define AR_LOG         0x10
-#define AR_MESSAGE     0x20
+typedef struct
+{
+       zbx_uint64_t    lastlogsize;    /* meta information */
+       zbx_uint64_t    ui64;
+       double          dbl;
+       char            *str;
+       char            *text;
+       char            *msg;           /* possible error message */
+       zbx_log_t       *log;
+       int             type;           /* flags: see AR_* above */
+       int             mtime;          /* meta information */
+}
+AGENT_RESULT_V3;
 
 /* SET RESULT */
 
@@ -113,13 +121,6 @@ AGENT_RESULT;
 )
 
 /* NOTE: always allocate new memory for val! DON'T USE STATIC OR STACK 
MEMORY!!! */
-#define SET_LOG_RESULT(res, val)               \
-(                                              \
-       (res)->type |= AR_LOG,                  \
-       (res)->logs = (zbx_log_t **)(val)       \
-)
-
-/* NOTE: always allocate new memory for val! DON'T USE STATIC OR STACK 
MEMORY!!! */
 #define SET_MSG_RESULT(res, val)               \
 (                                              \
        (res)->type |= AR_MESSAGE,              \
diff --git a/src/zabbix-agent/src/zbxpcp.c b/src/zabbix-agent/src/zbxpcp.c
index 5e20d4f..700630f 100644
--- a/src/zabbix-agent/src/zbxpcp.c
+++ b/src/zabbix-agent/src/zbxpcp.c
@@ -39,6 +39,10 @@
 #define ZBX_PCP_DERIVED_CONFIG "/etc/zabbix/zbxpcp-derived-metrics.conf"
 #endif
 
+#ifndef ZBX_PCP_CONFIG_VERSION3
+#define ZBX_PCP_CONFIG_VERSION3 "/etc/zabbix/zbxpcp.conf.d/version3"
+#endif
+
 /* PCP includes.  */
 #include "pmapi.h"
 #include "impl.h"
@@ -46,12 +50,14 @@
 /* Zabbix includes.  */
 #include "module.h"
 
+static int zbx_version = 2;    /* back-compat */
+
 /*
  * PCP connection
  */
 static int ctx = -1;
 
-int zbx_module_pcp_connect()
+static int zbx_module_pcp_connect()
 {
     /* Load possible derived metric definitions.  */
     if (access(ZBX_PCP_DERIVED_CONFIG, F_OK ) != -1)
@@ -61,7 +67,7 @@ int zbx_module_pcp_connect()
     return ctx;
 }
 
-int zbx_module_pcp_disconnect()
+static int zbx_module_pcp_disconnect()
 {
     return pmDestroyContext(ctx);
 }
@@ -73,6 +79,8 @@ int zbx_module_init()
 {
     if (zbx_module_pcp_connect() < 0)
         return ZBX_MODULE_FAIL;
+    if (access(ZBX_PCP_CONFIG_VERSION3, F_OK) != -1)
+        zbx_version = 3;
     return ZBX_MODULE_OK;
 }
 
@@ -83,7 +91,7 @@ int zbx_module_api_version()
 
 static int metric_count = 0;
 static ZBX_METRIC *metrics = NULL;
-void zbx_module_pcp_add_metric(const char *name);
+static void zbx_module_pcp_add_metric(const char *name);
 
 ZBX_METRIC *zbx_module_item_list()
 {
@@ -119,9 +127,10 @@ int zbx_module_uninit()
 /*
  * Zabbix/PCP connection
  */
-int zbx_module_pcp_fetch_metric(AGENT_REQUEST *request, AGENT_RESULT *result);
+static int zbx_module2_pcp_fetch_metric(AGENT_REQUEST *, AGENT_RESULT_V2 *);
+static int zbx_module3_pcp_fetch_metric(AGENT_REQUEST *, AGENT_RESULT_V3 *);
 
-void zbx_module_pcp_add_metric(const char *name)
+static void zbx_module_pcp_add_metric(const char *name)
 {
     int sts;
     pmID pmid[1];
@@ -162,12 +171,15 @@ void zbx_module_pcp_add_metric(const char *name)
     if (metrics == NULL) { metrics = mptr; free(metric); free(param); return; }
     metrics[metric_count].key = metric;
     metrics[metric_count].flags = flags;
-    metrics[metric_count].function = zbx_module_pcp_fetch_metric;
+    if (zbx_version >= 3)
+       metrics[metric_count].function = zbx_module3_pcp_fetch_metric;
+    else
+       metrics[metric_count].function = zbx_module2_pcp_fetch_metric;
     metrics[metric_count].test_param = param;
     metric_count++;
 }
 
-int zbx_module_pcp_fetch_metric(AGENT_REQUEST *request, AGENT_RESULT *result)
+static int zbx_module_pcp_fetch_metric(AGENT_REQUEST *request, int *type, 
pmAtomValue *atom, char **errmsg)
 {
     int sts;
     char *metric[] = { request->key + strlen(ZBX_PCP_METRIC_PREFIX) };
@@ -175,7 +187,6 @@ int zbx_module_pcp_fetch_metric(AGENT_REQUEST *request, 
AGENT_RESULT *result)
     pmID pmid[1];
     pmDesc desc[1];
     pmResult *rp;
-    pmAtomValue atom;
     int iid = 0;
     int i;
 
@@ -188,9 +199,8 @@ int zbx_module_pcp_fetch_metric(AGENT_REQUEST *request, 
AGENT_RESULT *result)
             inst = get_rparam(request, 0);
             break;
         default:
-            SET_MSG_RESULT(result, strdup("Extraneous instance 
specification."));
+            *errmsg = "Extraneous instance specification.";
             return SYSINFO_RET_FAIL;
-            break;
     }
 
     /* Try to reconnect if the initial lookup fails.  */
@@ -198,7 +208,7 @@ int zbx_module_pcp_fetch_metric(AGENT_REQUEST *request, 
AGENT_RESULT *result)
     if (sts < 0 && (sts == PM_ERR_IPC || sts == -ECONNRESET)) {
         ctx = pmReconnectContext(ctx);
         if (ctx < 0) {
-            SET_MSG_RESULT(result, strdup("Not connected to pmcd."));
+            *errmsg = "Not connected to pmcd.";
             return SYSINFO_RET_FAIL;
         }
         sts = pmLookupName(1, metric, pmid);
@@ -209,18 +219,18 @@ int zbx_module_pcp_fetch_metric(AGENT_REQUEST *request, 
AGENT_RESULT *result)
     sts = pmLookupDesc(pmid[0], desc);
     if (sts < 0) return SYSINFO_RET_FAIL;
     if (inst != NULL && desc[0].indom == PM_INDOM_NULL) {
-        SET_MSG_RESULT(result, strdup("Extraneous instance specification."));
+        *errmsg = "Extraneous instance specification.";
         return SYSINFO_RET_FAIL;
     }
     if ((inst == NULL && desc[0].indom != PM_INDOM_NULL) ||
         (request->nparam == 1 && !strlen(inst))) {
-        SET_MSG_RESULT(result, strdup("Missing instance specification."));
+        *errmsg = "Missing instance specification.";
         return SYSINFO_RET_FAIL;
     }
     if (desc[0].indom != PM_INDOM_NULL) {
         iid = pmLookupInDom(desc[0].indom, inst);
         if (iid < 0) {
-            SET_MSG_RESULT(result, strdup("Instance not available."));
+            *errmsg = "Instance not available.";
             return SYSINFO_RET_FAIL;
         }
     }
@@ -230,7 +240,7 @@ int zbx_module_pcp_fetch_metric(AGENT_REQUEST *request, 
AGENT_RESULT *result)
     if (sts < 0) return SYSINFO_RET_FAIL;
     if (rp->vset[0]->numval < 1) {
         pmFreeResult(rp);
-        SET_MSG_RESULT(result, strdup("No value available."));
+        *errmsg = "No value available.";
         return SYSINFO_RET_FAIL;
     }
 
@@ -245,12 +255,30 @@ int zbx_module_pcp_fetch_metric(AGENT_REQUEST *request, 
AGENT_RESULT *result)
 
     /* Extract the wanted value.  */
     sts = pmExtractValue(rp->vset[0]->valfmt, &rp->vset[0]->vlist[i],
-                         desc[0].type, &atom, desc[0].type);
+                         desc[0].type, atom, desc[0].type);
     pmFreeResult(rp);
     if (sts < 0) return SYSINFO_RET_FAIL;
+    *type = desc[0].type;
+
+    /* Success.  */
+    return SYSINFO_RET_OK;
+}
+
+static int zbx_module3_pcp_fetch_metric(AGENT_REQUEST *request, 
AGENT_RESULT_V3 *result)
+{
+    char *errmsg = NULL;
+    pmAtomValue atom;
+    int type;
+    int sts;
 
-    /* Hand it to the caller.  */
-    switch(desc[0].type) {
+    /* note: SET_*_RESULT macros evaluate to different code for v2/v3 */
+    sts = zbx_module_pcp_fetch_metric(request, &type, &atom, &errmsg);
+    if (sts < 0) {
+        if (errmsg)
+            SET_MSG_RESULT(result, strdup(errmsg));
+        return sts;
+    }
+    switch (type) {
         case PM_TYPE_32:
             SET_UI64_RESULT(result, atom.l);
             break;
@@ -274,10 +302,53 @@ int zbx_module_pcp_fetch_metric(AGENT_REQUEST *request, 
AGENT_RESULT *result)
             break;
         default:
             SET_MSG_RESULT(result, strdup("Unsupported metric value type."));
-            return SYSINFO_RET_FAIL;
+            sts = SYSINFO_RET_FAIL;
             break;
     }
 
-    /* Success.  */
-    return SYSINFO_RET_OK;
+    return sts;
+}
+
+static int zbx_module2_pcp_fetch_metric(AGENT_REQUEST *request, 
AGENT_RESULT_V2 *result)
+{
+    char *errmsg = NULL;
+    pmAtomValue atom;
+    int type;
+    int sts;
+
+    /* note: SET_*_RESULT macros evaluate to different code for v2/v3 */
+    sts = zbx_module_pcp_fetch_metric(request, &type, &atom, &errmsg);
+    if (sts < 0) {
+        if (errmsg)
+            SET_MSG_RESULT(result, strdup(errmsg));
+        return sts;
+    }
+    switch (type) {
+        case PM_TYPE_32:
+            SET_UI64_RESULT(result, atom.l);
+            break;
+        case PM_TYPE_U32:
+            SET_UI64_RESULT(result, atom.ul);
+            break;
+        case PM_TYPE_64:
+            SET_UI64_RESULT(result, atom.ll);
+            break;
+        case PM_TYPE_U64:
+            SET_UI64_RESULT(result, atom.ull);
+            break;
+        case PM_TYPE_FLOAT:
+            SET_DBL_RESULT(result, atom.f);
+            break;
+        case PM_TYPE_DOUBLE:
+            SET_DBL_RESULT(result, atom.d);
+            break;
+        case PM_TYPE_STRING:
+            SET_STR_RESULT(result, strdup(atom.cp));
+            break;
+        default:
+            SET_MSG_RESULT(result, strdup("Unsupported metric value type."));
+            sts = SYSINFO_RET_FAIL;
+            break;
+    }
+    return sts;
 }