From: Florian Forster <octo@collectd.org>
Date: Sun, 20 Nov 2016 21:53:34 +0000 (+0100)
Subject: tail plugin: Rename DSType from Latency to Distribution.
X-Git-Tag: collectd-5.7.0~16
X-Git-Url: https://git.tokkee.org/?p=collectd.git;a=commitdiff_plain;h=11ca18db12e489c40aa0ac41672872cfae4b1f0b

tail plugin: Rename DSType from Latency to Distribution.

This is a more flexible naming owing to the fact that percentiles and other
distribution parameters are interesting for a variety of metrics, not just
latencies.

The config handling is now completely in src/utils_latency_config.c so
that other plugins, such as the cURL plugin, can easily reuse the module
with consistent config handling.
---

diff --git a/src/collectd.conf.in b/src/collectd.conf.in
index 1a696693..e5b96435 100644
--- a/src/collectd.conf.in
+++ b/src/collectd.conf.in
@@ -1278,18 +1278,19 @@
 #    Instance "apache"
 #    <Match>
 #      Regex "^\\S+ \"([0-9.]+)\""
-#      DSType "Latency"
-#      Type "response_time"
-#      #LatencyPercentileType "percentile"
-#      LatencyPercentile 5
-#      LatencyPercentile 30
-#      LatencyPercentile 50
-#      LatencyPercentile 70
-#      LatencyPercentile 95
-#      LatencyRateType "requests"
-#      LatencyRate 0     0.100
-#      LatencyRate 0.100 0.200
-#      LatencyRate 0.200 0
+#      <DSType Distribution>
+#        Percentile 80    # -> latency-foo-80
+#        Percentile 95    # -> latency-foo-95
+#        Percentile 99    # -> latency-foo-99
+#        Bucket 0   0.1   # -> bucket-latency-foo-0_0.1
+#        Bucket 0.1 0.2   # -> bucket-latency-foo-0.1_0.2
+#        Bucket 0.2 0.5   # -> bucket-latency-foo-0.2_0.5
+#        Bucket 0.5 1.0   # -> bucket-latency-foo-0.5_1
+#        Bucket 1.0 2.0   # -> bucket-latency-foo-1_2
+#        Bucket 2.0 0     # -> bucket-latency-foo-2_inf
+#      </DSType>
+#      Type "latency"
+#      Instance "foo"
 #    </Match>
 #  </File>
 #</Plugin>
diff --git a/src/collectd.conf.pod b/src/collectd.conf.pod
index 1abca916..6af58d4a 100644
--- a/src/collectd.conf.pod
+++ b/src/collectd.conf.pod
@@ -7089,6 +7089,15 @@ user using (extended) regular expressions, as described in L<regex(7)>.
         Type "counter"
         Instance "local_user"
       </Match>
+      <Match>
+        Regex "l=([0-9]*\\.[0-9]*)"
+        <DSType "Distribution">
+          Percentile 99
+          Bucket 0 100
+        </DSType>
+        Type "latency"
+        Instance "foo"
+      </Match>
     </File>
   </Plugin>
 
@@ -7187,59 +7196,83 @@ Increase the internal counter by one. These B<DSType> are the only ones that do
 not use the matched subexpression, but simply count the number of matched
 lines. Thus, you may use a regular expression without submatch in this case.
 
-=item B<Latency>
+=item B<Distribution>
 
-Special type to handle latency values from logfiles. The matched value must be
-latency in seconds, floating point numbers are supported.
-Should be used with B<LatencyPercentile> or B<LatencyRate> options.
+Type to do calculations based on the distribution of values, primarily
+calculating percentiles. This is primarily geared towards latency, but can be
+used for other metrics as well. The range of values tracked with this setting
+must be in the range (0–2^34) and can be fractional. Please note that neither
+zero nor 2^34 are inclusive bounds, i.e. zero I<cannot> be handled by a
+distribution.
 
-The B<Instance> option cannot be used together with B<DSType> B<Latency>.
+This option must be used together with the B<Percentile> and/or B<Bucket>
+options.
 
-=back
+B<Synopsis:>
 
-As you'd expect the B<Gauge*> types interpret the submatch as a floating point
-number, using L<strtod(3)>. The B<Counter*> and B<AbsoluteSet> types interpret
-the submatch as an unsigned integer using L<strtoull(3)>. The B<Derive*> types
-interpret the submatch as a signed integer using L<strtoll(3)>. B<CounterInc>
-and B<DeriveInc> do not use the submatch at all and it may be omitted in this
-case.
+  <DSType "Distribution">
+    Percentile 99
+    Bucket 0 100
+  </DSType>
 
