Skip to content

Commit

Permalink
feat(metrics) histogram support in ngx_wa_metrics_get
Browse files Browse the repository at this point in the history 
Previously `ngx_wa_metrics_get` would refuse to return a histogram and
return an NGX_ABORT instead. This was due to Proxy-Wasm not supporting
histogram retrieval.

However, the upcoming FFI interface to shms will allow histograms to be
retrieved.

Also merges `ngx_wa_metrics_histogram.h` into `ngx_wa_metrics.h` as
histogram declaration was already in `ngx_wa_metrics.h` and removing
`ngx_wa_metrics_histogram.h` simplifies usage of the lib.
  • Loading branch information
@casimiro @thibaultcha
casimiro authored and thibaultcha committed Sep 19, 2024
1 parent ea2690f commit 8c6a796
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 68 deletions.
11 changes: 5 additions & 6 deletions config
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,10 @@ NGX_WASMX_DEPS="\
$ngx_addon_dir/src/common/shm/ngx_wa_shm.h \
$ngx_addon_dir/src/common/shm/ngx_wa_shm_kv.h \
$ngx_addon_dir/src/common/shm/ngx_wa_shm_queue.h \
$ngx_addon_dir/src/common/metrics/ngx_wa_metrics.h \
$ngx_addon_dir/src/common/proxy_wasm/ngx_proxy_wasm.h \
$ngx_addon_dir/src/common/proxy_wasm/ngx_proxy_wasm_maps.h \
$ngx_addon_dir/src/common/proxy_wasm/ngx_proxy_wasm_properties.h \
$ngx_addon_dir/src/common/metrics/ngx_wa_metrics.h \
$ngx_addon_dir/src/common/metrics/ngx_wa_histogram.h"
$ngx_addon_dir/src/common/proxy_wasm/ngx_proxy_wasm_properties.h"

NGX_WASMX_SRCS="\
$ngx_addon_dir/src/ngx_wasmx.c \
Expand All @@ -154,13 +153,13 @@ NGX_WASMX_SRCS="\
$ngx_addon_dir/src/common/shm/ngx_wa_shm.c \
$ngx_addon_dir/src/common/shm/ngx_wa_shm_kv.c \
$ngx_addon_dir/src/common/shm/ngx_wa_shm_queue.c \
$ngx_addon_dir/src/common/metrics/ngx_wa_metrics.c \
$ngx_addon_dir/src/common/metrics/ngx_wa_histogram.c \
$ngx_addon_dir/src/common/proxy_wasm/ngx_proxy_wasm.c \
$ngx_addon_dir/src/common/proxy_wasm/ngx_proxy_wasm_host.c \
$ngx_addon_dir/src/common/proxy_wasm/ngx_proxy_wasm_maps.c \
$ngx_addon_dir/src/common/proxy_wasm/ngx_proxy_wasm_properties.c \
$ngx_addon_dir/src/common/proxy_wasm/ngx_proxy_wasm_util.c \
$ngx_addon_dir/src/common/metrics/ngx_wa_metrics.c \
$ngx_addon_dir/src/common/metrics/ngx_wa_histogram.c"
$ngx_addon_dir/src/common/proxy_wasm/ngx_proxy_wasm_util.c"

# wasm

Expand Down
45 changes: 21 additions & 24 deletions src/common/metrics/ngx_wa_histogram.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,7 @@
#include "ddebug.h"

#include <ngx_wasm.h>
#include <ngx_wa_histogram.h>


#define NGX_WA_BINS_INIT 5
#define NGX_WA_BINS_MAX 18
#define NGX_WA_BINS_INCREMENT 4
#include <ngx_wa_metrics.h>


