From 630a5e7de0b4cf8f8e14ad0033a4998a97d6fe6b Mon Sep 17 00:00:00 2001 From: Caio Ramos Casimiro Date: Mon, 28 Oct 2024 14:55:03 +0000 Subject: [PATCH] feat(metrics) switch metric name metadata separator to ':' Switch from `.` to `:` since module names can contain dots, making the prefix difficult to parse. --- docs/METRICS.md | 6 +++--- lib/resty/wasmx/shm.lua | 12 ++++++------ src/common/proxy_wasm/ngx_proxy_wasm_host.c | 2 +- t/04-openresty/ffi/shm/020-metrics_define.t | 8 ++++---- t/04-openresty/ffi/shm/025-metrics_get_by_name.t | 10 +++++----- t/04-openresty/ffi/shm/026-metrics_iterate_keys.t | 6 +++--- t/04-openresty/ffi/shm/027-metrics_get_keys.t | 6 +++--- 7 files changed, 25 insertions(+), 25 deletions(-) diff --git a/docs/METRICS.md b/docs/METRICS.md index ea43e77f7..a5c4981bb 100644 --- a/docs/METRICS.md +++ b/docs/METRICS.md @@ -38,9 +38,9 @@ gauge, or a histogram. ## Name Prefixing To avoid naming conflicts between Proxy-Wasm filters, the name of a metric is -always prefixed with: `pw.{filter_name}.{metric_name}`. This means that a metric -named `a_counter` inserted by `a_filter` will have its name stored as: -`pw.a_filter.a_counter`. +always prefixed with colon-separated metadata: `pw:{filter_name}:`. This means +that a metric named `a_counter` inserted by `a_filter` will have its name stored +as: `pw:a_filter:a_counter`. Thus, the maximum length of a metric name configured via [max_metric_name_length] is enforced on the prefixed name and may need to be diff --git a/lib/resty/wasmx/shm.lua b/lib/resty/wasmx/shm.lua index 50880099c..a87627294 100644 --- a/lib/resty/wasmx/shm.lua +++ b/lib/resty/wasmx/shm.lua @@ -403,7 +403,7 @@ local function metrics_define(zone, name, metric_type, opts) end end - name = "lua." .. name + name = "lua:" .. name local cname = ffi_new("ngx_str_t", { data = name, len = #name }) local m_id = ffi_new("uint32_t[1]") @@ -528,12 +528,12 @@ end --- --- ngx_wasm_module internally prefixes metric names according to --- where they have been defined, e.g. "pw.filter.*", "lua.*", or --- "wa.*". +-- ngx_wasm_module internally prefixes metric names with colon-separated +-- metadata indicating where they have been defined, e.g. "pw:a_filter:*", +-- "lua:*", or "wa:". -- -- metrics_get_by_name assumes it is retrieving a Lua-defined metric --- and will by default prefix the given name with `lua.` +-- and will by default prefix the given name with `lua:` -- -- This behavior can be disabled by passing `opts.prefix` as false. local function metrics_get_by_name(zone, name, opts) @@ -554,7 +554,7 @@ local function metrics_get_by_name(zone, name, opts) ffi_fill(_mbuf, _mbs) ffi_fill(_hbuf, _hbs) - name = (opts and opts.prefix == false) and name or "lua." .. name + name = (opts and opts.prefix == false) and name or "lua:" .. name local cname = ffi_new("ngx_str_t", { data = name, len = #name }) diff --git a/src/common/proxy_wasm/ngx_proxy_wasm_host.c b/src/common/proxy_wasm/ngx_proxy_wasm_host.c index d92ffddb7..efde7b98f 100644 --- a/src/common/proxy_wasm/ngx_proxy_wasm_host.c +++ b/src/common/proxy_wasm/ngx_proxy_wasm_host.c @@ -1635,7 +1635,7 @@ ngx_proxy_wasm_hfuncs_define_metric(ngx_wavm_instance_t *instance, } prefixed_name.data = buf; - prefixed_name.len = ngx_sprintf(buf, "pw.%V.%V", filter_name, &name) + prefixed_name.len = ngx_sprintf(buf, "pw:%V:%V", filter_name, &name) - buf; rc = ngx_wa_metrics_define(metrics, &prefixed_name, type, NULL, 0, id); diff --git a/t/04-openresty/ffi/shm/020-metrics_define.t b/t/04-openresty/ffi/shm/020-metrics_define.t index c1393cf3a..966bb06ff 100644 --- a/t/04-openresty/ffi/shm/020-metrics_define.t +++ b/t/04-openresty/ffi/shm/020-metrics_define.t @@ -40,10 +40,10 @@ __DATA__ ok --- error_log eval [ - qr/.*? \[debug\] .*? defined counter "lua.c1" with id \d+/, - qr/.*? \[debug\] .*? defined gauge "lua.g1" with id \d+/, - qr/.*? \[debug\] .*? defined histogram "lua.h1" with id \d+/, - qr/.*? \[debug\] .*? defined histogram "lua.ch1" with id \d+/, + qr/.*? \[debug\] .*? defined counter "lua:c1" with id \d+/, + qr/.*? \[debug\] .*? defined gauge "lua:g1" with id \d+/, + qr/.*? \[debug\] .*? defined histogram "lua:h1" with id \d+/, + qr/.*? \[debug\] .*? defined histogram "lua:ch1" with id \d+/, ] --- no_error_log [error] diff --git a/t/04-openresty/ffi/shm/025-metrics_get_by_name.t b/t/04-openresty/ffi/shm/025-metrics_get_by_name.t index ff1f706ba..76943ef27 100644 --- a/t/04-openresty/ffi/shm/025-metrics_get_by_name.t +++ b/t/04-openresty/ffi/shm/025-metrics_get_by_name.t @@ -12,7 +12,7 @@ run_tests(); __DATA__ === TEST 1: shm_metrics - get_by_name() sanity FFI-defined metrics -prefix: lua.* +colon-separated prefix: "lua:*" --- valgrind --- metrics: 16k --- config @@ -51,7 +51,7 @@ ch1: {sum=2,type="histogram",value={{count=0,ub=1},{count=1,ub=3},{count=0,ub=5} === TEST 2: shm_metrics - get_by_name() sanity non-FFI-defined metrics -prefix: pw.hostcalls.* +colon-separated prefix: "pw:hostcalls:*" --- wasm_modules: hostcalls --- config eval my $record_histograms; @@ -84,9 +84,9 @@ qq{ local shm = require "resty.wasmx.shm" local pretty = require "pl.pretty" - ngx.say("c1: ", pretty.write(shm.metrics:get_by_name("pw.hostcalls.c1", { prefix = false }), "")) - ngx.say("g1: ", pretty.write(shm.metrics:get_by_name("pw.hostcalls.g1", { prefix = false }), "")) - ngx.say("h1: ", pretty.write(shm.metrics:get_by_name("pw.hostcalls.h1", { prefix = false }), "")) + ngx.say("c1: ", pretty.write(shm.metrics:get_by_name("pw:hostcalls:c1", { prefix = false }), "")) + ngx.say("g1: ", pretty.write(shm.metrics:get_by_name("pw:hostcalls:g1", { prefix = false }), "")) + ngx.say("h1: ", pretty.write(shm.metrics:get_by_name("pw:hostcalls:h1", { prefix = false }), "")) } } } diff --git a/t/04-openresty/ffi/shm/026-metrics_iterate_keys.t b/t/04-openresty/ffi/shm/026-metrics_iterate_keys.t index e0cf4b3cd..6011f5393 100644 --- a/t/04-openresty/ffi/shm/026-metrics_iterate_keys.t +++ b/t/04-openresty/ffi/shm/026-metrics_iterate_keys.t @@ -32,9 +32,9 @@ __DATA__ } } --- response_body -lua.c1 -lua.g1 -lua.h1 +lua:c1 +lua:g1 +lua:h1 --- no_error_log [error] [crit] diff --git a/t/04-openresty/ffi/shm/027-metrics_get_keys.t b/t/04-openresty/ffi/shm/027-metrics_get_keys.t index 69513503f..dbb8f8cee 100644 --- a/t/04-openresty/ffi/shm/027-metrics_get_keys.t +++ b/t/04-openresty/ffi/shm/027-metrics_get_keys.t @@ -30,9 +30,9 @@ __DATA__ } } --- response_body -lua.c1 -lua.g1 -lua.h1 +lua:c1 +lua:g1 +lua:h1 --- no_error_log [error] [crit]