-=item B<Type> I<Type>
+=over 4
 
-Sets the type used to dispatch this value. Detailed information about types and
-their configuration can be found in L<types.db(5)>.
+=item B<Percentile> I<Percent>
 
-=item B<Instance> I<TypeInstance>
+Calculate and dispatch the configured percentile, i.e. compute the value, so
+that I<Percent> of all matched values are smaller than or equal to the computed
+latency.
 
-This optional setting sets the type instance to use.
+Metrics are reported with the I<type> B<Type> (the value of the above option)
+and the I<type instance> C<[E<lt>InstanceE<gt>-]E<lt>PercentE<gt>>.
 
-=item B<LatencyPercentile> I<Percent>
+This option may be repeated to calculate more than one percentile.
 
-Calculate and dispatch the configured percentile, i.e. compute the latency, so
-that I<Percent> of all matched latency values are smaller than or equal to the
-computed latency.
+=item B<Bucket> I<lower_bound> I<upper_bound>
 
-Different percentiles can be calculated by setting this option several times.
+Export the number of values (a C<DERIVE>) falling within the given range. Both,
+I<lower_bound> and I<upper_bound> may be a fractional number, such as B<0.5>.
+Each B<Bucket> option specifies an interval C<(I<lower_bound>,
+I<upper_bound>]>, i.e. the range I<excludes> the lower bound and I<includes>
+the upper bound. I<lower_bound> and I<upper_bound> may be zero, meaning no
+lower/upper bound.
 
-=item B<LatencyPercentileType> I<Type>
+To export the entire (0–inf) range without overlap, use the upper bound of the
+previous range as the lower bound of the following range. In other words, use
+the following schema:
 
-Sets the type used to dispatch B<LatencyPercentile> values.
+  Bucket   0   1
+  Bucket   1   2
+  Bucket   2   5
+  Bucket   5  10
+  Bucket  10  20
+  Bucket  20  50
+  Bucket  50   0
 
-=item B<LatencyRate> I<lower_latency> I<upper_latency>
+Metrics are reported with the I<type> C<bucket> and the I<type instance>
+C<E<lt>TypeE<gt>[-E<lt>InstanceE<gt>]-E<lt>lower_boundE<gt>_E<lt>upper_boundE<gt>>.
 
-Calculate and dispatch rate of latency values fall within requested interval.
-Both, I<lower_latency> and I<upper_latency> is a duration in seconds and can be
-a fractional number, such as B<0.5>. The settings specify the interval
-C<(I<lower_latency>, I<upper_latency>]>, i.e. the range I<excludes> the lower
-bound and I<includes> the upper bound. I<lower_latency> and I<upper_latency>
-can be zero, meaning no lower/upper bound.
+This option may be repeated to calculate more than one rate.
 
-Rates for different intervals can be calculated by setting this option several
-times.
+=back
+
+=back
 
-=item B<LatencyRateType> I<Type>
+The B<Gauge*> and B<Distribution> types interpret the submatch as a floating
+point number, using L<strtod(3)>. The B<Counter*> and B<AbsoluteSet> types
+interpret the submatch as an unsigned integer using L<strtoull(3)>. The
+B<Derive*> types interpret the submatch as a signed integer using
+L<strtoll(3)>. B<CounterInc> and B<DeriveInc> do not use the submatch at all
+and it may be omitted in this case.
 
-Sets the type used to dispatch B<LatencyRate> values.
+=item B<Type> I<Type>
+
+Sets the type used to dispatch this value. Detailed information about types and
+their configuration can be found in L<types.db(5)>.
+
+=item B<Instance> I<TypeInstance>
+
+This optional setting sets the type instance to use.
 
 =back
 
diff --git a/src/daemon/utils_tail_match.c b/src/daemon/utils_tail_match.c
index 5dfd10ac..b52a319b 100644
--- a/src/daemon/utils_tail_match.c
+++ b/src/daemon/utils_tail_match.c
@@ -104,92 +104,76 @@ static int simple_submit_match (cu_match_t *match, void *user_data)
   return (0);
 } /* int simple_submit_match */
 
