From 8fb4bddb4c1a2724983f2e353d94538bbf5de2c4 Mon Sep 17 00:00:00 2001 From: Caio Ramos Casimiro Date: Fri, 3 May 2024 12:50:48 +0100 Subject: [PATCH] refactor(metrics) move histogram logic to its own module --- config | 2 + src/common/metrics/ngx_wa_histogram.c | 222 ++++++++++++++ src/common/metrics/ngx_wa_histogram.h | 16 + src/common/metrics/ngx_wa_metrics.c | 287 ++---------------- src/common/metrics/ngx_wa_metrics.h | 51 +++- src/common/proxy_wasm/ngx_proxy_wasm_host.c | 3 +- src/common/shm/ngx_wasm_shm_kv.c | 3 +- .../150-proxy_define_metric.t} | 0 .../151-proxy_increment_metric.t} | 0 .../152-proxy_record_metric.t} | 7 +- .../153-proxy_record_metric_histogram.t} | 0 ...cases.t => 001-define_metric_edge_cases.t} | 0 .../hfuncs/metrics/002-get_metric_misuse.t | 32 ++ ...misuse.t => 003-increment_metric_misuse.t} | 2 +- ...ic_misuse.t => 004-record_metric_misuse.t} | 2 +- t/07-metrics/001-metrics_sighup.t | 89 ++++-- t/07-metrics/002-histograms_sighup.t | 4 +- .../proxy-wasm-tests/hostcalls/src/filter.rs | 6 +- t/lib/proxy-wasm-tests/hostcalls/src/lib.rs | 14 +- .../hostcalls/src/tests/mod.rs | 43 +-- .../hostcalls/src/types/test_http.rs | 18 +- 21 files changed, 448 insertions(+), 353 deletions(-) create mode 100644 src/common/metrics/ngx_wa_histogram.c create mode 100644 src/common/metrics/ngx_wa_histogram.h rename t/03-proxy_wasm/hfuncs/{metrics/100-define_metric.t => contexts/150-proxy_define_metric.t} (100%) rename t/03-proxy_wasm/hfuncs/{metrics/200-increment_metric.t => contexts/151-proxy_increment_metric.t} (100%) rename t/03-proxy_wasm/hfuncs/{metrics/300-record_metric.t => contexts/152-proxy_record_metric.t} (95%) rename t/03-proxy_wasm/hfuncs/{metrics/400-record_histogram.t => contexts/153-proxy_record_metric_histogram.t} (100%) rename t/03-proxy_wasm/hfuncs/metrics/{101-define_metric_edge_cases.t => 001-define_metric_edge_cases.t} (100%) create mode 100644 t/03-proxy_wasm/hfuncs/metrics/002-get_metric_misuse.t rename t/03-proxy_wasm/hfuncs/metrics/{201-increment_metric_misuse.t => 003-increment_metric_misuse.t} (95%) rename t/03-proxy_wasm/hfuncs/metrics/{301-record_metric_misuse.t => 004-record_metric_misuse.t} (95%) diff --git a/config b/config index 1a62b68a2..78e62d9e2 100644 --- a/config +++ b/config @@ -143,6 +143,7 @@ NGX_WASMX_DEPS="\ $ngx_addon_dir/src/common/shm/ngx_wasm_shm.h \ $ngx_addon_dir/src/common/shm/ngx_wasm_shm_kv.h \ $ngx_addon_dir/src/common/shm/ngx_wasm_shm_queue.h \ + $ngx_addon_dir/src/common/metrics/ngx_wa_histogram.h \ $ngx_addon_dir/src/common/metrics/ngx_wa_metrics.h" NGX_WASMX_SRCS="\ @@ -158,6 +159,7 @@ NGX_WASMX_SRCS="\ $ngx_addon_dir/src/common/shm/ngx_wasm_shm.c \ $ngx_addon_dir/src/common/shm/ngx_wasm_shm_kv.c \ $ngx_addon_dir/src/common/shm/ngx_wasm_shm_queue.c \ + $ngx_addon_dir/src/common/metrics/ngx_wa_histogram.c \ $ngx_addon_dir/src/common/metrics/ngx_wa_metrics.c" # wasm diff --git a/src/common/metrics/ngx_wa_histogram.c b/src/common/metrics/ngx_wa_histogram.c new file mode 100644 index 000000000..929c6ecff --- /dev/null +++ b/src/common/metrics/ngx_wa_histogram.c @@ -0,0 +1,222 @@ +#ifndef DDEBUG +#define DDEBUG 0 +#endif +#include "ddebug.h" + +#include +#include + +#define NGX_WA_MIN_BIN 5 +#define NGX_WA_MAX_BIN 20 +#define NGX_WA_BIN_INCREASE 5 + + +static uint32_t +bin_log2_upper_bound(ngx_int_t n) +{ + uint32_t upper_bound = 2; + + if (n < 2) { + return 1; + } + + for (n = n - 1; n >>= 1; upper_bound <<= 1) { /* void */ } + + return upper_bound; +} + + +static ngx_int_t +histogram_grow(ngx_wa_metrics_t *metrics, ngx_wa_metrics_histogram_t *h, + ngx_wa_metrics_histogram_t **out) +{ + size_t old_size, size; + ngx_int_t rc = NGX_OK; + ngx_uint_t n; + ngx_wa_metrics_histogram_t *new_h = NULL; + + if (h->n_bins == NGX_WA_MAX_BIN) { + return NGX_ERROR; + } + + n = ngx_min(NGX_WA_BIN_INCREASE, NGX_WA_MAX_BIN - 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; + + if (metrics->shm->eviction == NGX_WASM_SHM_EVICTION_NONE) { + ngx_wasm_shm_lock(metrics->shm); + } + + new_h = ngx_slab_calloc_locked(metrics->shm->shpool, size); + if (new_h == NULL) { + rc = NGX_ERROR; + goto error; + } + + ngx_memcpy(new_h, h, old_size); + ngx_slab_free_locked(metrics->shm->shpool, h); + + new_h->n_bins += n; + *out = new_h; + +error: + + if (metrics->shm->eviction == NGX_WASM_SHM_EVICTION_NONE) { + ngx_wasm_shm_unlock(metrics->shm); + } + + return rc; +} + + +static ngx_wa_metrics_bin_t * +histogram_bin(ngx_wa_metrics_t *metrics, ngx_wa_metrics_histogram_t *h, + ngx_uint_t n, ngx_wa_metrics_histogram_t **out) +{ + size_t i, j = 0; + uint32_t ub = bin_log2_upper_bound(n); + ngx_wa_metrics_bin_t *b; + + for (i = 0; i < h->n_bins; i++) { + b = &h->bins[i]; + j = (j == 0 && ub < b->upper_bound) ? i : j; + + if (b->upper_bound == ub) { + return b; + + } else if (b->upper_bound == 0) { + break; + } + } + + if (i == h->n_bins) { + if (out && histogram_grow(metrics, h, out) == NGX_OK) { + h = *out; + + } else { + ngx_wasm_log_error(NGX_LOG_WARN, metrics->shm->log, 0, + "cannot add a new histogram bin for value " + "\"%uD\", returning next closest bin", n); + return &h->bins[j]; + } + } + + /* shift bins to create space for the new one */ + ngx_memcpy(&h->bins[j + 1], &h->bins[j], + sizeof(ngx_wa_metrics_bin_t) * (i - j)); + + h->bins[j].upper_bound = ub; + h->bins[j].count = 0; + + return &h->bins[j]; +} + + +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) +{ + size_t i, j = 0; + ngx_wa_metrics_bin_t *b, *out_b; + ngx_wa_metrics_histogram_t *h; + + for (i = 0; i < slots; i++) { + h = m->slots[i].histogram; + + 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 (NGX_DEBUG) +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_MAX_BIN; + ngx_wa_metrics_bin_t *b; + ngx_wa_metrics_histogram_t *h; + u_char *p, buf[size], s_buf[NGX_MAX_ERROR_STR]; + + ngx_memzero(buf, size); + + p = s_buf; + h = (ngx_wa_metrics_histogram_t *) buf; + h->n_bins = NGX_WA_MAX_BIN; + 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) { + break; + } + + p = ngx_sprintf(p, " %uD: %uD;", b->upper_bound, b->count); + } + + ngx_log_debug3(NGX_LOG_DEBUG_WASM, metrics->shm->log, 0, + "histogram \"%uD\": %*s", mid, p - s_buf - 1, s_buf + 1); +} +#endif + +ngx_int_t +ngx_wa_metrics_histogram_add_locked(ngx_wa_metrics_t *metrics, + ngx_wa_metric_t *m) +{ + size_t i; + uint16_t n_bins = NGX_WA_MIN_BIN; + ngx_wa_metrics_histogram_t **h; + + for (i = 0; i < metrics->workers; i++) { + h = &m->slots[i].histogram; + *h = ngx_slab_calloc_locked(metrics->shm->shpool, + sizeof(ngx_wa_metrics_histogram_t) + + sizeof(ngx_wa_metrics_bin_t) * n_bins); + if (*h == NULL) { + goto error; + } + + (*h)->n_bins = n_bins; + (*h)->bins[0].upper_bound = NGX_MAX_UINT32_VALUE; + } + + return NGX_OK; + +error: + + for (/* void */ ; i > 0; i--) { + ngx_slab_free_locked(metrics->shm->shpool, m->slots[i - 1].histogram); + } + + return NGX_ERROR; +} + + +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) +{ + ngx_wa_metrics_bin_t *b; + ngx_wa_metrics_histogram_t *h; + + h = m->slots[slot].histogram; + b = histogram_bin(metrics, h, n, &m->slots[slot].histogram); + b->count += 1; + +#if (NGX_DEBUG) + histogram_log(metrics, m, mid); +#endif + + return NGX_OK; +} diff --git a/src/common/metrics/ngx_wa_histogram.h b/src/common/metrics/ngx_wa_histogram.h new file mode 100644 index 000000000..854b1ad57 --- /dev/null +++ b/src/common/metrics/ngx_wa_histogram.h @@ -0,0 +1,16 @@ +#ifndef _NGX_WA_HISTOGRAM_H_INCLUDED_ +#define _NGX_WA_HISTOGRAM_H_INCLUDED_ + + +#include + + +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); + + +#endif /* _NGX_WA_HISTOGRAM_H_INCLUDED_ */ diff --git a/src/common/metrics/ngx_wa_metrics.c b/src/common/metrics/ngx_wa_metrics.c index 0b591a734..bb79b46cb 100644 --- a/src/common/metrics/ngx_wa_metrics.c +++ b/src/common/metrics/ngx_wa_metrics.c @@ -5,48 +5,10 @@ #include #include +#include - -typedef enum { - NGX_WA_HISTOGRAM_LOG2, - NGX_WA_HISTOGRAM_CUSTOM, -} ngx_wa_histogram_type_e; - - -typedef struct { - ngx_uint_t value; - ngx_msec_t last_update; -} ngx_wa_metrics_gauge_t; - - -typedef struct { - uint32_t upper_bound; /* could be uint16_t if it's ok to limit it at 65535 */ - uint32_t count; -} ngx_wa_histogram_bin_t; - - -typedef struct { - ngx_wa_histogram_type_e type; - uint8_t n_bins; - ngx_wa_histogram_bin_t bins[]; -} ngx_wa_histogram_t; - - -typedef union { - ngx_uint_t counter; - ngx_wa_metrics_gauge_t gauge; - ngx_wa_histogram_t *histogram; -} ngx_wa_metric_val_t; - - -typedef struct { - ngx_wa_metric_type_e type; - ngx_wa_metric_val_t slots[]; -} ngx_wa_metric_t; - - -static ngx_wa_histogram_bin_t *histogram_bin(ngx_wa_metrics_t *metrics, - ngx_wa_histogram_t *h, ngx_int_t n, ngx_wa_histogram_t **out); +#define NGX_WA_DEFAULT_METRIC_NAME_LEN 64 +#define NGX_WA_DEFAULT_METRICS_SLAB_SIZE 1024 * 1024 * 5 static ngx_str_t * @@ -73,139 +35,6 @@ metric_type_name(ngx_wa_metric_type_e type) } -static ngx_int_t -histogram_grow(ngx_wa_metrics_t *metrics, ngx_wa_histogram_t *h, - ngx_wa_histogram_t **out) -{ - size_t old_size, size; - ngx_int_t rc = NGX_OK; - ngx_uint_t n; - ngx_wa_histogram_t *new_h = NULL; - - if (h->n_bins == metrics->config.max_histogram_bins) { - return NGX_ERROR; /* histogram full */ - } - - n = ngx_min(5, metrics->config.max_histogram_bins - h->n_bins); - old_size = sizeof(ngx_wa_histogram_t) - + sizeof(ngx_wa_histogram_bin_t) * h->n_bins; - size = old_size + sizeof(ngx_wa_histogram_bin_t) * n; - - if (metrics->shm->eviction == NGX_WASM_SHM_EVICTION_NONE) { - ngx_wasm_shm_lock(metrics->shm); - } - - new_h = ngx_slab_calloc_locked(metrics->shm->shpool, size); - if (new_h == NULL) { - rc = NGX_ERROR; - goto error; - } - - ngx_memcpy(new_h, h, old_size); - ngx_slab_free_locked(metrics->shm->shpool, h); - - new_h->n_bins += n; - *out = new_h; - -error: - - if (metrics->shm->eviction == NGX_WASM_SHM_EVICTION_NONE) { - ngx_wasm_shm_unlock(metrics->shm); - } - - return rc; -} - - -static uint32_t -bin_log2_upper_bound(ngx_int_t n) -{ - uint32_t upper_bound = 2; - - if (n < 2) { - return 1; - } - - for (n = n - 1; n >>= 1; upper_bound <<= 1) { /* void */ } - - return upper_bound; -} - - -static ngx_wa_histogram_bin_t * -histogram_bin(ngx_wa_metrics_t *metrics, ngx_wa_histogram_t *h, ngx_int_t n, - ngx_wa_histogram_t **out) -{ - size_t i, j = 0; - uint32_t ub = bin_log2_upper_bound(n); - ngx_wa_histogram_bin_t *b; - - for (i = 0; i < h->n_bins; i++) { - b = &h->bins[i]; - j = (j == 0 && ub < b->upper_bound) ? i : j; - - if (b->upper_bound == ub) { - return b; - - } else if (b->upper_bound == 0) { - break; - } - } - - if (i == h->n_bins) { - if (out && histogram_grow(metrics, h, out) == NGX_OK) { - h = *out; - - } else { - return &h->bins[j]; /* can't grow, return next closest bin */ - } - } - - /* shift bins to create space for the new one */ - ngx_memcpy(&h->bins[j + 1], &h->bins[j], - sizeof(ngx_wa_histogram_bin_t) * (i - j)); - - h->bins[j].upper_bound = ub; - h->bins[j].count = 0; - - return &h->bins[j]; -} - - -static ngx_int_t -histogram_create_locked(ngx_wa_metrics_t *metrics, ngx_wa_metric_t *m) -{ - size_t i; - uint16_t n_bins = 5; - ngx_wa_histogram_t **h; - - for (i = 0; i < metrics->workers; i++) { - h = &m->slots[i].histogram; - *h = ngx_slab_calloc_locked(metrics->shm->shpool, - sizeof(ngx_wa_histogram_t) - + sizeof(ngx_wa_histogram_bin_t) - * n_bins); - if (*h == NULL) { - goto error; - } - - (*h)->type = NGX_WA_HISTOGRAM_LOG2; - (*h)->n_bins = n_bins; - (*h)->bins[0].upper_bound = NGX_MAX_UINT32_VALUE; - } - - return NGX_OK; - -error: - - for (/* void */ ; i > 0; i--) { - ngx_slab_free(metrics->shm->shpool, m->slots[i - 1].histogram); - } - - return NGX_ERROR; -} - - static ngx_uint_t counter_get(ngx_wa_metric_t *m, ngx_uint_t slots) { @@ -239,64 +68,6 @@ gauge_get(ngx_wa_metric_t *m, ngx_uint_t slots) } -static void -histogram_get(ngx_wa_metrics_t *metrics, ngx_wa_metric_t *m, ngx_uint_t slots, - ngx_wa_histogram_t *out) -{ - size_t i, j = 0; - ngx_wa_histogram_t *h; - ngx_wa_histogram_bin_t *b, *out_b; - - for (i = 0; i < slots; i++) { - h = m->slots[i].histogram; - - 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 (NGX_DEBUG) -static void -histogram_log(ngx_wa_metrics_t *metrics, ngx_wa_metric_t *m, uint32_t mid) -{ - size_t i, size = sizeof(ngx_wa_histogram_t) - + sizeof(ngx_wa_histogram_bin_t) - * metrics->config.max_histogram_bins; - ngx_wa_histogram_t *h; - ngx_wa_histogram_bin_t *b; - u_char *p, buf[size], s_buf[128]; - - ngx_memzero(buf, size); - - p = s_buf; - h = (ngx_wa_histogram_t *) buf; - h->n_bins = metrics->config.max_histogram_bins; - h->bins[0].upper_bound = NGX_MAX_UINT32_VALUE; - - histogram_get(metrics, m, metrics->workers, h); - - for (i = 0; i < h->n_bins; i++) { - b = &h->bins[i]; - if (b->upper_bound == 0) { - break; - } - - p = ngx_sprintf(p, " %uD: %uD;", b->upper_bound, b->count); - } - - ngx_log_debug3(NGX_LOG_DEBUG_WASM, metrics->shm->log, 0, - "histogram \"%uD\": %*s", mid, p - s_buf - 1, s_buf + 1); -} -#endif - - static ngx_int_t histogram_reallocate(ngx_wa_metrics_t *metrics, ngx_wa_metric_t *old_m, uint32_t mid) @@ -313,7 +84,7 @@ histogram_reallocate(ngx_wa_metrics_t *metrics, ngx_wa_metric_t *old_m, m = (ngx_wa_metric_t *) val->data; - histogram_get(metrics, old_m, slots, m->slots[0].histogram); + ngx_wa_metrics_histogram_get(metrics, old_m, slots, m->slots[0].histogram); return NGX_OK; } @@ -399,11 +170,9 @@ ngx_wa_metrics_create(ngx_cycle_t *cycle) return NULL; } - metrics->workers = 1; metrics->old_metrics = ngx_wasmx_metrics(); metrics->config.slab_size = NGX_CONF_UNSET_SIZE; metrics->config.max_metric_name_length = NGX_CONF_UNSET_SIZE; - metrics->config.max_histogram_bins = NGX_CONF_UNSET_SIZE; metrics->shm = ngx_pcalloc(cycle->pool, sizeof(ngx_wasm_shm_t)); if (metrics->shm == NULL) { @@ -414,7 +183,6 @@ ngx_wa_metrics_create(ngx_cycle_t *cycle) return NULL; } - /* cycle->log is set to `&cycle->new_log` later in ngx_init_cycle */ metrics->shm->log = &cycle->new_log; metrics->shm->name = shm_name; metrics->shm->type = NGX_WASM_SHM_TYPE_METRICS; @@ -434,18 +202,14 @@ ngx_wa_metrics_init_conf(ngx_wa_metrics_t *metrics, ngx_conf_t *cf) ngx_wa_metrics_t *old_metrics = metrics->old_metrics;; if (metrics->config.slab_size == NGX_CONF_UNSET_SIZE) { - metrics->config.slab_size = 5242880; /* 5m */ + metrics->config.slab_size = NGX_WA_DEFAULT_METRICS_SLAB_SIZE; } if (metrics->config.max_metric_name_length == NGX_CONF_UNSET_SIZE) { - metrics->config.max_metric_name_length = 64; - } - - if (metrics->config.max_histogram_bins == NGX_CONF_UNSET_SIZE) { - metrics->config.max_histogram_bins = 20; + metrics->config.max_metric_name_length = NGX_WA_DEFAULT_METRIC_NAME_LEN; } - /* if eviction is enabled, metrics->workers must be set to 1 */ + /* TODO: if eviction is enabled, metrics->workers must be set to 1 */ metrics->workers = ccf->worker_processes; metrics->shm_zone = ngx_shared_memory_add(cf, &metrics->shm->name, metrics->config.slab_size, @@ -456,13 +220,13 @@ ngx_wa_metrics_init_conf(ngx_wa_metrics_t *metrics, ngx_conf_t *cf) metrics->shm_zone->data = metrics->shm; metrics->shm_zone->init = ngx_wasm_shm_init_zone; - metrics->shm_zone->noreuse = false; + metrics->shm_zone->noreuse = 0; if (old_metrics && (metrics->workers != old_metrics->workers || metrics->config.slab_size != old_metrics->config.slab_size)) { - metrics->shm_zone->noreuse = true; + metrics->shm_zone->noreuse = 1; } return NGX_OK; @@ -473,7 +237,6 @@ ngx_int_t ngx_wa_metrics_init(ngx_wa_metrics_t *metrics, ngx_cycle_t *cycle) { ngx_int_t rc; - ngx_rbtree_node_t *root, *sentinel; ngx_wasm_shm_kv_t *old_shm_kv; if (metrics->old_metrics && !metrics->shm_zone->noreuse) { @@ -490,10 +253,9 @@ ngx_wa_metrics_init(ngx_wa_metrics_t *metrics, ngx_cycle_t *cycle) if (metrics->old_metrics && metrics->shm_zone->noreuse) { old_shm_kv = ngx_wasm_shm_get_kv(metrics->old_metrics->shm); - root = old_shm_kv->rbtree.root; - sentinel = old_shm_kv->rbtree.sentinel; - return metrics_reallocate(metrics, root, sentinel); + return metrics_reallocate(metrics, old_shm_kv->rbtree.root, + old_shm_kv->rbtree.sentinel); } return NGX_OK; @@ -504,12 +266,12 @@ ngx_int_t ngx_wa_metrics_add(ngx_wa_metrics_t *metrics, ngx_str_t *name, ngx_wa_metric_type_e type, uint32_t *out) { + ssize_t size = sizeof(ngx_wa_metric_t) + + sizeof(ngx_wa_metric_val_t) * metrics->workers; uint32_t cas, mid; ngx_int_t rc, written; ngx_str_t *p, val; ngx_wa_metric_t *m; - ssize_t size = sizeof(ngx_wa_metric_t) + - sizeof(ngx_wa_metric_val_t) * metrics->workers; u_char buf[size]; mid = ngx_crc32_long(name->data, name->len); @@ -541,7 +303,7 @@ ngx_wa_metrics_add(ngx_wa_metrics_t *metrics, ngx_str_t *name, m->type = type; if (type == NGX_WA_METRIC_HISTOGRAM) { - rc = histogram_create_locked(metrics, m); + rc = ngx_wa_metrics_histogram_add_locked(metrics, m); if (rc != NGX_OK) { dd("failed creating histogram"); goto error; @@ -586,6 +348,8 @@ ngx_wa_metrics_get(ngx_wa_metrics_t *metrics, uint32_t mid, ngx_uint_t *out) rc = ngx_wasm_shm_kv_get_locked(metrics->shm, NULL, &mid, &n, &cas); if (rc != NGX_OK) { + ngx_wasm_log_error(NGX_LOG_ERR, metrics->shm->log, 0, + "metric \"%uD\" not found", mid); return rc; } @@ -668,12 +432,11 @@ ngx_wa_metrics_increment(ngx_wa_metrics_t *metrics, uint32_t mid, ngx_int_t n) ngx_int_t ngx_wa_metrics_record(ngx_wa_metrics_t *metrics, uint32_t mid, ngx_int_t n) { - uint32_t cas; - ngx_int_t rc = NGX_OK; - ngx_str_t *val; - ngx_uint_t slot; - ngx_wa_metric_t *m; - ngx_wa_histogram_bin_t *bin; + uint32_t cas; + ngx_int_t rc = NGX_OK; + ngx_str_t *val; + ngx_uint_t slot; + ngx_wa_metric_t *m; slot = (ngx_process == NGX_PROCESS_WORKER) ? ngx_worker : 0; @@ -702,12 +465,8 @@ ngx_wa_metrics_record(ngx_wa_metrics_t *metrics, uint32_t mid, ngx_int_t n) break; case NGX_WA_METRIC_HISTOGRAM: - bin = histogram_bin(metrics, m->slots[slot].histogram, n, - &m->slots[slot].histogram); - bin->count += 1; -#if (NGX_DEBUG) - histogram_log(metrics, m, mid); -#endif + ngx_wa_metrics_histogram_record(metrics, m, slot, mid, n); + break; default: diff --git a/src/common/metrics/ngx_wa_metrics.h b/src/common/metrics/ngx_wa_metrics.h index b325a9d28..a884b0e6d 100644 --- a/src/common/metrics/ngx_wa_metrics.h +++ b/src/common/metrics/ngx_wa_metrics.h @@ -8,6 +8,12 @@ typedef struct ngx_wa_metrics_s ngx_wa_metrics_t; +typedef struct { + size_t slab_size; + size_t max_metric_name_length; +} ngx_wa_metrics_conf_t; + + typedef enum { NGX_WA_METRIC_COUNTER, NGX_WA_METRIC_GAUGE, @@ -16,17 +22,41 @@ typedef enum { typedef struct { - size_t slab_size; - size_t max_metric_name_length; - size_t max_histogram_bins; -} ngx_wa_metrics_conf_t; + ngx_uint_t value; + ngx_msec_t last_update; +} ngx_wa_metrics_gauge_t; + + +typedef struct { + uint32_t upper_bound; + uint32_t count; +} ngx_wa_metrics_bin_t; + + +typedef struct { + uint8_t n_bins; + ngx_wa_metrics_bin_t bins[]; +} ngx_wa_metrics_histogram_t; + + +typedef union { + ngx_uint_t counter; + ngx_wa_metrics_gauge_t gauge; + ngx_wa_metrics_histogram_t *histogram; +} ngx_wa_metric_val_t; + + +typedef struct { + ngx_wa_metric_type_e type; + ngx_wa_metric_val_t slots[]; +} ngx_wa_metric_t; struct ngx_wa_metrics_s { - ngx_uint_t workers; - ngx_shm_zone_t *shm_zone; - ngx_wasm_shm_t *shm; - ngx_wa_metrics_t *old_metrics; + ngx_uint_t workers; + ngx_shm_zone_t *shm_zone; + ngx_wasm_shm_t *shm; + ngx_wa_metrics_t *old_metrics; ngx_wa_metrics_conf_t config; }; @@ -34,11 +64,11 @@ struct ngx_wa_metrics_s { ngx_wa_metrics_t *ngx_wasmx_metrics(); -ngx_wa_metrics_t *ngx_wa_metrics_create(ngx_cycle_t *cycle); +ngx_wa_metrics_t *ngx_wa_metrics_create(ngx_cycle_t *cycle); // alloc ngx_int_t ngx_wa_metrics_init_conf(ngx_wa_metrics_t *metrics, ngx_conf_t *cf); ngx_int_t ngx_wa_metrics_init(ngx_wa_metrics_t *metrics, ngx_cycle_t *cycle); ngx_int_t ngx_wa_metrics_add(ngx_wa_metrics_t *metrics, ngx_str_t *name, - ngx_wa_metric_type_e type, uint32_t *out); + ngx_wa_metric_type_e type, uint32_t *out); // create ngx_int_t ngx_wa_metrics_get(ngx_wa_metrics_t *metrics, uint32_t metric_id, ngx_uint_t *out); ngx_int_t ngx_wa_metrics_increment(ngx_wa_metrics_t *metrics, @@ -46,4 +76,5 @@ 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); + #endif /* _NGX_WA_METRICS_H_INCLUDED_ */ diff --git a/src/common/proxy_wasm/ngx_proxy_wasm_host.c b/src/common/proxy_wasm/ngx_proxy_wasm_host.c index 3faf92c09..a0b160aaf 100644 --- a/src/common/proxy_wasm/ngx_proxy_wasm_host.c +++ b/src/common/proxy_wasm/ngx_proxy_wasm_host.c @@ -1618,6 +1618,7 @@ ngx_proxy_wasm_hfuncs_define_metric(ngx_wavm_instance_t *instance, NGX_WAVM_ERROR); } + /* consider adding filter position in the filter-chain to the prefix */ filter_name = pwexec->filter->name; if (filter_name->len + 4 + name.len > max_len) { @@ -1700,7 +1701,7 @@ ngx_proxy_wasm_hfuncs_get_metric(ngx_wavm_instance_t *instance, rc = ngx_wa_metrics_get(metrics, metric_id, ret_value); - if (rc == NGX_ERROR) { + if (rc != NGX_OK) { /* TODO: format with key */ return ngx_proxy_wasm_result_trap(pwexec, "could not retrieve metric", rets, NGX_WAVM_ERROR); diff --git a/src/common/shm/ngx_wasm_shm_kv.c b/src/common/shm/ngx_wasm_shm_kv.c index 94e77cb95..834b46212 100644 --- a/src/common/shm/ngx_wasm_shm_kv.c +++ b/src/common/shm/ngx_wasm_shm_kv.c @@ -270,8 +270,7 @@ ngx_wasm_shm_kv_set_locked(ngx_wasm_shm_t *shm, ngx_str_t *key, ngx_wasm_shm_kv_node_t *n, *old; old = NULL; - n = (ngx_wasm_shm_kv_node_t *) ngx_wasm_shm_rbtree_lookup(&kv->rbtree, - key_hash); + n = ngx_wasm_shm_rbtree_lookup(&kv->rbtree, key_hash); if (cas != (n == NULL ? 0 : n->cas)) { *written = 0; diff --git a/t/03-proxy_wasm/hfuncs/metrics/100-define_metric.t b/t/03-proxy_wasm/hfuncs/contexts/150-proxy_define_metric.t similarity index 100% rename from t/03-proxy_wasm/hfuncs/metrics/100-define_metric.t rename to t/03-proxy_wasm/hfuncs/contexts/150-proxy_define_metric.t diff --git a/t/03-proxy_wasm/hfuncs/metrics/200-increment_metric.t b/t/03-proxy_wasm/hfuncs/contexts/151-proxy_increment_metric.t similarity index 100% rename from t/03-proxy_wasm/hfuncs/metrics/200-increment_metric.t rename to t/03-proxy_wasm/hfuncs/contexts/151-proxy_increment_metric.t diff --git a/t/03-proxy_wasm/hfuncs/metrics/300-record_metric.t b/t/03-proxy_wasm/hfuncs/contexts/152-proxy_record_metric.t similarity index 95% rename from t/03-proxy_wasm/hfuncs/metrics/300-record_metric.t rename to t/03-proxy_wasm/hfuncs/contexts/152-proxy_record_metric.t index 00ad66089..ff5745139 100644 --- a/t/03-proxy_wasm/hfuncs/metrics/300-record_metric.t +++ b/t/03-proxy_wasm/hfuncs/contexts/152-proxy_record_metric.t @@ -46,6 +46,7 @@ qr/$check/ === TEST 2: proxy_wasm metrics - record_metric(), gauges - on_tick +--- ONLY --- wasm_modules: hostcalls --- load_nginx_modules: ngx_http_echo_module --- config eval @@ -70,13 +71,13 @@ qq{ } } --- wait: 1 ---- grep_error_log eval: qr/(set \w+ to \d+|\w+: \d+) at \w+/ +--- grep_error_log eval: qr/(record \d+ on \w+|\w+: \d+) at \w+/ --- grep_error_log_out eval my $checks; foreach my $worker_id (0 .. $::workers - 1) { - $checks .= "set g1_Configure to $worker_id at Tick -set g2_Configure to $worker_id at Tick + $checks .= "record $worker_id on g1_Configure at Tick +record $worker_id on g2_Configure at Tick g1_Configure: $worker_id at Tick g2_Configure: $worker_id at Tick "; diff --git a/t/03-proxy_wasm/hfuncs/metrics/400-record_histogram.t b/t/03-proxy_wasm/hfuncs/contexts/153-proxy_record_metric_histogram.t similarity index 100% rename from t/03-proxy_wasm/hfuncs/metrics/400-record_histogram.t rename to t/03-proxy_wasm/hfuncs/contexts/153-proxy_record_metric_histogram.t diff --git a/t/03-proxy_wasm/hfuncs/metrics/101-define_metric_edge_cases.t b/t/03-proxy_wasm/hfuncs/metrics/001-define_metric_edge_cases.t similarity index 100% rename from t/03-proxy_wasm/hfuncs/metrics/101-define_metric_edge_cases.t rename to t/03-proxy_wasm/hfuncs/metrics/001-define_metric_edge_cases.t diff --git a/t/03-proxy_wasm/hfuncs/metrics/002-get_metric_misuse.t b/t/03-proxy_wasm/hfuncs/metrics/002-get_metric_misuse.t new file mode 100644 index 000000000..652646e83 --- /dev/null +++ b/t/03-proxy_wasm/hfuncs/metrics/002-get_metric_misuse.t @@ -0,0 +1,32 @@ +# vim:set ft= ts=4 sts=4 sw=4 et fdm=marker: + +use strict; +use lib '.'; +use t::TestWasmX; + +plan_tests(7); +run_tests(); + +__DATA__ + +=== TEST 1: proxy_wasm metrics - get_metric() invalid metric id +--- valgrind +--- load_nginx_modules: ngx_http_echo_module +--- wasm_modules: hostcalls +--- config + location /t { + proxy_wasm hostcalls 'on=request_headers \ + test=/t/metrics/get_invalid_metric'; + echo ok; + } +--- error_code: 500 +--- error_log eval +[ + qr/.+on_request_headers.+/, + qr/metric \"0\" not found.*/, +] +--- no_error_log +[crit] +[emerg] +[alert] +[stub] diff --git a/t/03-proxy_wasm/hfuncs/metrics/201-increment_metric_misuse.t b/t/03-proxy_wasm/hfuncs/metrics/003-increment_metric_misuse.t similarity index 95% rename from t/03-proxy_wasm/hfuncs/metrics/201-increment_metric_misuse.t rename to t/03-proxy_wasm/hfuncs/metrics/003-increment_metric_misuse.t index 68874ccfd..56b641bb7 100644 --- a/t/03-proxy_wasm/hfuncs/metrics/201-increment_metric_misuse.t +++ b/t/03-proxy_wasm/hfuncs/metrics/003-increment_metric_misuse.t @@ -18,7 +18,7 @@ qq{ location /t { proxy_wasm hostcalls 'on_configure=define_metrics \ on=request_headers \ - test=/t/metrics/increment_counters \ + test=/t/metrics/increment_gauges \ metrics=g1,g2'; echo ok; } diff --git a/t/03-proxy_wasm/hfuncs/metrics/301-record_metric_misuse.t b/t/03-proxy_wasm/hfuncs/metrics/004-record_metric_misuse.t similarity index 95% rename from t/03-proxy_wasm/hfuncs/metrics/301-record_metric_misuse.t rename to t/03-proxy_wasm/hfuncs/metrics/004-record_metric_misuse.t index 954b9ebdd..f1e6532d6 100644 --- a/t/03-proxy_wasm/hfuncs/metrics/301-record_metric_misuse.t +++ b/t/03-proxy_wasm/hfuncs/metrics/004-record_metric_misuse.t @@ -17,7 +17,7 @@ __DATA__ location /t { proxy_wasm hostcalls 'on_configure=define_metrics \ on=request_headers \ - test=/t/metrics/toggle_gauges \ + test=/t/metrics/toggle_counters \ metrics=c1,c2'; echo ok; } diff --git a/t/07-metrics/001-metrics_sighup.t b/t/07-metrics/001-metrics_sighup.t index 2bad9eefb..58854e5ce 100644 --- a/t/07-metrics/001-metrics_sighup.t +++ b/t/07-metrics/001-metrics_sighup.t @@ -7,6 +7,8 @@ skip_no_hup(); our $workers = 2; +our $metrics = "c2,g2000,h2000"; + workers($workers); if ($workers > 1) { master_on(); @@ -18,7 +20,7 @@ run_tests(); __DATA__ -=== TEST 1: SIGHUP metrics - define and increment counters +=== TEST 1: SIGHUP metrics - define metrics and increment counters Evaluating counters values in error_log rather than in response headers as some worker(s) might not have done their increment by the time response_headers phase is invoked. @@ -30,14 +32,12 @@ is invoked. qq{ location /t { proxy_wasm hostcalls 'on_configure=define_and_increment_counters \ - metrics=c2'; + metrics=$::metrics'; echo ok; } } ---- grep_error_log eval: qr/c\d_Configure: \d+/ ---- grep_error_log_out eval -qr/c1_Configure: $::workers.* -c2_Configure: $::workers.*/ +--- error_log eval +qr/c2_Configure: $::workers.*/ --- no_error_log [error] [crit] @@ -58,7 +58,7 @@ qq{ proxy_wasm hostcalls 'on_configure=define_and_increment_counters \ on=response_headers \ test=/t/metrics/get \ - metrics=c2'; + metrics=$::metrics'; echo ok; } } @@ -85,17 +85,15 @@ qq{ proxy_wasm hostcalls 'on_configure=define_and_increment_counters \ on=response_headers \ test=/t/metrics/get \ - metrics=c2'; + metrics=$::metrics'; echo ok; } } --- response_headers c1-Configure: 8 c2-Configure: 8 ---- grep_error_log eval: qr/reallocating metric .+/ ---- grep_error_log_out eval -qr/reallocating metric .*c1.* -reallocating metric .*c2.*/ +--- error_log eval +qr/reallocating metric .+c2.+/ --- no_error_log [error] [crit] @@ -115,17 +113,15 @@ qq{ proxy_wasm hostcalls 'on_configure=define_and_increment_counters \ on=response_headers \ test=/t/metrics/get \ - metrics=c2'; + metrics=$::metrics'; echo ok; } } --- response_headers c1-Configure: 10 c2-Configure: 10 ---- grep_error_log eval: qr/reallocating metric .+/ ---- grep_error_log_out eval -qr/reallocating metric .*c1.* -reallocating metric .*c2.*/ +--- error_log eval +qr/reallocating metric .+c2.+/ --- no_error_log [error] [crit] @@ -144,7 +140,7 @@ qq{ module hostcalls $ENV{TEST_NGINX_CRATES_DIR}/hostcalls.wasm; metrics { - slab_size 12k; + slab_size 4m; } } } @@ -154,17 +150,15 @@ qq{ proxy_wasm hostcalls 'on_configure=define_and_increment_counters \ on=response_headers \ test=/t/metrics/get \ - metrics=c2'; + metrics=$::metrics'; echo ok; } } --- response_headers c1-Configure: 12 c2-Configure: 12 ---- grep_error_log eval: qr/reallocating metric .+/ ---- grep_error_log_out eval -qr/reallocating metric .*c1.* -reallocating metric .*c2.*/ +--- error_log eval +qr/reallocating metric .+c2.+/ --- no_error_log [error] [crit] @@ -183,7 +177,7 @@ qq{ module hostcalls $ENV{TEST_NGINX_CRATES_DIR}/hostcalls.wasm; metrics { - slab_size 16k; + slab_size 5m; } } } @@ -193,19 +187,56 @@ qq{ proxy_wasm hostcalls 'on_configure=define_and_increment_counters \ on=response_headers \ test=/t/metrics/get \ - metrics=c2'; + metrics=$::metrics'; echo ok; } } --- response_headers c1-Configure: 14 c2-Configure: 14 ---- grep_error_log eval: qr/reallocating metric .+/ ---- grep_error_log_out eval -qr/reallocating metric .*c1.* -reallocating metric .*c2.*/ +--- error_log eval +qr/reallocating metric .+c2.+/ --- no_error_log [error] [crit] [emerg] [alert] + + + +=== TEST 7: SIGHUP metrics - decreased slab_size - not enough memory +--- workers: 2 +--- valgrind +--- must_die: 0 +--- load_nginx_modules: ngx_http_echo_module +--- main_config eval +qq{ + wasm { + module hostcalls $ENV{TEST_NGINX_CRATES_DIR}/hostcalls.wasm; + + metrics { + slab_size 12k; + } + } +} +--- config eval +qq{ + location /t { + proxy_wasm hostcalls 'on_configure=define_and_increment_counters \ + on=response_headers \ + test=/t/metrics/get \ + metrics=$::metrics'; + echo ok; + } +} +--- error_log eval +[ + qr/\[crit\] .+ \[wasm\] "metrics" shm store: no memory; cannot allocate pair with key size \d+ and value size \d+/, + qr/host trap \(internal error\): could not define metric.*/, +] +--- no_error_log +[stub] +[stub] +[stub] +[stub] +[stub] diff --git a/t/07-metrics/002-histograms_sighup.t b/t/07-metrics/002-histograms_sighup.t index ed6d77c22..5ce503693 100644 --- a/t/07-metrics/002-histograms_sighup.t +++ b/t/07-metrics/002-histograms_sighup.t @@ -19,7 +19,7 @@ run_tests(); __DATA__ -=== TEST 1: SIGHUP metrics - define and record histograms +=== TEST 1: SIGHUP metrics - define metrics and record histograms --- valgrind --- load_nginx_modules: ngx_http_echo_module --- wasm_modules: hostcalls @@ -27,7 +27,7 @@ __DATA__ qq{ location /t { proxy_wasm hostcalls 'on_configure=define_and_record_histograms \ - metrics=h2'; + metrics=c2,g2,h2'; echo ok; } } diff --git a/t/lib/proxy-wasm-tests/hostcalls/src/filter.rs b/t/lib/proxy-wasm-tests/hostcalls/src/filter.rs index 7eda5d6fc..0da33c0ba 100644 --- a/t/lib/proxy-wasm-tests/hostcalls/src/filter.rs +++ b/t/lib/proxy-wasm-tests/hostcalls/src/filter.rs @@ -103,9 +103,9 @@ impl Context for TestHttp { ); } "define_metrics" => test_define_metrics(self, TestPhase::HTTPCallResponse), - "increment_counters" => test_increment_counters(self, TestPhase::HTTPCallResponse), - "toggle_gauges" => test_toggle_gauges(self, TestPhase::HTTPCallResponse), - "record_histograms" => test_record_histograms(self, TestPhase::HTTPCallResponse), + "increment_counters" => test_increment_counters(self, TestPhase::HTTPCallResponse, None), + "toggle_gauges" => test_toggle_gauges(self, TestPhase::HTTPCallResponse, None), + "record_histograms" => test_record_metric(self, TestPhase::HTTPCallResponse), _ => {} } diff --git a/t/lib/proxy-wasm-tests/hostcalls/src/lib.rs b/t/lib/proxy-wasm-tests/hostcalls/src/lib.rs index d226fe2d0..f3479dda6 100644 --- a/t/lib/proxy-wasm-tests/hostcalls/src/lib.rs +++ b/t/lib/proxy-wasm-tests/hostcalls/src/lib.rs @@ -70,15 +70,15 @@ impl RootContext for TestRoot { "define_metrics" => test_define_metrics(self, TestPhase::Configure), "define_and_increment_counters" => { test_define_metrics(self, TestPhase::Configure); - test_increment_counters(self, TestPhase::Configure); + test_increment_counters(self, TestPhase::Configure, None); } "define_and_toggle_gauges" => { test_define_metrics(self, TestPhase::Configure); - test_toggle_gauges(self, TestPhase::Configure); + test_toggle_gauges(self, TestPhase::Configure, None); } "define_and_record_histograms" => { test_define_metrics(self, TestPhase::Configure); - test_record_histograms(self, TestPhase::Configure); + test_record_metric(self, TestPhase::Configure); } _ => (), } @@ -111,14 +111,14 @@ impl RootContext for TestRoot { test_define_metrics(self, TestPhase::Tick); self.n_sync_calls += 1; } - "increment_counters" => test_increment_counters(self, TestPhase::Tick), - "toggle_gauges" => test_toggle_gauges(self, TestPhase::Tick), + "increment_counters" => test_increment_counters(self, TestPhase::Tick, None), + "toggle_gauges" => test_toggle_gauges(self, TestPhase::Tick, None), "set_gauges" => { - test_set_gauges(self, TestPhase::Tick); + test_record_metric(self, TestPhase::Tick); self.n_sync_calls += 1; } "record_histograms" => { - test_record_histograms(self, TestPhase::Tick); + test_record_metric(self, TestPhase::Tick); self.n_sync_calls += 1; } "log_metrics" => test_log_metrics(self, TestPhase::Tick), diff --git a/t/lib/proxy-wasm-tests/hostcalls/src/tests/mod.rs b/t/lib/proxy-wasm-tests/hostcalls/src/tests/mod.rs index 0c27cb9b4..74e6e21f4 100644 --- a/t/lib/proxy-wasm-tests/hostcalls/src/tests/mod.rs +++ b/t/lib/proxy-wasm-tests/hostcalls/src/tests/mod.rs @@ -125,6 +125,7 @@ pub(crate) fn test_log_properties(ctx: &(dyn TestContext + 'static), phase: Test pub(crate) fn test_log_metrics(ctx: &(dyn TestContext + 'static), phase: TestPhase) { for (n, id) in ctx.get_metrics_mapping() { + if n.starts_with('h') { continue } let value = get_metric(*id).unwrap(); info!("{}: {} at {:?}", n, value, phase) } @@ -462,18 +463,14 @@ pub(crate) fn test_define_metrics(ctx: &mut (dyn TestContext + 'static), phase: let n = metric[1..].parse::().expect("bad metrics value"); for i in 1..(n + 1) { - let m_id; - let name = format!("{}{}_{:?}", metric_char, i, phase); + let mut name = format!("{}{}_{:?}", metric_char, i, phase); if name_len > 0 { - let suffixed_name = - format!("{}{}", name, "x".repeat(name_len - name.chars().count())); - m_id = - define_metric(metric_type, &suffixed_name).expect("cannot define new metric"); - } else { - m_id = define_metric(metric_type, &name).expect("cannot define new metric"); + name = format!("{}{}", name, "x".repeat(name_len - name.chars().count())); } + let m_id = define_metric(metric_type, &name).expect("cannot define new metric"); + info!("defined metric {} as {:?} at {:?}", &name, m_id, phase); ctx.save_metric_mapping(name.as_str(), m_id); @@ -481,7 +478,7 @@ pub(crate) fn test_define_metrics(ctx: &mut (dyn TestContext + 'static), phase: } } -pub(crate) fn test_increment_counters(ctx: &(dyn TestContext + 'static), phase: TestPhase) { +pub(crate) fn test_increment_counters(ctx: &(dyn TestContext + 'static), phase: TestPhase, skip_others: Option) { if !should_run_on_current_worker(ctx) { return; } @@ -491,6 +488,8 @@ pub(crate) fn test_increment_counters(ctx: &(dyn TestContext + 'static), phase: .map_or(1, |x| x.parse::().expect("bad n_increments value")); for (n, id) in ctx.get_metrics_mapping() { + if skip_others.unwrap_or(true) && !n.starts_with('c') { continue } + for _ in 0..n_increments { increment_metric(*id, 1).unwrap(); } @@ -501,12 +500,14 @@ pub(crate) fn test_increment_counters(ctx: &(dyn TestContext + 'static), phase: test_log_metrics(ctx, phase); } -pub(crate) fn test_toggle_gauges(ctx: &(dyn TestContext + 'static), phase: TestPhase) { +pub(crate) fn test_toggle_gauges(ctx: &(dyn TestContext + 'static), phase: TestPhase, skip_others: Option) { if !should_run_on_current_worker(ctx) { return; } for (n, id) in ctx.get_metrics_mapping() { + if skip_others.unwrap_or(true) && !n.starts_with('g') { continue } + let new_value = match get_metric(*id).unwrap() { 0 => 1, _ => 0, @@ -519,7 +520,7 @@ pub(crate) fn test_toggle_gauges(ctx: &(dyn TestContext + 'static), phase: TestP test_log_metrics(ctx, phase); } -pub(crate) fn test_set_gauges(ctx: &(dyn TestContext + 'static), phase: TestPhase) { +pub(crate) fn test_record_metric(ctx: &(dyn TestContext + 'static), phase: TestPhase) { if !should_run_on_current_worker(ctx) { return; } @@ -529,30 +530,18 @@ pub(crate) fn test_set_gauges(ctx: &(dyn TestContext + 'static), phase: TestPhas .map_or(1, |x| x.parse::().expect("bad value")); for (n, id) in ctx.get_metrics_mapping() { + if n.starts_with('c') { continue } record_metric(*id, value).unwrap(); - info!("set {} to {} at {:?}", n, value, phase); + info!("record {} on {} at {:?}", value, n, phase); } test_log_metrics(ctx, phase); } -pub(crate) fn test_record_histograms(ctx: &(dyn TestContext + 'static), phase: TestPhase) { - if !should_run_on_current_worker(ctx) { - return; - } - - let value = ctx - .get_config("value") - .map_or(1, |x| x.parse::().expect("bad value")); - - for (n, id) in ctx.get_metrics_mapping() { - record_metric(*id, value).unwrap(); - info!("record {} on {} at {:?}", value, n, phase); - } -} - pub(crate) fn test_get_metrics(ctx: &TestHttp) { for (n, id) in ctx.get_metrics_mapping() { + if n.starts_with('h') { continue } + let name = n.replace("_", "-"); let value = get_metric(*id).unwrap().to_string(); ctx.add_http_response_header(name.as_str(), value.as_str()); diff --git a/t/lib/proxy-wasm-tests/hostcalls/src/types/test_http.rs b/t/lib/proxy-wasm-tests/hostcalls/src/types/test_http.rs index b579f6b6b..bd9ce7ae2 100644 --- a/t/lib/proxy-wasm-tests/hostcalls/src/types/test_http.rs +++ b/t/lib/proxy-wasm-tests/hostcalls/src/types/test_http.rs @@ -119,12 +119,24 @@ impl TestHttp { /* metrics */ "/t/metrics/define" => test_define_metrics(self, cur_phase), - "/t/metrics/increment_counters" => test_increment_counters(self, cur_phase), - "/t/metrics/toggle_gauges" => test_toggle_gauges(self, cur_phase), - "/t/metrics/record_histograms" => test_record_histograms(self, cur_phase), + "/t/metrics/increment_counters" => test_increment_counters(self, cur_phase, None), + "/t/metrics/increment_gauges" => { + let skip_non_counters = false; + test_increment_counters(self, cur_phase, Some(skip_non_counters)); + } + "/t/metrics/toggle_gauges" => test_toggle_gauges(self, cur_phase, None), + "/t/metrics/toggle_counters" => { + let skip_non_gauges = false; + test_toggle_gauges(self, cur_phase, Some(skip_non_gauges)); + } + "/t/metrics/record_histograms" => test_record_metric(self, cur_phase), "/t/metrics/get" => test_get_metrics(self), "/t/metrics/increment_invalid_counter" => increment_metric(0, 1).unwrap(), "/t/metrics/set_invalid_gauge" => record_metric(0, 1).unwrap(), + "/t/metrics/get_invalid_metric" => { + info!("[hostcalls] retrieving invalid metric in \"{:?}\"", cur_phase); + get_metric(0).unwrap(); + } /* errors */ "/t/trap" => panic!("custom trap"),