feat(metrics) implement user-defined histogram bins

This feature is only available through the FFI.

If a user provides a list of numbers when defining a histogram, those
numbers will be used as the upper-bounds of the histogram's bins.

For instance, the following code:

    local shm = require "resty.wasmx.shm"
    shm.metrics:define("h", shm.metrics.HISTOGRAM, { bins = { 1, 3, 5 } })

Creates a histogram with bins `[0, 1], (1, 3], (3, 5], (5, Inf+]`.

docs/METRICS.md

@@ -8,7 +8,9 @@ memory necessary for your use-case.
 
 - [Types of Metrics](#types-of-metrics)
 - [Name Prefixing](#name-prefixing)
-- [Histogram Binning Strategy](#histogram-binning-strategy)
+- [Histogram Binning Strategies](#histogram-binning-strategies)
+    - [Logarithmic Binning](#logarithmic-binning)
+    - [Custom Binning](#custom-binning)
 - [Histogram Update and Expansion](#histogram-update-and-expansion)
 - [Memory Consumption](#memory-consumption)
 - [Shared Memory Allocation](#shared-memory-allocation)
@@ -46,41 +48,38 @@ increased in some cases.
 
 [Back to TOC](#table-of-contents)
 
-## Histogram Binning Strategy
+## Histogram Binning Strategies
 
-The above example demonstrates a histogram with ranges (or bins) whose
-upper-bound grows in powers of 2, i.e. `2^0`, `2^1`, and `2^2`. This is usually
-called "logarithmic binning" and is how histograms bins are by default
-represented in ngx_wasm_module.
+### Logarithmic Binning
 
-This binning strategy implies that when a value `v` is recorded, it is matched
-with the smallest power of two that is bigger than `v`. This value is the
-*upper-bound* of the bin associated with `v`. If the histogram contains or can
-contain such a bin, that bin's counter is incremented. If not, the bin with the
-next smallest upper-bound bigger than `v` has its counter incremented instead.
+By default, histograms use a logarithmic-binning strategy.
 
-Histograms can also be created with a fixed set of bins. These histograms are
-similar to the logarithmic-binning ones, except for their number of bins, and
-respective upper-bounds, being user-defined instead of given by powers of 2, and
-for their non-expandable nature.
+As an example of logarithmic-binning, take the histogram with ranges (i.e.
+"bins") `[0, 1] (1, 2] (2, 4] (4, Inf]`: it has upper-bounds growing in powers
+of 2: `2^0`, `2^1`, and `2^2`. In logarithmic-binning, a value `v` being
+recorded is matched with the smallest power of two that is bigger than `v`. This
+value is the *upper-bound* of the bin associated with `v`. If the histogram
+contains or can contain such a bin, then its counter is incremented. If not, the
+bin with the next smallest upper-bound bigger than `v` has its counter
+incremented instead.
 
-[Back to TOC](#table-of-contents)
+In ngx_wasm_module, logarithmic-binning histograms are created with one
+initialized bin with upper-bound `2^32`. This bin's counter is incremented if it
+is the only bin whose upper-bound is bigger than the recorded value.
 
-## Histogram Update and Expansion
+When a value `v` is recorded and its bin does not yet exist, a new bin with the
+upper-bound associated with `v` is initialized and its counter is incremented.
 
-Histograms without a user-defined set of bins are created with 5 bins: 1
-initialized and 4 uninitialized.
+A logarithmic-binning histogram can contain up to 18 initialized bins.
 
-The bin initialized upon histogram creation has upper-bound `2^32` and its
-counter is incremented if it is the only bin whose upper-bound is bigger than
-the recorded value.
+[Back to TOC](#table-of-contents)
 
-If a value `v` is recorded and its bin is not part of the initialized bins, a
-new bin with the upper-bound associated with `v` is initialized, and its counter
-is incremented.
+### Custom Binning
 
-If the histogram is out of uninitialized bins, it can be expanded up to 18
-bins so as to accommodate the additional bins for other ranges of `v`.
+Histograms can also be created with a fixed set of bins with user-defined
+upper-bounds. These histograms store values exactly like the logarithmic-binning
+ones, except the number of bins and their upper-bounds are user-defined and
+pre-initialized. A custom-binning histogram cannot be expanded with new bins.
 
 [Back to TOC](#table-of-contents)
 

lib/resty/wasmx/shm.lua

@@ -192,7 +192,8 @@ local function key_iterator(ctx)
         ctx.ccur_index[0] = 0
 
         local rc = C.ngx_wa_ffi_shm_iterate_keys(ctx.shm, ctx.page_size,
-                                                 ctx.clast_index, ctx.ccur_index, ctx.ckeys)
+                                                 ctx.clast_index, ctx.ccur_index,
+                                                 ctx.ckeys)
 
         if rc == FFI_ABORT then
             -- users must manage locking themselves (e.g. break condition in the for loop)
@@ -351,28 +352,6 @@ local function shm_kv_set(zone, key, value, cas)
 end
 
 
-local function validate_bins(bins)
-    if type(bins) ~= "table" then
-        error("opts.bins must be a table", 3)
-    end
-
-    if #bins > HISTOGRAM_MAX_BINS then
-        local err = "opts.bins must have up to %d numbers"
-        error(str_fmt(err, HISTOGRAM_MAX_BINS), 3)
-    end
-
-    local previous = 0
-
-    for _, n in ipairs(bins) do
-        if type(n) ~= "number" or n < 0 or n % 1 > 0 or n < previous then
-            error("opts.bins must be an ascending list of positive integers", 3)
-        end
-
-        previous = n
-    end
-end
-
-
 local function metrics_define(zone, name, metric_type, opts)
     if type(name) ~= "string" or name == "" then
         error("name must be a non-empty string", 2)
@@ -383,7 +362,6 @@ local function metrics_define(zone, name, metric_type, opts)
                     " resty.wasmx.shm.metrics.COUNTER," ..
                     " resty.wasmx.shm.metrics.GAUGE, or" ..
                     " resty.wasmx.shm.metrics.HISTOGRAM"
-
         error(err, 2)
     end
 
@@ -395,20 +373,43 @@ local function metrics_define(zone, name, metric_type, opts)
             error("opts must be a table", 2)
         end
 
-        if opts.bins ~= nil and metric_type == _types.ffi_metric.HISTOGRAM then
-            validate_bins(opts.bins)
+        if metric_type == _types.ffi_metric.HISTOGRAM
+           and opts.bins ~= nil
+        then
+            if type(opts.bins) ~= "table" then
+                error("opts.bins must be a table", 2)
+            end
+
+            if #opts.bins > HISTOGRAM_MAX_BINS then
+                local err = "opts.bins cannot have more than %d numbers"
+                error(str_fmt(err, HISTOGRAM_MAX_BINS), 2)
+            end
+
+            local previous = 0
+
+            for _, n in ipairs(opts.bins) do
+                if type(n) ~= "number"
+                   or n < 0 or n % 1 > 0 or n <= previous
+                then
+                    error("opts.bins must be an ascending list of " ..
+                          "positive integers", 2)
+                end
+
+                previous = n
+            end
 
-            cbins = ffi_new("uint32_t[?]", #opts.bins, opts.bins)
             n_bins = #opts.bins
+            cbins = ffi_new("uint32_t[?]", n_bins, opts.bins)
         end
     end
 
     name = "lua." .. name
 
     local cname = ffi_new("ngx_str_t", { data = name, len = #name })
-    local m_id = ffi_new("uint32_t [1]")
+    local m_id = ffi_new("uint32_t[1]")
 
-    local rc = C.ngx_wa_ffi_shm_metric_define(cname, metric_type, cbins, n_bins, m_id)
+    local rc = C.ngx_wa_ffi_shm_metric_define(cname, metric_type,
+                                              cbins, n_bins, m_id)
     if rc == FFI_ERROR then
         return nil, "no memory"
     end

src/common/debug/ngx_wasm_debug_module.c

@@ -27,7 +27,7 @@ ngx_wasm_debug_init(ngx_cycle_t *cycle)
     ngx_str_t                    metric_name;
     ngx_wa_metric_t             *m;
     ngx_wa_metrics_histogram_t  *h, *h2;
-    uint32_t                     bins[NGX_WA_METRICS_BINS_MAX + 1];
+    uint32_t                     bins[NGX_WA_METRICS_HISTOGRAM_BINS_MAX + 1];
     u_char                       buf[long_metric_name_len];
     u_char                       m_buf[NGX_WA_METRICS_ONE_SLOT_SIZE];
     u_char                       h_buf[NGX_WA_METRICS_HISTOGRAM_MAX_SIZE];
@@ -85,7 +85,7 @@ ngx_wasm_debug_init(ngx_cycle_t *cycle)
         ngx_wa_metrics_define(ngx_wasmx_metrics(cycle),
                               ngx_wa_metric_type_name(NGX_WA_METRIC_HISTOGRAM),
                               NGX_WA_METRIC_HISTOGRAM,
-                              bins, NGX_WA_METRICS_BINS_MAX + 1,
+                              bins, NGX_WA_METRICS_HISTOGRAM_BINS_MAX + 1,
                               &mid) == NGX_ABORT
     );
 
@@ -103,7 +103,7 @@ ngx_wasm_debug_init(ngx_cycle_t *cycle)
 
     m = (ngx_wa_metric_t *) m_buf;
     h = (ngx_wa_metrics_histogram_t *) h_buf;
-    h->n_bins = NGX_WA_METRICS_BINS_MAX;
+    h->n_bins = NGX_WA_METRICS_HISTOGRAM_BINS_MAX;
     h->h_type = 10;
     h->sum = 1;
     ngx_wa_metrics_histogram_set_buffer(m, h_buf, sizeof(h_buf));

src/common/lua/ngx_wasm_lua_ffi.h

@@ -97,7 +97,7 @@ ngx_wa_ffi_shm_metrics_histogram_max_size()
 ngx_int_t
 ngx_wa_ffi_shm_metrics_histogram_max_bins()
 {
-    return NGX_WA_METRICS_BINS_MAX;
+    return NGX_WA_METRICS_HISTOGRAM_BINS_MAX;
 }
 
 

src/common/metrics/ngx_wa_histogram.c

@@ -35,15 +35,15 @@ histogram_grow(ngx_wa_metrics_t *metrics, ngx_wa_metrics_histogram_t *h,
     ngx_uint_t                   n;
     ngx_wa_metrics_histogram_t  *new_h = NULL;
 
-    if (h->n_bins == NGX_WA_METRICS_BINS_MAX) {
+    if (h->n_bins == NGX_WA_METRICS_HISTOGRAM_BINS_MAX) {
         return NGX_ERROR;
     }
 
     ngx_log_debug0(NGX_LOG_DEBUG_WASM, metrics->shm->log, 0,
                    "growing histogram");
 
-    n = ngx_min(NGX_WA_METRICS_BINS_INCREMENT,
-                NGX_WA_METRICS_BINS_MAX - h->n_bins);
+    n = ngx_min(NGX_WA_METRICS_HISTOGRAM_BINS_INCREMENT,
+                NGX_WA_METRICS_HISTOGRAM_BINS_MAX - h->n_bins);
     old_size = sizeof(ngx_wa_metrics_histogram_t)
                + sizeof(ngx_wa_metrics_bin_t) * h->n_bins;
     size = old_size + sizeof(ngx_wa_metrics_bin_t) * n;
@@ -79,7 +79,7 @@ histogram_grow(ngx_wa_metrics_t *metrics, ngx_wa_metrics_histogram_t *h,
 static ngx_wa_metrics_bin_t *
 histogram_custom_bin(ngx_wa_metrics_histogram_t *h, ngx_uint_t n)
 {
-    size_t                 i = 0;
+    size_t                 i;
     ngx_wa_metrics_bin_t  *b = NULL;
 
     for (i = 0; i < (size_t) h->n_bins; i++) {
@@ -156,7 +156,7 @@ histogram_log(ngx_wa_metrics_t *metrics, ngx_wa_metric_t *m, uint32_t mid)
 
     p = s_buf;
     h = (ngx_wa_metrics_histogram_t *) h_buf;
-    h->n_bins = NGX_WA_METRICS_BINS_MAX;
+    h->n_bins = NGX_WA_METRICS_HISTOGRAM_BINS_MAX;
     h->bins[0].upper_bound = NGX_MAX_UINT32_VALUE;
 
     ngx_wa_metrics_histogram_get(metrics, m, metrics->workers, h);
@@ -183,17 +183,17 @@ ngx_int_t
 ngx_wa_metrics_histogram_add_locked(ngx_wa_metrics_t *metrics, uint32_t *bins,
     uint16_t cn_bins, ngx_wa_metric_t *m)
 {
-    size_t                        i, j = 0;
-    uint16_t                      n_bins = NGX_WA_METRICS_BINS_INIT;
+    size_t                        i, j;
+    uint16_t                      n_bins = NGX_WA_METRICS_HISTOGRAM_BINS_INIT;
     ngx_wa_histogram_type_e       h_type = NGX_WA_HISTOGRAM_LOG2;
     ngx_wa_metrics_histogram_t  **h;
 
     if (bins) {
-        if (cn_bins > NGX_WA_METRICS_BINS_MAX) {
+        if (cn_bins > NGX_WA_METRICS_HISTOGRAM_BINS_MAX) {
             return NGX_ABORT;
         }
 
-        /* user-defined bins + an NGX_MAX_UINT32_VALUE upper-bounded bin */
+        /* user-defined bins + a bin with NGX_MAX_UINT32_VALUE upper-bound */
         n_bins = cn_bins + 1;
         h_type = NGX_WA_HISTOGRAM_CUSTOM;
     }

src/common/metrics/ngx_wa_metrics.c

@@ -346,7 +346,9 @@ ngx_wa_metrics_define(ngx_wa_metrics_t *metrics, ngx_str_t *name,
                        ngx_wa_metric_type_name(type), name, mid);
     }
 
-    ngx_wa_assert(rc == NGX_OK || rc == NGX_ERROR || rc == NGX_ABORT);
+    ngx_wa_assert(rc == NGX_OK
+                  || rc == NGX_ERROR
+                  || rc == NGX_ABORT);
 
     return rc;
 }

src/common/metrics/ngx_wa_metrics.h

@@ -9,15 +9,17 @@
 #define ngx_wa_metrics_gauge(m) m->slots[0].gauge.value
 
 
-#define NGX_WA_METRICS_BINS_INIT             5
-#define NGX_WA_METRICS_BINS_MAX              18
-#define NGX_WA_METRICS_BINS_INCREMENT        4
-#define NGX_WA_METRICS_ONE_SLOT_SIZE         sizeof(ngx_wa_metric_t)         \
-                                             + sizeof(ngx_wa_metric_val_t)
+#define NGX_WA_METRICS_HISTOGRAM_BINS_INIT             5
+#define NGX_WA_METRICS_HISTOGRAM_BINS_MAX              18
+#define NGX_WA_METRICS_HISTOGRAM_BINS_INCREMENT        4
 #define NGX_WA_METRICS_HISTOGRAM_MAX_SIZE                                    \
     sizeof(ngx_wa_metrics_histogram_t)                                       \
     + sizeof(ngx_wa_metrics_bin_t)                                           \
-    * NGX_WA_METRICS_BINS_MAX
+    * NGX_WA_METRICS_HISTOGRAM_BINS_MAX
+
+#define NGX_WA_METRICS_ONE_SLOT_SIZE                                         \
+    sizeof(ngx_wa_metric_t)                                                  \
+    + sizeof(ngx_wa_metric_val_t)
 
 
 typedef struct ngx_wa_metrics_s  ngx_wa_metrics_t;

t/04-openresty/ffi/shm/020-metrics_define.t

@@ -129,15 +129,15 @@ ok
             _, perr = pcall(shm.metrics.define, {}, "ch1", shm.metrics.HISTOGRAM, { bins = bins })
             ngx.say(perr)
 
-            local bins = { 1, 3, -5 }
+            bins = { 1, 3, -5 }
             _, perr = pcall(shm.metrics.define, {}, "ch1", shm.metrics.HISTOGRAM, { bins = bins })
             ngx.say(perr)
 
-            local bins = { 1, 3, 5.5 }
+            bins = { 1, 3, 5.5 }
             _, perr = pcall(shm.metrics.define, {}, "ch1", shm.metrics.HISTOGRAM, { bins = bins })
             ngx.say(perr)
 
-            local bins = { 1, 5, 3 }
+            bins = { 1, 5, 3 }
             _, perr = pcall(shm.metrics.define, {}, "ch1", shm.metrics.HISTOGRAM, { bins = bins })
             ngx.say(perr)
 
@@ -150,7 +150,7 @@ name must be a non-empty string
 name must be a non-empty string
 metric_type must be one of resty.wasmx.shm.metrics.COUNTER, resty.wasmx.shm.metrics.GAUGE, or resty.wasmx.shm.metrics.HISTOGRAM
 opts.bins must be a table
-opts.bins must have up to 18 numbers
+opts.bins cannot have more than 18 numbers
 opts.bins must be an ascending list of positive integers
 opts.bins must be an ascending list of positive integers
 opts.bins must be an ascending list of positive integers

t/04-openresty/ffi/shm/023-metrics_record_custom_histogram.t

@@ -6,8 +6,8 @@ use t::TestWasmX::Lua;
 
 skip_no_openresty();
 
-workers(2);
 master_on();
+workers(2);
 
 plan_tests(4);
 run_tests();
@@ -16,8 +16,8 @@ __DATA__
 
 === TEST 1: shm_metrics - record() custom histogram, multiple workers
 Metric creation and update by multiple workers is covered in Proxy-Wasm tests;
-custom histogram creation and update by multiple workers is covered here because
-custom histograms can't be created from Proxy-Wasm filters yet.
+custom histogram creation and update by multiple workers is covered here
+because custom histograms cannot be created from Proxy-Wasm filters yet.
 
 --- valgrind
 --- metrics: 16k