[merge] feat/metrics_lua_interface #591

thibaultcha · 2024-09-19T01:50:20Z

@casimiro Please have a look especially at the wip() commit and let me know if everything is still looking good. I ended up moving the document back to docs/ but not linking anything to it as it's so minor.

codecov · 2024-09-19T02:08:13Z

Codecov Report

Attention: Patch coverage is 96.94656% with 12 lines in your changes missing coverage. Please review.

Project coverage is 90.24501%. Comparing base (2febf47) to head (9e74d2f).
Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
src/common/metrics/ngx_wa_metrics.c	87.27273%	7 Missing ⚠️
src/ngx_wasmx.c	84.21053%	3 Missing ⚠️
src/common/proxy_wasm/ngx_proxy_wasm_host.c	98.14815%	1 Missing ⚠️
src/common/shm/ngx_wa_shm.c	96.55172%	1 Missing ⚠️

Additional details and impacted files

@@                 Coverage Diff                 @@
##                main        #591         +/-   ##
===================================================
- Coverage   90.61814%   90.24501%   -0.37314%     
===================================================
  Files             49          51          +2     
  Lines          10936       10979         +43     
===================================================
- Hits            9910        9908          -2     
- Misses          1026        1071         +45

Files with missing lines	Coverage Δ
src/common/lua/ngx_wasm_lua_ffi.c	`95.89744% <100.00000%> (+2.97709%)`	⬆️
src/common/lua/ngx_wasm_lua_ffi.h	`100.00000% <100.00000%> (ø)`
src/common/metrics/ngx_wa_histogram.c	`99.11504% <100.00000%> (+0.02412%)`	⬆️
src/common/metrics/ngx_wa_metrics.h	`100.00000% <100.00000%> (ø)`
src/common/shm/ngx_wa_shm.h	`100.00000% <100.00000%> (ø)`
src/common/shm/ngx_wa_shm_kv.c	`97.61905% <100.00000%> (ø)`
src/common/shm/ngx_wa_shm_queue.c	`96.49123% <100.00000%> (ø)`
src/wasm/ngx_wasm.h	`100.00000% <ø> (ø)`
src/wasm/ngx_wasm_core_module.c	`94.40000% <ø> (+1.75294%)`	⬆️
src/wasm/ngx_wasm_directives.c	`96.51741% <100.00000%> (ø)`
... and 4 more

... and 6 files with indirect coverage changes

Flag	Coverage Δ
unit	`90.44252% <97.26444%> (+0.11489%)`	⬆️
valgrind	`81.87773% <95.06849%> (-0.30317%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

lib/resty/wasmx/shm.lua

src/common/lua/ngx_wasm_lua_ffi.c

casimiro · 2024-09-19T11:46:15Z

Thank you for improving the PR, @thibaultcha!

I left comments about error handling, locking/unlocking in (get/set)_kv when already locked and a note on optional arguments for iterate_keys().

lib/resty/wasmx/shm.lua

This makes custom blocks injected into `wasm{}` take effect even when `--- wasm_modules` isn't specified (e.g. `--- shm_kv`).

This corrects metrics code to properly use ngx_wasm_shm facilities for shm zone handling.

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.

Iterating a tree in batches without knowing the number of elements in it implies an additional traversal through its elements after the last batch is retrieved. This last traversal returns no elements which signals the end of the iteration. Knowing the number of entries in the tree renders the additional traverse unnecessary, when the last batch is retrieved it can be verified that the tree has been fully iterated.

Co-authored-by: Thibault Charbonnier <[email protected]>

thibaultcha force-pushed the feat/metrics_lua_interface_merge branch 2 times, most recently from b320512 to 7306ebb Compare September 19, 2024 02:00