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