Hi everyone,  

as already anounced on IRC, I'd like to bring the percentile calculation
available in the new statsd module to the tail module.  

Reading through the source it seems to be that there is one main difference
between the way in tail.so and statsd;(if I get that correctly?)  

The tail infrastructure does all calculation one by one, and it seems that
only one gauge can be added per regex, so if one wants Average & Max, the
regex has to be there twice?  

In contrast, utils_latency adds values to a data-row, and can run several
calculations on one dataset for reporting once the aggregation timespan has
passed, and values are to be delivered.  

   

So I started knitting eveything together as I thought was right (see patch)
and did some tests; however, are the results correct?  

    <File "/home/willi/test.log">
        Instance "blarg"
        <Match>
            Regex "S=([1-9][0-9]*)"
            DSType "GaugePercentile"
            Type "gauge"
            Instance "percentile"
        </Match>
was my test config, and I generated a logfile using  

for z in `seq 1 1000000`; do sleep 0.1;  for j in `seq 1 10`; do for i in
`seq 1 10`; do printf "S=1${i}000\n" >> test.log; done; touch test.log ;
sleep 1; done; done  

I hacked in some test code to print out the calculated gauges:   

        values[0].gauge = CDTIME_T_TO_DOUBLE (
          latency_counter_get_min(match_value->Counter));
      fprintf(stderr, "Min: %f\n", values[0].gauge);
          latency_counter_get_max  ->      fprintf(stderr, "Max:
%f\n", values[0].gauge);
          latency_counter_get_average -> fprintf(stderr, "Avg: %f\n",
values[0].gauge);
          latency_counter_get_percentile(match_value->Counter, 95));
-> fprintf(stderr, "perc: %f\n", values[0].gauge);  

and it would always give me s.th. like this:  

Min: 0.000010
Max: 0.000102
Avg: 0.000023
perc: 0.001000  

while the input data looks like this:  

S=11000
S=12000
S=13000
S=14000
S=15000
S=16000
S=17000
S=18000
S=19000
S=110000  

so, for the MIN-value  one can clearly see that this should be something
with '11' in it, regardless of where the float pushes the decimal point while
the debug print reads Min: 0.000010 ?  

   

So, my questions are:  

  - do I feed in the data correctly?  

  - do I read out the data correctly?  

  - am I right that the data series can be used to calculate several gauges
in a row?  

  - is my debug printf flawed? how should it look?  

  - to aproach a final state, should all (or most) calculations of tail be
replaced by the latency one?  

  - if the old should live alongside the new ones, make DSType 'Gauges' and
have some more settings with a list of the gauges to calculate?  

TIA,  

Wilfried Goesgens
diff --git a/src/Makefile.am b/src/Makefile.am
index 59454dc..2c16742 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -39,6 +39,7 @@ collectd_SOURCES = collectd.c collectd.h \
 		   utils_subst.c utils_subst.h \
 		   utils_tail.c utils_tail.h \
 		   utils_time.c utils_time.h \
+		   utils_latency.c utils_latency.h \
 		   types_list.c types_list.h
 
 collectd_CPPFLAGS =  $(AM_CPPFLAGS) $(LTDLINCL)
@@ -1091,8 +1092,8 @@ endif
 
 if BUILD_PLUGIN_STATSD
 pkglib_LTLIBRARIES += statsd.la
-statsd_la_SOURCES = statsd.c \
-                    utils_latency.h utils_latency.c
+statsd_la_SOURCES = statsd.c
+
 statsd_la_LDFLAGS = -module -avoid-version
 statsd_la_LIBADD = -lpthread
 collectd_LDADD += "-dlopen" statsd.la