static uint32_t
Expand Down Expand Up @@ -40,14 +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_BINS_MAX) {
if (h->n_bins == NGX_WA_METRICS_BINS_MAX) {
return NGX_ERROR;
}

ngx_log_debug0(NGX_LOG_DEBUG_WASM, metrics->shm->log, 0,
"growing histogram");

n = ngx_min(NGX_WA_BINS_INCREMENT, NGX_WA_BINS_MAX - h->n_bins);
n = ngx_min(NGX_WA_METRICS_BINS_INCREMENT,
NGX_WA_METRICS_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;
Expand Down Expand Up @@ -96,7 +92,8 @@ histogram_bin(ngx_wa_metrics_t *metrics, ngx_wa_metrics_histogram_t *h,
return b;
}

if (b->upper_bound == 0) {
if (b->upper_bound == NGX_MAX_UINT32_VALUE) {
i++;
break;
}
}
Expand Down Expand Up @@ -128,29 +125,29 @@ histogram_bin(ngx_wa_metrics_t *metrics, ngx_wa_metrics_histogram_t *h,
static void
histogram_log(ngx_wa_metrics_t *metrics, ngx_wa_metric_t *m, uint32_t mid)
{
size_t i, size = sizeof(ngx_wa_metrics_histogram_t)
+ sizeof(ngx_wa_metrics_bin_t)
* NGX_WA_BINS_MAX;
size_t i;
u_char *p;
ngx_wa_metrics_bin_t *b;
ngx_wa_metrics_histogram_t *h;
u_char *p, buf[size], s_buf[NGX_MAX_ERROR_STR];
u_char h_buf[NGX_WA_METRICS_MAX_HISTOGRAM_SIZE];
u_char s_buf[NGX_MAX_ERROR_STR];

ngx_memzero(buf, size);
ngx_memzero(h_buf, sizeof(h_buf));

p = s_buf;
h = (ngx_wa_metrics_histogram_t *) buf;
h->n_bins = NGX_WA_BINS_MAX;
h = (ngx_wa_metrics_histogram_t *) h_buf;
h->n_bins = NGX_WA_METRICS_BINS_MAX;
h->bins[0].upper_bound = NGX_MAX_UINT32_VALUE;

ngx_wa_metrics_histogram_get(metrics, m, metrics->workers, h);

for (i = 0; i < h->n_bins; i++) {
b = &h->bins[i];
if (b->upper_bound == 0) {
p = ngx_sprintf(p, " %uD: %uD;", b->upper_bound, b->count);

if (b->upper_bound == NGX_MAX_UINT32_VALUE) {
break;
}

p = ngx_sprintf(p, " %uD: %uD;", b->upper_bound, b->count);
}

ngx_log_debug3(NGX_LOG_DEBUG_WASM, metrics->shm->log, 0,
Expand All @@ -165,7 +162,7 @@ ngx_wa_metrics_histogram_add_locked(ngx_wa_metrics_t *metrics,
ngx_wa_metric_t *m)
{
size_t i;
static uint16_t n_bins = NGX_WA_BINS_INIT;
static uint16_t n_bins = NGX_WA_METRICS_BINS_INIT;
ngx_wa_metrics_histogram_t **h;

for (i = 0; i < metrics->workers; i++) {
Expand Down Expand Up @@ -228,12 +225,12 @@ ngx_wa_metrics_histogram_get(ngx_wa_metrics_t *metrics, ngx_wa_metric_t *m,

for (j = 0; j < h->n_bins; j++) {
b = &h->bins[j];
if (b->upper_bound == 0) {
break;
}

out_b = histogram_bin(metrics, out, b->upper_bound, NULL);
out_b->count += b->count;

if (b->upper_bound == NGX_MAX_UINT32_VALUE) {
break;
}
}
}
}
16 changes: 0 additions & 16 deletions src/common/metrics/ngx_wa_histogram.h
View file

This file was deleted.

20 changes: 13 additions & 7 deletions src/common/metrics/ngx_wa_metrics.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

#include <ngx_wasm.h>
#include <ngx_wa_metrics.h>
#include <ngx_wa_histogram.h>


ngx_str_t *
Expand Down Expand Up @@ -478,7 +477,8 @@ ngx_wa_metrics_record(ngx_wa_metrics_t *metrics, uint32_t mid, ngx_int_t n)
* NGX_DECLINED: not found
*/
ngx_int_t
ngx_wa_metrics_get(ngx_wa_metrics_t *metrics, uint32_t mid, ngx_uint_t *out)
ngx_wa_metrics_get(ngx_wa_metrics_t *metrics, uint32_t mid,
ngx_wa_metric_t *out)
{
uint32_t cas;
ngx_int_t rc;
Expand All @@ -491,26 +491,32 @@ ngx_wa_metrics_get(ngx_wa_metrics_t *metrics, uint32_t mid, ngx_uint_t *out)
}

m = (ngx_wa_metric_t *) n->data;
out->type = m->type;

switch (m->type) {
case NGX_WA_METRIC_COUNTER:
*out = get_counter(m, metrics->workers);
out->slots[0].counter = get_counter(m, metrics->workers);
break;

case NGX_WA_METRIC_GAUGE:
*out = get_gauge(m, metrics->workers);
out->slots[0].gauge.value = get_gauge(m, metrics->workers);
break;

case NGX_WA_METRIC_HISTOGRAM:
ngx_wa_metrics_histogram_get(metrics, m, metrics->workers,
out->slots[0].histogram);
break;

default:
ngx_wa_assert(m->type == NGX_WA_METRIC_HISTOGRAM);
rc = NGX_ABORT;
ngx_wa_assert(0);
rc = NGX_ERROR;
break;
}

done:

ngx_wa_assert(rc == NGX_OK
|| rc == NGX_ABORT
|| rc == NGX_ERROR
|| rc == NGX_DECLINED);

return rc;
Expand Down
35 changes: 34 additions & 1 deletion src/common/metrics/ngx_wa_metrics.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,20 @@
#include <ngx_wa_shm_kv.h>


#define ngx_wa_metrics_counter(m) m->slots[0].counter
#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_MAX_HISTOGRAM_SIZE sizeof(ngx_wa_metrics_histogram_t)\
+ sizeof(ngx_wa_metrics_bin_t) \
* NGX_WA_METRICS_BINS_MAX
#define NGX_WA_METRICS_ONE_SLOT_METRIC_SIZE sizeof(ngx_wa_metric_t) \
+ sizeof(ngx_wa_metric_val_t)


typedef struct ngx_wa_metrics_s ngx_wa_metrics_t;

typedef enum {
Expand Down Expand Up @@ -74,7 +88,26 @@ ngx_int_t ngx_wa_metrics_increment(ngx_wa_metrics_t *metrics,
ngx_int_t ngx_wa_metrics_record(ngx_wa_metrics_t *metrics, uint32_t metric_id,
ngx_int_t val);
ngx_int_t ngx_wa_metrics_get(ngx_wa_metrics_t *metrics, uint32_t metric_id,
ngx_uint_t *out);
ngx_wa_metric_t *o);

ngx_int_t ngx_wa_metrics_histogram_add_locked(ngx_wa_metrics_t *metrics,
ngx_wa_metric_t *m);
ngx_int_t ngx_wa_metrics_histogram_record(ngx_wa_metrics_t *metrics,
ngx_wa_metric_t *m, ngx_uint_t slot, uint32_t mid, ngx_uint_t n);
void ngx_wa_metrics_histogram_get(ngx_wa_metrics_t *metrics, ngx_wa_metric_t *m,
ngx_uint_t slots, ngx_wa_metrics_histogram_t *out);


static ngx_inline ngx_wa_metrics_histogram_t *
ngx_wa_metrics_histogram_set_buffer(ngx_wa_metric_t *m, u_char *b, size_t s)
{
m->slots[0].histogram = (ngx_wa_metrics_histogram_t *) b;
m->slots[0].histogram->n_bins = (s - sizeof(ngx_wa_metrics_histogram_t))
/ sizeof(ngx_wa_metrics_bin_t);
m->slots[0].histogram->bins[0].upper_bound = NGX_MAX_UINT32_VALUE;

return m->slots[0].histogram;
}


#endif /* _NGX_WA_METRICS_H_INCLUDED_ */
42 changes: 28 additions & 14 deletions src/common/proxy_wasm/ngx_proxy_wasm_host.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1666,37 +1666,51 @@ ngx_proxy_wasm_hfuncs_get_metric(ngx_wavm_instance_t *instance,
ngx_uint_t *ret_value;
ngx_cycle_t *cycle = (ngx_cycle_t *) ngx_cycle;
ngx_wa_metrics_t *metrics = ngx_wasmx_metrics(cycle);
ngx_wa_metric_t *m;
ngx_proxy_wasm_exec_t *pwexec = ngx_proxy_wasm_instance2pwexec(instance);
u_char m_buf[NGX_WA_METRICS_ONE_SLOT_METRIC_SIZE];
u_char h_buf[NGX_WA_METRICS_MAX_HISTOGRAM_SIZE];
u_char trapmsg[NGX_MAX_ERROR_STR];

ngx_memzero(m_buf, sizeof(m_buf));
ngx_memzero(h_buf, sizeof(h_buf));

metric_id = args[0].of.i32;
ret_value = NGX_WAVM_HOST_LIFT(instance, args[1].of.i32, ngx_uint_t);

rc = ngx_wa_metrics_get(metrics, metric_id, ret_value);
if (rc != NGX_OK) {
ngx_memzero(trapmsg, NGX_MAX_ERROR_STR);
p = ngx_sprintf(trapmsg, "could not retrieve metric id \"%ui\": ",
metric_id);
m = (ngx_wa_metric_t *) m_buf;
ngx_wa_metrics_histogram_set_buffer(m, h_buf, sizeof(h_buf));

switch (rc) {
case NGX_DECLINED:
ngx_sprintf(p, "metric not found");
rc = ngx_wa_metrics_get(metrics, metric_id, m);
if (rc == NGX_OK && m->type != NGX_WA_METRIC_HISTOGRAM) {
switch (m->type) {
case NGX_WA_METRIC_COUNTER:
*ret_value = ngx_wa_metrics_counter(m);
break;

case NGX_ABORT:
ngx_sprintf(p, "metric is a histogram");
case NGX_WA_METRIC_GAUGE:
*ret_value = ngx_wa_metrics_gauge(m);
break;

default:
ngx_wa_assert(rc == NGX_OK);
break;
}

return ngx_proxy_wasm_result_trap(pwexec, (char *) trapmsg,
rets, NGX_WAVM_ERROR);
return ngx_proxy_wasm_result_ok(rets);
}

return ngx_proxy_wasm_result_ok(rets);
ngx_memzero(trapmsg, NGX_MAX_ERROR_STR);
p = ngx_sprintf(trapmsg, "could not retrieve metric id \"%ui\"", metric_id);

if (rc == NGX_DECLINED) {
ngx_sprintf(p, ": metric not found");

} else if (m->type == NGX_WA_METRIC_HISTOGRAM) {
ngx_sprintf(p, ": metric is a histogram");
}

return ngx_proxy_wasm_result_trap(pwexec, (char *) trapmsg,
rets, NGX_WAVM_ERROR);
}


Expand Down

0 comments on commit 8c6a796

Please sign in to comment.