Skip to content

Commit

Permalink
feat(metrics) switch metric name metadata separator to ':'
Browse files Browse the repository at this point in the history
Switch from `.` to `:` since module names can contain dots, making the
prefix difficult to parse.
  • Loading branch information
casimiro authored and thibaultcha committed Oct 29, 2024
1 parent 3434919 commit 630a5e7
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 25 deletions.
6 changes: 3 additions & 3 deletions docs/METRICS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions lib/resty/wasmx/shm.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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]")
Expand Down Expand Up @@ -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)
Expand All @@ -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 })

Expand Down
2 changes: 1 addition & 1 deletion src/common/proxy_wasm/ngx_proxy_wasm_host.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions t/04-openresty/ffi/shm/020-metrics_define.t
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
10 changes: 5 additions & 5 deletions t/04-openresty/ffi/shm/025-metrics_get_by_name.t
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 }), ""))
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions t/04-openresty/ffi/shm/026-metrics_iterate_keys.t
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ __DATA__
}
}
--- response_body
lua.c1
lua.g1
lua.h1
lua:c1
lua:g1
lua:h1
--- no_error_log
[error]
[crit]
6 changes: 3 additions & 3 deletions t/04-openresty/ffi/shm/027-metrics_get_keys.t
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ __DATA__
}
}
--- response_body
lua.c1
lua.g1
lua.h1
lua:c1
lua:g1
lua:h1
--- no_error_log
[error]
[crit]

0 comments on commit 630a5e7

Please sign in to comment.