From: Florian Forster <octo@noris.net>
Date: Tue, 18 Mar 2008 09:18:11 +0000 (+0100)
Subject: src/utils_{cache,threshold}.c: Fix the concept of failed and missing values.
X-Git-Tag: collectd-4.3.2~17^2
X-Git-Url: https://git.octo.it/?a=commitdiff_plain;h=c5dd07b67b8f41af7c971117a026c4c1fa406ec9;p=collectd.git

src/utils_{cache,threshold}.c: Fix the concept of failed and missing values.

Apparently I was confused at the time - and still am ;)
---

diff --git a/src/utils_cache.c b/src/utils_cache.c
index 224748f2..b9b89621 100644
--- a/src/utils_cache.c
+++ b/src/utils_cache.c
@@ -305,22 +305,22 @@ int uc_check_timeout (void)
     {
       DEBUG ("uc_check_timeout: %s is missing, sending notification.",
 	  keys[i]);
-      ce->state = STATE_ERROR;
+      ce->state = STATE_MISSING;
     }
     else if (status == 2) /* do not persist */
     {
-      if (ce->state == STATE_ERROR)
+      if (ce->state == STATE_MISSING)
       {
 	DEBUG ("uc_check_timeout: %s is missing but "
 	    "notification has already been sent.",
 	    keys[i]);
 	sfree (keys[i]);
       }
-      else /* (ce->state != STATE_ERROR) */
+      else /* (ce->state != STATE_MISSING) */
       {
 	DEBUG ("uc_check_timeout: %s is missing, sending one notification.",
 	    keys[i]);
-	ce->state = STATE_ERROR;
+	ce->state = STATE_MISSING;
       }
     }
     else
@@ -389,7 +389,7 @@ int uc_update (const data_set_t *ds, const value_list_t *vl)
 
   /* Send a notification (after the lock has been released) if we switch the
    * state from something else to `okay'. */
-  if (ce->state != STATE_OKAY)
+  if (ce->state == STATE_MISSING)
   {
     send_okay_notification = 1;
     ce->state = STATE_OKAY;
@@ -526,11 +526,6 @@ int uc_set_state (const data_set_t *ds, const value_list_t *vl, int state)
   cache_entry_t *ce = NULL;
   int ret = -1;
 
-  if (state < STATE_OKAY)
-    state = STATE_OKAY;
-  if (state > STATE_ERROR)
-    state = STATE_ERROR;
-
   if (FORMAT_VL (name, sizeof (name), vl, ds) != 0)
   {
     ERROR ("uc_get_state: FORMAT_VL failed.");
diff --git a/src/utils_cache.h b/src/utils_cache.h
index d6a56ab4..51d9c031 100644
--- a/src/utils_cache.h
+++ b/src/utils_cache.h
@@ -24,9 +24,10 @@
 
 #include "plugin.h"
 
-#define STATE_OKAY    0
-#define STATE_WARNING 1
-#define STATE_ERROR   2
+#define STATE_OKAY     0
+#define STATE_WARNING  1
+#define STATE_ERROR    2
+#define STATE_MISSING 15
 
 int uc_init (void);
 int uc_check_timeout (void);
diff --git a/src/utils_threshold.c b/src/utils_threshold.c
index ea4b5916..41fcddaf 100644
--- a/src/utils_threshold.c
+++ b/src/utils_threshold.c
@@ -146,9 +146,9 @@ static int ut_config_type_max (threshold_t *th, oconfig_item_t *ci)
   }
 
   if (strcasecmp (ci->key, "WarningMax") == 0)
-    th->warning_min = ci->values[0].value.number;
+    th->warning_max = ci->values[0].value.number;
   else
-    th->failure_min = ci->values[0].value.number;
+    th->failure_max = ci->values[0].value.number;
 
   return (0);
 } /* int ut_config_type_max */