-static int simple_submit_latency (cu_match_t *match, void *user_data)
-{
-  cu_tail_match_simple_t *data = (cu_tail_match_simple_t *) user_data;
+static int latency_submit_match(cu_match_t *match, void *user_data) {
+  cu_tail_match_simple_t *data = (cu_tail_match_simple_t *)user_data;
   cu_match_value_t *match_value;
   value_list_t vl = VALUE_LIST_INIT;
 
-  match_value = (cu_match_value_t *) match_get_user_data (match);
+  match_value = (cu_match_value_t *)match_get_user_data(match);
   if (match_value == NULL)
     return (-1);
 
-  vl.values = &(value_t) { .gauge = NAN };
-  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,
-      sizeof (vl.plugin_instance));
-  sstrncpy (vl.type, data->type, sizeof (vl.type));
+  sstrncpy(vl.host, hostname_g, sizeof(vl.host));
+  sstrncpy(vl.plugin, data->plugin, sizeof(vl.plugin));
+  sstrncpy(vl.plugin_instance, data->plugin_instance,
+           sizeof(vl.plugin_instance));
   vl.interval = data->interval;
-  vl.time = cdtime ();
-
-  if (data->latency_config.lower) {
-    ssnprintf (vl.type_instance, sizeof (vl.type_instance),
-        "lower");
-    vl.values[0].gauge = (match_value->values_num != 0)
-      ? CDTIME_T_TO_DOUBLE (latency_counter_get_min (match_value->latency))
-      : NAN;
-    plugin_dispatch_values (&vl);
-  }
-
-  if (data->latency_config.avg) {
-    ssnprintf (vl.type_instance, sizeof (vl.type_instance),
-        "average");
-    vl.values[0].gauge = (match_value->values_num != 0)
-      ? CDTIME_T_TO_DOUBLE (latency_counter_get_average (match_value->latency))
-      : NAN;
-    plugin_dispatch_values (&vl);
-  }
-
-  if (data->latency_config.upper) {
-    ssnprintf (vl.type_instance, sizeof (vl.type_instance),
-        "upper");
-    vl.values[0].gauge = (match_value->values_num != 0)
-      ? CDTIME_T_TO_DOUBLE (latency_counter_get_max (match_value->latency))
-      : NAN;
-    plugin_dispatch_values (&vl);
-  }
+  vl.time = cdtime();
 
   /* Submit percentiles */
-  if (data->latency_config.percentile_type != NULL)
-    sstrncpy (vl.type, data->latency_config.percentile_type, sizeof (vl.type));
-  for (size_t i = 0; i < data->latency_config.percentile_num; i++)
-  {
-    ssnprintf (vl.type_instance, sizeof (vl.type_instance),
-        "percentile-%.0f",  data->latency_config.percentile[i]);
-    vl.values[0].gauge = (match_value->values_num != 0)
-      ? CDTIME_T_TO_DOUBLE (latency_counter_get_percentile (match_value->latency,
-                                            data->latency_config.percentile[i]))
-      : NAN;
-    plugin_dispatch_values (&vl);
+  sstrncpy(vl.type, data->type, sizeof(vl.type));
+  for (size_t i = 0; i < data->latency_config.percentile_num; i++) {
+    if (strlen(data->type_instance) != 0)
+      ssnprintf(vl.type_instance, sizeof(vl.type_instance), "%s-%.0f",
+                data->type_instance, data->latency_config.percentile[i]);
+    else
+      ssnprintf(vl.type_instance, sizeof(vl.type_instance), "%.0f",
+                data->latency_config.percentile[i]);
+
+    vl.values = &(value_t){
+        .gauge =
+            (match_value->values_num != 0)
+                ? CDTIME_T_TO_DOUBLE(latency_counter_get_percentile(
+                      match_value->latency, data->latency_config.percentile[i]))
+                : NAN,
+    };
+    vl.values_len = 1;
+
+    plugin_dispatch_values(&vl);
   }
 
-  /* Submit rates */
-  sstrncpy (vl.type, data->type, sizeof (vl.type));
-  if (data->latency_config.rates_type != NULL)
-    sstrncpy (vl.type, data->latency_config.rates_type, sizeof (vl.type));
-  for (size_t i = 0; i < data->latency_config.rates_num; i++)
-  {
-    ssnprintf (vl.type_instance, sizeof (vl.type_instance),
-        "rate-%.3f-%.3f",
-        CDTIME_T_TO_DOUBLE(data->latency_config.rates[i * 2]),
-        CDTIME_T_TO_DOUBLE(data->latency_config.rates[i * 2 + 1]));
-    vl.values[0].gauge = (match_value->values_num != 0) 
-      ? latency_counter_get_rate (match_value->latency,
-                                  data->latency_config.rates[i * 2],
-                                  data->latency_config.rates[i * 2 + 1],
-                                  vl.time)
-      : NAN;
-    plugin_dispatch_values (&vl);
+  /* Submit buckets */
+  sstrncpy(vl.type, "bucket", sizeof(vl.type));
+  for (size_t i = 0; i < data->latency_config.buckets_num; i++) {
+    latency_bucket_t bucket = data->latency_config.buckets[i];
+
+    double lower_bound = CDTIME_T_TO_DOUBLE(bucket.lower_bound);
+    double upper_bound =
+        bucket.upper_bound ? CDTIME_T_TO_DOUBLE(bucket.upper_bound) : INFINITY;
+
+    if (strlen(data->type_instance) != 0)
+      ssnprintf(vl.type_instance, sizeof(vl.type_instance), "%s-%s-%g_%g",
+                data->type, data->type_instance, lower_bound, upper_bound);
+    else
+      ssnprintf(vl.type_instance, sizeof(vl.type_instance), "%s-%g_%g",
+                data->type, lower_bound, upper_bound);
+
+    vl.values = &(value_t){
+        .gauge = latency_counter_get_rate(match_value->latency,
+                                          bucket.lower_bound,
+                                          bucket.upper_bound, vl.time),
+    };
+    vl.values_len = 1;
+
+    plugin_dispatch_values(&vl);
   }
-  latency_counter_reset (match_value->latency);
 
   match_value->value.gauge = NAN;
   match_value->values_num = 0;
+  latency_counter_reset(match_value->latency);
 
   return (0);
-} /* int simple_submit_latency */
+} /* int latency_submit_match */
 
 static int tail_callback (void *data, char *buf,
     int __attribute__((unused)) buflen)
@@ -311,12 +295,12 @@ int tail_match_add_match_simple (cu_tail_match_t *obj,
   sstrncpy (user_data->plugin, plugin, sizeof (user_data->plugin));
   if (plugin_instance != NULL)
     sstrncpy (user_data->plugin_instance, plugin_instance,
-	sizeof (user_data->plugin_instance));
+        sizeof (user_data->plugin_instance));
 
   sstrncpy (user_data->type, type, sizeof (user_data->type));
   if (type_instance != NULL)
     sstrncpy (user_data->type_instance, type_instance,
-	sizeof (user_data->type_instance));
+        sizeof (user_data->type_instance));
 
   user_data->interval = interval;
 