diff --git a/src/tail.c b/src/tail.c
index bcb1572..4dba6d1 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -55,6 +55,7 @@ size_t tail_match_list_num = 0;
 static int ctail_config_add_match_dstype (ctail_config_match_t *cm,
     oconfig_item_t *ci)
 {
+	fprint(stderr, "blaroeug\n");
   if ((ci->values_num != 1) || (ci->values[0].type != OCONFIG_TYPE_STRING))
   {
     WARNING ("tail plugin: `DSType' needs exactly one string argument.");
@@ -72,6 +73,11 @@ static int ctail_config_add_match_dstype (ctail_config_match_t *cm,
       cm->flags |= UTILS_MATCH_CF_GAUGE_MAX;
     else if (strcasecmp ("GaugeLast", ci->values[0].value.string) == 0)
       cm->flags |= UTILS_MATCH_CF_GAUGE_LAST;
+    else if (strcasecmp ("GaugePercentile", ci->values[0].value.string) == 0)
+    {
+      cm->flags |= UTILS_MATCH_CF_GAUGE_PERCENTILE;
+      cm->flags |= UTILS_LATENCY_GAUGE;
+    }
     else
       cm->flags = 0;
   }
@@ -114,7 +120,7 @@ static int ctail_config_add_match_dstype (ctail_config_match_t *cm,
 
   if (cm->flags == 0)
   {
-    WARNING ("tail plugin: `%s' is not a valid argument to `DSType'.",
+    WARNING ("tail plugin: `%s' is not aaoeuaoeuaoeu valid argument to `DSType'.",
 	ci->values[0].value.string);
     return (-1);
   }
diff --git a/src/utils_match.c b/src/utils_match.c
index 062bcfe..c2107d0 100644
--- a/src/utils_match.c
+++ b/src/utils_match.c
@@ -98,6 +98,10 @@ static int default_callback (const char __attribute__((unused)) *str,
 	/ ((double) (data->values_num + 1));
       data->value.gauge = (data->value.gauge * f) + (value * (1.0 - f));
     }
+    else if (data->ds_type & UTILS_MATCH_CF_GAUGE_PERCENTILE)
+    {
+	    latency_counter_add(data->Counter, value);
+    }
     else if (data->ds_type & UTILS_MATCH_CF_GAUGE_MIN)
     {
       if (data->value.gauge > value)
@@ -110,7 +114,7 @@ static int default_callback (const char __attribute__((unused)) *str,
     }
     else
     {
-      ERROR ("utils_match: default_callback: obj->ds_type is invalid!");
+      ERROR ("utils_match: default_callback: obj->ds_type is invalid 1!");
       return (-1);
     }
 
@@ -141,7 +145,7 @@ static int default_callback (const char __attribute__((unused)) *str,
       data->value.counter += value;
     else
     {
-      ERROR ("utils_match: default_callback: obj->ds_type is invalid!");
+      ERROR ("utils_match: default_callback: obj->ds_type is invalid 2!");
       return (-1);
     }
 
@@ -172,7 +176,7 @@ static int default_callback (const char __attribute__((unused)) *str,
       data->value.derive += value;
     else
     {
-      ERROR ("utils_match: default_callback: obj->ds_type is invalid!");
+      ERROR ("utils_match: default_callback: obj->ds_type is invalid 3!");
       return (-1);
     }
 
@@ -194,7 +198,7 @@ static int default_callback (const char __attribute__((unused)) *str,
       data->value.absolute = value;
     else
     {
-      ERROR ("utils_match: default_callback: obj->ds_type is invalid!");
+      ERROR ("utils_match: default_callback: obj->ds_type is invalid 4!");
       return (-1);
     }
 
@@ -202,7 +206,7 @@ static int default_callback (const char __attribute__((unused)) *str,
   }
   else
   {
-    ERROR ("utils_match: default_callback: obj->ds_type is invalid!");
+    ERROR ("utils_match: default_callback: obj->ds_type is invalid 5!");
     return (-1);
   }
 
@@ -248,6 +252,10 @@ cu_match_t *match_create_callback (const char *regex, const char *excluderegex,
     obj->flags |= UTILS_MATCH_FLAGS_EXCLUDE_REGEX;
   }
 
+  if (1)
+  {
+  }
+///  obj->counter = 
   obj->callback = callback;
   obj->user_data = user_data;
 
@@ -265,6 +273,8 @@ cu_match_t *match_create_simple (const char *regex,
     return (NULL);
   memset (user_data, '\0', sizeof (cu_match_value_t));
   user_data->ds_type = match_ds_type;
+  if (match_ds_type & UTILS_LATENCY_GAUGE)
+	  user_data->Counter = latency_counter_create ();
 
   obj = match_create_callback (regex, excluderegex,
 			       default_callback, user_data);
@@ -275,7 +285,7 @@ cu_match_t *match_create_simple (const char *regex,
   }
 
   obj->flags |= UTILS_MATCH_FLAGS_FREE_USER_DATA;
-
+  /// TODO: howto free user data -> latency?
   return (obj);
 } /* cu_match_t *match_create_simple */
 
@@ -286,6 +296,7 @@ void match_destroy (cu_match_t *obj)
 
   if (obj->flags & UTILS_MATCH_FLAGS_FREE_USER_DATA)
   {
+  /// TODO: howto free user data -> latency?
     sfree (obj->user_data);
   }
 
@@ -347,7 +358,7 @@ int match_apply (cu_match_t *obj, const char *str)
     status = obj->callback (str, matches, matches_num, obj->user_data);
     if (status != 0)
     {
-      ERROR ("utils_match: match_apply: callback failed.");
+      ERROR ("utils_match: match_apply: callback failed. 1");
     }
   }
 
diff --git a/src/utils_match.h b/src/utils_match.h
index 36abe30..201ae04 100644
--- a/src/utils_match.h
+++ b/src/utils_match.h
@@ -24,19 +24,23 @@
 #define UTILS_MATCH_H 1
 
 #include "plugin.h"
-
+#include "utils_latency.h"
 /*
  * Defines
  */
-#define UTILS_MATCH_DS_TYPE_GAUGE    0x10
-#define UTILS_MATCH_DS_TYPE_COUNTER  0x20
-#define UTILS_MATCH_DS_TYPE_DERIVE   0x40
-#define UTILS_MATCH_DS_TYPE_ABSOLUTE 0x80
+#define UTILS_MATCH_DS_TYPE_GAUGE    0x20
+#define UTILS_MATCH_DS_TYPE_COUNTER  0x40
+#define UTILS_MATCH_DS_TYPE_DERIVE   0x80
+#define UTILS_MATCH_DS_TYPE_ABSOLUTE 0x100
+#define UTILS_MATCH_DS_TYPE_PERCENTILE 0x200
 
 #define UTILS_MATCH_CF_GAUGE_AVERAGE 0x01
 #define UTILS_MATCH_CF_GAUGE_MIN     0x02
 #define UTILS_MATCH_CF_GAUGE_MAX     0x04
 #define UTILS_MATCH_CF_GAUGE_LAST    0x08
+#define UTILS_MATCH_CF_GAUGE_PERCENTILE 0x10
+
+#define UTILS_LATENCY_GAUGE 0x400
 
 #define UTILS_MATCH_CF_COUNTER_SET   0x01
 #define UTILS_MATCH_CF_COUNTER_ADD   0x02
@@ -61,6 +65,7 @@ struct cu_match_value_s
   int ds_type;
   value_t value;
   unsigned int values_num;
+  latency_counter_t *Counter;
 };
 typedef struct cu_match_value_s cu_match_value_t;
 
diff --git a/src/utils_tail_match.c b/src/utils_tail_match.c
index 8ae2208..7ee4845 100644
--- a/src/utils_tail_match.c
+++ b/src/utils_tail_match.c
@@ -75,11 +75,27 @@ static int simple_submit_match (cu_match_t *match, void *user_data)
   if ((match_value->ds_type & UTILS_MATCH_DS_TYPE_GAUGE)
       && (match_value->values_num == 0))
     values[0].gauge = NAN;
+  else if (match_value->ds_type & UTILS_LATENCY_GAUGE)
+  {
+  	  values[0].gauge = CDTIME_T_TO_DOUBLE (
+		  latency_counter_get_min(match_value->Counter));
+	  fprintf(stderr, "Min: %f\n", values[0].gauge);
+  	  values[0].gauge = CDTIME_T_TO_DOUBLE (
+		  latency_counter_get_max(match_value->Counter));
+	  fprintf(stderr, "Max: %f\n", values[0].gauge);
+  	  values[0].gauge = CDTIME_T_TO_DOUBLE (
+		  latency_counter_get_average(match_value->Counter));
+	  fprintf(stderr, "Avg: %f\n", values[0].gauge);
+  	  values[0].gauge = CDTIME_T_TO_DOUBLE (
+		  latency_counter_get_percentile(match_value->Counter, 95));
+	  fprintf(stderr, "perc: %f\n", values[0].gauge);
+  }
   else
     values[0] = match_value->value;
 
   vl.values = values;
   vl.values_len = 1;
+
   sstrncpy (vl.host, hostname_g, sizeof (vl.host));
   sstrncpy (vl.plugin, data->plugin, sizeof (vl.plugin));
   sstrncpy (vl.plugin_instance, data->plugin_instance,
@@ -95,6 +111,10 @@ static int simple_submit_match (cu_match_t *match, void *user_data)
     match_value->value.gauge = NAN;
     match_value->values_num = 0;
   }
+  else if (match_value->ds_type & UTILS_LATENCY_GAUGE)
+  {
+    latency_counter_reset(match_value->Counter);
+  }
 
   return (0);
 } /* int simple_submit_match */
_______________________________________________
collectd mailing list
collectd@verplant.org
http://mailman.verplant.org/listinfo/collectd

Reply via email to