feat(metrics) add 'sum' field to histograms

Kong · Sep 27, 2024 · ad1e5c7 · ad1e5c7
1 parent 35e8eb7
commit ad1e5c7
Show file tree

Hide file tree

Showing 9 changed files with 18 additions and 11 deletions.
diff --git a/docs/METRICS.md b/docs/METRICS.md
@@ -98,14 +98,14 @@ consolidated and returned as a single metric value. This storage strategy allows
 metric updates to be performed without the aid of shared memory read/write locks
 at the cost of 16 bytes per worker.
 
-Histogram values also have a baseline size of 8 bytes + 16 bytes per worker
-process. However, histograms also need extra space per worker for bins storage.
+Histogram values have a baseline size of 8 bytes + 24 bytes per worker process.
+However, histograms also need extra space per worker for bins storage.
 Bins storage costs 4 bytes + 8 bytes per bin. Thus, a 5-bin histogram takes: 8
-bytes + (16 + 4 + 5*8), so 60 bytes per worker.
+bytes + (24 + 4 + 5*8), so 68 bytes per worker.
 
 As such, in a 4-workers setup, a counter or gauge whose name is 64 chars long
 occupies 168 bytes, and a 5-bin histogram with the same name length occupies 408
-bytes. A 18-bin histogram with the same length name occupies 824 bytes.
+bytes. A 18-bin histogram with the same length name occupies 856 bytes.
 
 [Back to TOC](#table-of-contents)
 

diff --git a/lib/resty/wasmx/shm.lua b/lib/resty/wasmx/shm.lua
@@ -70,6 +70,7 @@ ffi.cdef [[
 
     typedef struct {
         uint8_t                      n_bins;
+        uint64_t                     sum;
         ngx_wa_metrics_bin_t         bins[];
     } ngx_wa_metrics_histogram_t;
 
@@ -435,7 +436,7 @@ local function parse_cmetric(cmetric)
     if cmetric.metric_type == _types.ffi_metric.HISTOGRAM then
         local hbuf = cmetric.slots[0].histogram
         local ch = ffi_cast("ngx_wa_metrics_histogram_t *", hbuf)
-        local h = { type = "histogram", value = {} }
+        local h = { type = "histogram", value = {}, sum = tonumber(ch.sum) }
 
         for i = 0, ch.n_bins do
             local cb = ch.bins[i]

diff --git a/src/common/metrics/ngx_wa_histogram.c b/src/common/metrics/ngx_wa_histogram.c
@@ -150,6 +150,8 @@ histogram_log(ngx_wa_metrics_t *metrics, ngx_wa_metric_t *m, uint32_t mid)
         }
     }
 
+    p = ngx_sprintf(p, " SUM: %uD;", h->sum);
+
     ngx_log_debug3(NGX_LOG_DEBUG_WASM, metrics->shm->log, 0,
                    "histogram \"%uD\": %*s",
                    mid, p - s_buf - 1, s_buf + 1);
@@ -201,6 +203,8 @@ ngx_wa_metrics_histogram_record(ngx_wa_metrics_t *metrics, ngx_wa_metric_t *m,
     ngx_wa_metrics_histogram_t  *h;
 
     h = m->slots[slot].histogram;
+    h->sum += n;
+
     b = histogram_bin(metrics, h, n, &m->slots[slot].histogram);
     b->count += 1;
 
@@ -222,6 +226,7 @@ ngx_wa_metrics_histogram_get(ngx_wa_metrics_t *metrics, ngx_wa_metric_t *m,
 
     for (i = 0; i < slots; i++) {
         h = m->slots[i].histogram;
+        out->sum += h->sum;
 
         for (j = 0; j < h->n_bins; j++) {
             b = &h->bins[j];

diff --git a/src/common/metrics/ngx_wa_metrics.h b/src/common/metrics/ngx_wa_metrics.h
@@ -43,6 +43,7 @@ typedef struct {
 
 typedef struct {
     uint8_t                      n_bins;
+    uint64_t                     sum;
     ngx_wa_metrics_bin_t         bins[];
 } ngx_wa_metrics_histogram_t;
 

diff --git a/t/03-proxy_wasm/hfuncs/metrics/020-proxy_record_metric.t b/t/03-proxy_wasm/hfuncs/metrics/020-proxy_record_metric.t
@@ -81,7 +81,7 @@ qq{
 --- error_log eval
 [
     "growing histogram",
-    qr/histogram "\d+": 1: 1; 2: 1; 4: 1; 8: 1; 16: 1; 32: 1; 64: 1; 128: 1; 256: 1; 512: 1; 1024: 1; 2048: 1; 4096: 1; 8192: 1; 16384: 1; 32768: 1; 65536: 1; 4294967295: 1;/
+    qr/histogram "\d+": 1: 1; 2: 1; 4: 1; 8: 1; 16: 1; 32: 1; 64: 1; 128: 1; 256: 1; 512: 1; 1024: 1; 2048: 1; 4096: 1; 8192: 1; 16384: 1; 32768: 1; 65536: 1; 4294967295: 1; SUM: 262143/
 ]
 --- no_error_log
 [error]

diff --git a/t/03-proxy_wasm/hfuncs/metrics/021-proxy_record_metric_edge_cases.t b/t/03-proxy_wasm/hfuncs/metrics/021-proxy_record_metric_edge_cases.t
@@ -118,7 +118,7 @@ qq{
 --- error_log eval
 [
     "cannot expand histogram",
-    qr/histogram \"\d+\": 1: 1; 2: 1; 4: 1; 8: 1; 4294967295: 1/,
+    qr/histogram \"\d+\": 1: 1; 2: 1; 4: 1; 8: 1; 4294967295: 1; SUM: 31/,
 ]
 --- no_error_log
 [error]

diff --git a/t/04-openresty/ffi/shm/022-metrics_record.t b/t/04-openresty/ffi/shm/022-metrics_record.t
@@ -32,7 +32,7 @@ __DATA__
     }
 --- response_body
 g1: {type="gauge",value=10}
-h1: {type="histogram",value={{count=1,ub=128},{count=0,ub=4294967295}}}
+h1: {sum=100,type="histogram",value={{count=1,ub=128},{count=0,ub=4294967295}}}
 --- no_error_log
 [error]
 [crit]

diff --git a/t/04-openresty/ffi/shm/023-metrics_get.t b/t/04-openresty/ffi/shm/023-metrics_get.t
@@ -36,7 +36,7 @@ __DATA__
 --- response_body
 c1: {type="counter",value=1}
 g1: {type="gauge",value=10}
-h1: {type="histogram",value={{count=1,ub=128},{count=0,ub=4294967295}}}
+h1: {sum=100,type="histogram",value={{count=1,ub=128},{count=0,ub=4294967295}}}
 --- no_error_log
 [error]
 [crit]

diff --git a/t/04-openresty/ffi/shm/024-metrics_get_by_name.t b/t/04-openresty/ffi/shm/024-metrics_get_by_name.t
@@ -37,7 +37,7 @@ prefix: lua.*
 --- response_body
 c1: {type="counter",value=1}
 g1: {type="gauge",value=10}
-h1: {type="histogram",value={{count=1,ub=128},{count=0,ub=4294967295}}}
+h1: {sum=100,type="histogram",value={{count=1,ub=128},{count=0,ub=4294967295}}}
 --- no_error_log
 [error]
 [crit]
@@ -78,7 +78,7 @@ prefix: pw.hostcalls.*
 --- response_body
 c1: {type="counter",value=13}
 g1: {type="gauge",value=1}
-h1: {type="histogram",value={{count=1,ub=128},{count=0,ub=4294967295}}}
+h1: {sum=100,type="histogram",value={{count=1,ub=128},{count=0,ub=4294967295}}}
 --- no_error_log
 [error]
 [crit]