@@ -331,7 +315,7 @@ int tail_match_add_match_simple (cu_tail_match_t *obj,
       goto out;
     }
 
-    status = tail_match_add_match (obj, match, simple_submit_latency,
+    status = tail_match_add_match (obj, match, latency_submit_match,
       user_data, tail_match_simple_free);
   } else {
     status = tail_match_add_match (obj, match, simple_submit_match,
diff --git a/src/tail.c b/src/tail.c
index 022ecfd1..c1e55621 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -72,58 +72,63 @@ static int ctail_config_add_match_dstype (ctail_config_match_t *cm,
     return (-1);
   }
 
-  if (strncasecmp ("Gauge", ci->values[0].value.string, strlen ("Gauge")) == 0)
+  char const *ds_type = ci->values[0].value.string;
+  if (strncasecmp ("Gauge", ds_type, strlen ("Gauge")) == 0)
   {
     cm->flags = UTILS_MATCH_DS_TYPE_GAUGE;
-    if (strcasecmp ("GaugeAverage", ci->values[0].value.string) == 0)
+    if (strcasecmp ("GaugeAverage", ds_type) == 0)
       cm->flags |= UTILS_MATCH_CF_GAUGE_AVERAGE;
-    else if (strcasecmp ("GaugeMin", ci->values[0].value.string) == 0)
+    else if (strcasecmp ("GaugeMin", ds_type) == 0)
       cm->flags |= UTILS_MATCH_CF_GAUGE_MIN;
-    else if (strcasecmp ("GaugeMax", ci->values[0].value.string) == 0)
+    else if (strcasecmp ("GaugeMax", ds_type) == 0)
       cm->flags |= UTILS_MATCH_CF_GAUGE_MAX;
-    else if (strcasecmp ("GaugeLast", ci->values[0].value.string) == 0)
+    else if (strcasecmp ("GaugeLast", ds_type) == 0)
       cm->flags |= UTILS_MATCH_CF_GAUGE_LAST;
-    else if (strcasecmp ("GaugeInc", ci->values[0].value.string) == 0)
+    else if (strcasecmp ("GaugeInc", ds_type) == 0)
       cm->flags |= UTILS_MATCH_CF_GAUGE_INC;
-    else if (strcasecmp ("GaugeAdd", ci->values[0].value.string) == 0)
+    else if (strcasecmp ("GaugeAdd", ds_type) == 0)
       cm->flags |= UTILS_MATCH_CF_GAUGE_ADD;
     else if (strcasecmp ("GaugePersist", ci->values[0].value.string) == 0)
       cm->flags |= UTILS_MATCH_CF_GAUGE_PERSIST;
     else
       cm->flags = 0;
   }
-  else if (strcasecmp ("Latency", ci->values[0].value.string) == 0)
+  else if (strcasecmp ("Distribution", ds_type) == 0)
   {
     cm->flags = UTILS_MATCH_DS_TYPE_GAUGE | UTILS_MATCH_CF_GAUGE_LATENCY;
+
+    int status = latency_config (&cm->latency, ci, "tail");
+    if (status != 0)
+      return (status);
   }
-  else if (strncasecmp ("Counter", ci->values[0].value.string, strlen ("Counter")) == 0)
+  else if (strncasecmp ("Counter", ds_type, strlen ("Counter")) == 0)
   {
     cm->flags = UTILS_MATCH_DS_TYPE_COUNTER;
-    if (strcasecmp ("CounterSet", ci->values[0].value.string) == 0)
+    if (strcasecmp ("CounterSet", ds_type) == 0)
       cm->flags |= UTILS_MATCH_CF_COUNTER_SET;
-    else if (strcasecmp ("CounterAdd", ci->values[0].value.string) == 0)
+    else if (strcasecmp ("CounterAdd", ds_type) == 0)
       cm->flags |= UTILS_MATCH_CF_COUNTER_ADD;
-    else if (strcasecmp ("CounterInc", ci->values[0].value.string) == 0)
+    else if (strcasecmp ("CounterInc", ds_type) == 0)
       cm->flags |= UTILS_MATCH_CF_COUNTER_INC;
     else
       cm->flags = 0;
   }
-  else if (strncasecmp ("Derive", ci->values[0].value.string, strlen ("Derive")) == 0)
+  else if (strncasecmp ("Derive", ds_type, strlen ("Derive")) == 0)
   {
     cm->flags = UTILS_MATCH_DS_TYPE_DERIVE;
-    if (strcasecmp ("DeriveSet", ci->values[0].value.string) == 0)
+    if (strcasecmp ("DeriveSet", ds_type) == 0)
       cm->flags |= UTILS_MATCH_CF_DERIVE_SET;
-    else if (strcasecmp ("DeriveAdd", ci->values[0].value.string) == 0)
+    else if (strcasecmp ("DeriveAdd", ds_type) == 0)
       cm->flags |= UTILS_MATCH_CF_DERIVE_ADD;
-    else if (strcasecmp ("DeriveInc", ci->values[0].value.string) == 0)
+    else if (strcasecmp ("DeriveInc", ds_type) == 0)
       cm->flags |= UTILS_MATCH_CF_DERIVE_INC;
     else
       cm->flags = 0;
   }
-  else if (strncasecmp ("Absolute", ci->values[0].value.string, strlen ("Absolute")) == 0)
+  else if (strncasecmp ("Absolute", ds_type, strlen ("Absolute")) == 0)
   {
     cm->flags = UTILS_MATCH_DS_TYPE_ABSOLUTE;
-    if (strcasecmp ("AbsoluteSet", ci->values[0].value.string) == 0)
+    if (strcasecmp ("AbsoluteSet", ds_type) == 0)
       cm->flags |= UTILS_MATCH_CF_ABSOLUTE_SET;
     else
       cm->flags = 0;
@@ -169,28 +174,6 @@ static int ctail_config_add_match (cu_tail_match_t *tm,
       status = cf_util_get_string (option, &cm.type);
     else if (strcasecmp ("Instance", option->key) == 0)
       status = cf_util_get_string (option, &cm.type_instance);
-    else if (strncasecmp ("Latency", option->key, strlen ("Latency")) == 0)
-    {
-      if (strcasecmp ("LatencyPercentile", option->key) == 0)
-        status = latency_config_add_percentile ("tail", &cm.latency, option);
-      else if (strcasecmp ("LatencyPercentileType", option->key) == 0)
-        status = cf_util_get_string (option, &cm.latency.percentile_type);
-      else if (strcasecmp ("LatencyRate", option->key) == 0)
-        status = latency_config_add_rate ("tail", &cm.latency, option);
-      else if (strcasecmp ("LatencyRateType", option->key) == 0)
-        status = cf_util_get_string (option, &cm.latency.rates_type);
-      else if (strcasecmp ("LatencyLower", option->key) == 0)
-        status = cf_util_get_boolean (option, &cm.latency.lower);
-      else if (strcasecmp ("LatencyUpper", option->key) == 0)
-        status = cf_util_get_boolean (option, &cm.latency.upper);
-      else if (strcasecmp ("LatencyAvg", option->key) == 0)
-        status = cf_util_get_boolean (option, &cm.latency.avg);
-      else 
-      {
-        WARNING ("tail plugin: Option `%s' not allowed here.", option->key);
-        status = -1;
-      }
-    }
     else
     {
       WARNING ("tail plugin: Option `%s' not allowed here.", option->key);
@@ -224,40 +207,18 @@ static int ctail_config_add_match (cu_tail_match_t *tm,
       break;
     }
 
-    if ((cm.flags & UTILS_MATCH_DS_TYPE_GAUGE)
-        && (cm.flags & UTILS_MATCH_CF_GAUGE_LATENCY))
-    {
-
-      if (cm.type_instance != NULL)
-      {
-        WARNING ("tail plugin: `DSType Latency' and `Instance %s' in `Match' "
-                 "block could not be used together.", cm.type_instance);
-        status = -1;
-        break;
-      }
-
-      if (cm.latency.percentile_num == 0 && cm.latency.rates_num == 0)
-      {
-        WARNING ("tail plugin: `Match' with `DSType Latency' has no "
-                 "`LatencyPercentile' or `LatencyRate' options.");
-        status = -1;
-        break;
-      }
-    }
-
     break;
   } /* while (status == 0) */
 
   if (status == 0)
   {
+    // TODO(octo): there's nothing "simple" about the latency stuff …
     status = tail_match_add_match_simple (tm, cm.regex, cm.excluderegex,
       cm.flags, "tail", plugin_instance, cm.type, cm.type_instance,
       cm.latency, interval);
 
     if (status != 0)
-    {
       ERROR ("tail plugin: tail_match_add_match_simple failed.");
-    }
   }
 
   sfree (cm.regex);
diff --git a/src/types.db b/src/types.db
index cb7501ec..68551875 100644
--- a/src/types.db
+++ b/src/types.db
@@ -9,6 +9,7 @@ ath_stat                value:DERIVE:0:U
 backends                value:GAUGE:0:65535
 bitrate                 value:GAUGE:0:4294967295
 blocked_clients         value:GAUGE:0:U
+bucket                  value:GAUGE:0:U
 bytes                   value:GAUGE:0:U
 cache_eviction          value:DERIVE:0:U
 cache_operation         value:DERIVE:0:U
diff --git a/src/utils_latency_config.c b/src/utils_latency_config.c
index f99fe583..36c78ebe 100644
--- a/src/utils_latency_config.c
+++ b/src/utils_latency_config.c
@@ -25,130 +25,126 @@
  *   Pavel Rochnyack <pavel2000 at ngs.ru>
  */
 
+#include "utils_latency_config.h"
 #include "collectd.h"
 #include "common.h"
-#include "utils_latency_config.h"
-
-int latency_config_add_percentile(const char *plugin, latency_config_t *cl,
-                                  oconfig_item_t *ci) {
-  if ((ci->values_num != 1) || (ci->values[0].type != OCONFIG_TYPE_NUMBER)) {
-    ERROR("%s plugin: \"%s\" requires exactly one numeric argument.", plugin,
-          ci->key);
-    return (-1);
-  }
 
-  double percent = ci->values[0].value.number;
-  double *tmp;
+static int latency_config_add_percentile(latency_config_t *conf,
+                                         oconfig_item_t *ci,
+                                         const char *plugin) {
+  double percent;
+  int status = cf_util_get_double(ci, &percent);
+  if (status != 0)
+    return status;
 
   if ((percent <= 0.0) || (percent >= 100)) {
     ERROR("%s plugin: The value for \"%s\" must be between 0 and 100, "
           "exclusively.",
           plugin, ci->key);
-    return (ERANGE);
+    return ERANGE;
   }
 
-  tmp = realloc(cl->percentile,
-                sizeof(*cl->percentile) * (cl->percentile_num + 1));
+  double *tmp = realloc(conf->percentile,
+                        sizeof(*conf->percentile) * (conf->percentile_num + 1));
   if (tmp == NULL) {
     ERROR("%s plugin: realloc failed.", plugin);
-    return (ENOMEM);
+    return ENOMEM;
   }
-  cl->percentile = tmp;
-  cl->percentile[cl->percentile_num] = percent;
-  cl->percentile_num++;
+  conf->percentile = tmp;
+  conf->percentile[conf->percentile_num] = percent;
+  conf->percentile_num++;
 
-  return (0);
+  return 0;
 } /* int latency_config_add_percentile */
 
-int latency_config_add_rate(const char *plugin, latency_config_t *cl,
-                            oconfig_item_t *ci) {
+static int latency_config_add_bucket(latency_config_t *conf, oconfig_item_t *ci,
+                                     const char *plugin) {
   if ((ci->values_num != 2) || (ci->values[0].type != OCONFIG_TYPE_NUMBER) ||
       (ci->values[1].type != OCONFIG_TYPE_NUMBER)) {
     ERROR("%s plugin: \"%s\" requires exactly two numeric arguments.", plugin,
           ci->key);
-    return (-1);
+    return EINVAL;
   }
 
   if (ci->values[1].value.number &&
       ci->values[1].value.number <= ci->values[0].value.number) {
     ERROR("%s plugin: MIN must be less than MAX in \"%s\".", plugin, ci->key);
-    return (-1);
+    return ERANGE;
   }
 
-  if (ci->values[0].value.number < 0.001) {
-    ERROR("%s plugin: MIN must be greater or equal to 0.001 in \"%s\".", plugin,
-          ci->key);
-    return (-1);
+  if (ci->values[0].value.number < 0) {
+    ERROR("%s plugin: MIN must be greater then or equal to zero in \"%s\".",
+          plugin, ci->key);
+    return ERANGE;
   }
 
-  cdtime_t lower = DOUBLE_TO_CDTIME_T(ci->values[0].value.number);
-  cdtime_t upper = DOUBLE_TO_CDTIME_T(ci->values[1].value.number);
-  cdtime_t *tmp;
-
-  tmp = realloc(cl->rates, sizeof(*cl->rates) * (cl->rates_num + 1) * 2);
+  latency_bucket_t *tmp =
+      realloc(conf->buckets, sizeof(*conf->buckets) * (conf->buckets_num + 1));
   if (tmp == NULL) {
     ERROR("%s plugin: realloc failed.", plugin);
-    return (ENOMEM);
+    return ENOMEM;
   }
-  cl->rates = tmp;
-  cl->rates[cl->rates_num * 2] = lower;
-  cl->rates[cl->rates_num * 2 + 1] = upper;
-  cl->rates_num++;
+  conf->buckets = tmp;
+  conf->buckets[conf->buckets_num] = (latency_bucket_t){
+      .lower_bound = DOUBLE_TO_CDTIME_T(ci->values[0].value.number),
+      .upper_bound = DOUBLE_TO_CDTIME_T(ci->values[1].value.number),
+  };
+  conf->buckets_num++;
 
   return (0);
-} /* int latency_config_add_rate */
+} /* int latency_config_add_bucket */
+
+int latency_config(latency_config_t *conf, oconfig_item_t *ci,
+                   char const *plugin) {
+  int status = 0;
+
+  for (int i = 0; i < ci->children_num; i++) {
+    oconfig_item_t *child = ci->children + i;
+
+    if (strcasecmp("Percentile", child->key) == 0)
+      status = latency_config_add_percentile(conf, child, plugin);
+    else if (strcasecmp("Bucket", child->key) == 0)
+      status = latency_config_add_bucket(conf, child, plugin);
+    else
+      WARNING("%s plugin: \"%s\" is not a valid option within a \"%s\" block.",
+              plugin, child->key, ci->key);
+
+    if (status != 0)
+      return status;
+  }
+
+  if ((status == 0) && (conf->percentile_num == 0) &&
+      (conf->buckets_num == 0)) {
+    ERROR("%s plugin: The \"%s\" block must contain at least one "
+          "\"Percentile\" or \"Bucket\" option.",
+          plugin, ci->key);
+    return EINVAL;
+  }
+
+  return 0;
+}
 
 int latency_config_copy(latency_config_t *dst, const latency_config_t src) {
   *dst = (latency_config_t){
-      .rates = NULL,
-      .rates_num = src.rates_num,
-      .rates_type = NULL,
-      .percentile = NULL,
-      .percentile_num = src.percentile_num,
-      .percentile_type = NULL,
+      .percentile_num = src.percentile_num, .buckets_num = src.buckets_num,
   };
 
-  /* Copy percentiles configuration */
-  dst->percentile_num = src.percentile_num;
-  dst->percentile = calloc(src.percentile_num, sizeof(*dst->percentile));
-  if (dst->percentile == NULL)
-    return (-1);
-
-  memcpy(dst->percentile, src.percentile,
-         (sizeof(*dst->percentile) * (src.percentile_num)));
-
-  if (src.percentile_type != NULL) {
-    dst->percentile_type = strdup(src.percentile_type);
-    if (dst->percentile_type == NULL) {
-      latency_config_free(*dst);
-      return (-1);
-    }
-  }
+  dst->percentile = calloc(dst->percentile_num, sizeof(*dst->percentile));
+  dst->buckets = calloc(dst->buckets_num, sizeof(*dst->buckets));
 
-  /* Copy rates configuration */
-  dst->rates_num = src.rates_num;
-  dst->rates = calloc(src.rates_num * 2, sizeof(*dst->rates));
-  if (dst->rates == NULL) {
+  if ((dst->percentile == NULL) || (dst->buckets == NULL)) {
     latency_config_free(*dst);
-    return (-1);
+    return ENOMEM;
   }
 
-  memcpy(dst->rates, src.rates, (sizeof(*dst->rates) * (src.rates_num) * 2));
+  memmove(dst->percentile, src.percentile,
+          dst->percentile_num * sizeof(*dst->percentile));
+  memmove(dst->buckets, src.buckets, dst->buckets_num * sizeof(*dst->buckets));
 
-  if (src.rates_type != NULL) {
-    dst->rates_type = strdup(src.rates_type);
-    if (dst->rates_type == NULL) {
-      latency_config_free(*dst);
-      return (-1);
-    }
-  }
-
-  return (0);
+  return 0;
 } /* int latency_config_copy */
 
-void latency_config_free(latency_config_t lc) {
-  sfree(lc.rates);
-  sfree(lc.rates_type);
-  sfree(lc.percentile);
-  sfree(lc.percentile_type);
+void latency_config_free(latency_config_t conf) {
+  sfree(conf.percentile);
+  sfree(conf.buckets);
 } /* void latency_config_free */
diff --git a/src/utils_latency_config.h b/src/utils_latency_config.h
index a5354960..9a7a11af 100644
--- a/src/utils_latency_config.h
+++ b/src/utils_latency_config.h
@@ -29,31 +29,34 @@
 #define UTILS_LATENCY_CONFIG_H 1
 
 #include "collectd.h"
+
+#include "liboconfig/oconfig.h"
 #include "utils_time.h"
 
-struct latency_config_s {
+typedef struct {
+  cdtime_t lower_bound;
+  cdtime_t upper_bound;
+} latency_bucket_t;
+
+typedef struct {
   double *percentile;
   size_t percentile_num;
-  char *percentile_type;
-  cdtime_t *rates;
-  size_t rates_num;
-  char *rates_type;
+
+  latency_bucket_t *buckets;
+  size_t buckets_num;
+
+  /*
   _Bool lower;
   _Bool upper;
-  //_Bool sum;
   _Bool avg;
-  //_Bool count;
-};
-typedef struct latency_config_s latency_config_t;
-
-int latency_config_add_percentile(const char *plugin, latency_config_t *cl,
-                                  oconfig_item_t *ci);
+  */
+} latency_config_t;
 
-int latency_config_add_rate(const char *plugin, latency_config_t *cl,
-                            oconfig_item_t *ci);
+int latency_config(latency_config_t *conf, oconfig_item_t *ci,
+                   char const *plugin);
 
 int latency_config_copy(latency_config_t *dst, const latency_config_t src);
 
-void latency_config_free(latency_config_t lc);
+void latency_config_free(latency_config_t conf);
 
 #endif /* UTILS_LATENCY_CONFIG_H */