@@ -509,12 +509,22 @@ static threshold_t *threshold_search (const data_set_t *ds,
 
 int ut_check_threshold (const data_set_t *ds, const value_list_t *vl)
 {
+  notification_t n;
   threshold_t *th;
   gauge_t *values;
   int i;
 
+  int state_orig;
+  int state_new = STATE_OKAY;
+  int ds_index = 0;
+
+  char *buf;
+  size_t bufsize;
+  int status;
+
   if (threshold_tree == NULL)
     return (0);
+
   /* Is this lock really necessary? So far, thresholds are only inserted at
    * startup. -octo */
   pthread_mutex_lock (&threshold_lock);
@@ -529,6 +539,8 @@ int ut_check_threshold (const data_set_t *ds, const value_list_t *vl)
   if (values == NULL)
     return (0);
 
+  state_orig = uc_get_state (ds, vl);
+
   for (i = 0; i < ds->ds_num; i++)
   {
     int is_inverted = 0;
@@ -544,92 +556,122 @@ int ut_check_threshold (const data_set_t *ds, const value_list_t *vl)
 	|| (!isnan (th->warning_max) && (th->warning_max < values[i])))
       is_warning = is_inverted - 1;
 
-    if ((is_failure != 0) || (is_warning != 0))
+    if ((is_failure != 0) && (state_new != STATE_ERROR))
     {
-      notification_t n;
-      char *buf;
-      size_t bufsize;
-      int status;
+      state_new = STATE_ERROR;
+      ds_index = i;
+    }
+    else if ((is_warning != 0)
+	&& (state_new != STATE_ERROR)
+	&& (state_new != STATE_WARNING))
+    {
+      state_new = STATE_WARNING;
+      ds_index = i;
+    }
+  }
 
-      double min;
-      double max;
+  if (state_new != state_orig)
+    uc_set_state (ds, vl, state_new);
 
-      min = (is_failure != 0) ? th->failure_min : th->warning_min;
-      max = (is_failure != 0) ? th->failure_max : th->warning_max;
+  /* Return here if we're not going to send a notification */
+  if ((state_new == state_orig)
+      && ((state_new == STATE_OKAY)
+	|| ((th->flags & UT_FLAG_PERSIST) == 0)))
+  {
+    sfree (values);
+    return (0);
+  }
 
-      DEBUG ("ut_check_threshold: ds[%s]: %lf <= !%lf <= %lf (invert: %s)",
-	  ds->ds[i].name, min, values[i], max,
-	  is_inverted ? "true" : "false");
+  NOTIFICATION_INIT_VL (&n, vl, ds);
+  {
+    /* Copy the associative members */
+    if (state_new == STATE_OKAY)
+      n.severity = NOTIF_OKAY;
+    else if (state_new == STATE_WARNING)
+      n.severity = NOTIF_WARNING;
+    else
+      n.severity = NOTIF_FAILURE;
 
-      /* Copy the associative members */
-      NOTIFICATION_INIT_VL (&n, vl, ds);
+    n.time = vl->time;
 
-      n.severity = (is_failure != 0) ? NOTIF_FAILURE : NOTIF_WARNING;
-      n.time = vl->time;
+    buf = n.message;
+    bufsize = sizeof (n.message);
 
-      buf = n.message;
-      bufsize = sizeof (n.message);
+    status = snprintf (buf, bufsize, "Host %s, plugin %s",
+	vl->host, vl->plugin);
+    buf += status;
+    bufsize -= status;
 
-      status = snprintf (buf, bufsize, "Host %s, plugin %s",
-	  vl->host, vl->plugin);
+    if (vl->plugin_instance[0] != '\0')
+    {
+      status = snprintf (buf, bufsize, " (instance %s)",
+	  vl->plugin_instance);
       buf += status;
       bufsize -= status;
+    }
 
-      if (vl->plugin_instance[0] != '\0')
-      {
-	status = snprintf (buf, bufsize, " (instance %s)",
-	    vl->plugin_instance);
-	buf += status;
-	bufsize -= status;
-      }
+    status = snprintf (buf, bufsize, " type %s", ds->type);
+    buf += status;
+    bufsize -= status;
 
-      status = snprintf (buf, bufsize, " type %s", ds->type);
+    if (vl->type_instance[0] != '\0')
+    {
+      status = snprintf (buf, bufsize, " (instance %s)",
+	  vl->type_instance);
       buf += status;
       bufsize -= status;
+    }
+  }
 
-      if (vl->type_instance[0] != '\0')
-      {
-	status = snprintf (buf, bufsize, " (instance %s)",
-	    vl->type_instance);
-	buf += status;
-	bufsize -= status;
-      }
+  /* Send a okay notification */
+  if (state_new == STATE_OKAY)
+  {
+    status = snprintf (buf, bufsize, ": All data sources are within range again.");
+    buf += status;
+    bufsize -= status;
+  }
+  else
+  {
+    double min;
+    double max;
+
+    min = (state_new == STATE_ERROR) ? th->failure_min : th->warning_min;
+    max = (state_new == STATE_ERROR) ? th->failure_max : th->warning_max;
 
-      if (is_inverted)
+    if (th->flags & UT_FLAG_INVERT)
+    {
+      if (!isnan (min) && !isnan (max))
       {
-	if (!isnan (min) && !isnan (max))
-	{
-	  status = snprintf (buf, bufsize, ": Data source \"%s\" is currently "
-	      "%f. That is within the %s region of %f and %f.",
-	      ds->ds[i].name, values[i],
-	      (is_failure != 0) ? "failure" : "warning",
-	      min, min);
-	}
-	else
-	{
-	  status = snprintf (buf, bufsize, ": Data source \"%s\" is currently "
-	      "%f. That is %s the %s threshold of %f.",
-	      ds->ds[i].name, values[i],
-	      isnan (min) ? "below" : "above",
-	      (is_failure != 0) ? "failure" : "warning",
-	      isnan (min) ? max : min);
-	}
+	status = snprintf (buf, bufsize, ": Data source \"%s\" is currently "
+	    "%f. That is within the %s region of %f and %f.",
+	    ds->ds[ds_index].name, values[ds_index],
+	    (state_new == STATE_ERROR) ? "failure" : "warning",
+	    min, min);
       }
-      else /* (!is_inverted) */
+      else
       {
 	status = snprintf (buf, bufsize, ": Data source \"%s\" is currently "
 	    "%f. That is %s the %s threshold of %f.",
-	    ds->ds[i].name, values[i],
-	    (values[i] < min) ? "below" : "above",
-	    (is_failure != 0) ? "failure" : "warning",
-	    (values[i] < min) ? min : max);
+	    ds->ds[ds_index].name, values[ds_index],
+	    isnan (min) ? "below" : "above",
+	    (state_new == STATE_ERROR) ? "failure" : "warning",
+	    isnan (min) ? max : min);
       }
-      buf += status;
-      bufsize -= status;
-
-      plugin_dispatch_notification (&n);
     }
-  } /* for (i = 0; i < ds->ds_num; i++) */
+    else /* is not inverted */
+    {
+      status = snprintf (buf, bufsize, ": Data source \"%s\" is currently "
+	  "%f. That is %s the %s threshold of %f.",
+	  ds->ds[ds_index].name, values[ds_index],
+	  (values[ds_index] < min) ? "below" : "above",
+	  (state_new == STATE_ERROR) ? "failure" : "warning",
+	  (values[ds_index] < min) ? min : max);
+    }
+    buf += status;
+    bufsize -= status;
+  }
+
+  plugin_dispatch_notification (&n);
 
   sfree (values);