Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ffi) expose kv & metrics shms through FFI #582

Closed
wants to merge 8 commits into from

Conversation

casimiro
Copy link
Contributor

@casimiro casimiro commented Aug 8, 2024

No description provided.

@casimiro casimiro force-pushed the feat/metrics_lua_interface branch from a1b9043 to 5384812 Compare August 8, 2024 14:36
Copy link

codecov bot commented Aug 8, 2024

Codecov Report

Attention: Patch coverage is 76.79558% with 84 lines in your changes missing coverage. Please review.

Project coverage is 62.97581%. Comparing base (96b4e27) to head (12dd92b).
Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
src/common/proxy_wasm/ngx_proxy_wasm_host.c 42.85714% 28 Missing ⚠️
src/common/shm/ngx_wa_shm_kv.c 71.64179% 19 Missing ⚠️
src/common/shm/ngx_wa_shm_queue.c 26.92308% 19 Missing ⚠️
src/common/metrics/ngx_wa_metrics.c 76.08696% 11 Missing ⚠️
src/common/metrics/ngx_wa_histogram.c 68.42105% 6 Missing ⚠️
src/common/shm/ngx_wa_shm.c 96.42857% 1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (96b4e27) and HEAD (12dd92b). Click for more details.

HEAD has 14 uploads less than BASE
Flag BASE (96b4e27) HEAD (12dd92b)
valgrind 5 1
unit 10 0
Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                  @@
##                main        #582          +/-   ##
====================================================
- Coverage   90.60900%   62.97581%   -27.63319%     
====================================================
  Files             49          49                  
  Lines          10936        9510        -1426     
====================================================
- Hits            9909        5989        -3920     
- Misses          1027        3521        +2494     
Files with missing lines Coverage Δ
src/common/lua/ngx_wasm_lua_ffi.c 94.05405% <100.00000%> (+1.13370%) ⬆️
src/common/lua/ngx_wasm_lua_ffi.h 100.00000% <100.00000%> (ø)
src/common/metrics/ngx_wa_metrics.h 100.00000% <100.00000%> (ø)
src/common/shm/ngx_wa_shm.h 100.00000% <100.00000%> (ø)
src/ngx_wasmx.c 82.78689% <100.00000%> (-6.60706%) ⬇️
src/wasm/ngx_wasm.h 90.90909% <ø> (-9.09091%) ⬇️
src/wasm/ngx_wasm_core_module.c 97.14286% <ø> (+4.49580%) ⬆️
src/wasm/ngx_wasm_directives.c 35.08772% <100.00000%> (-61.42970%) ⬇️
src/common/shm/ngx_wa_shm.c 95.83333% <96.42857%> (ø)
src/common/metrics/ngx_wa_histogram.c 67.30769% <68.42105%> (-31.78323%) ⬇️
... and 4 more

... and 37 files with indirect coverage changes

Flag Coverage Δ
unit ?
valgrind 62.97581% <76.79558%> (-19.08449%) ⬇️

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

@casimiro casimiro force-pushed the feat/metrics_lua_interface branch from 5384812 to 78097bb Compare August 8, 2024 14:56
@casimiro casimiro force-pushed the feat/metrics_lua_interface branch 2 times, most recently from f79dad6 to 239bc8c Compare August 8, 2024 22:19
@casimiro casimiro added the pr/work-in-progress PR: Work in progress label Aug 8, 2024
src/ngx_wasmx.h Outdated Show resolved Hide resolved
@casimiro casimiro force-pushed the feat/metrics_lua_interface branch 2 times, most recently from ae9316e to dc68e4a Compare August 12, 2024 21:14
lib/resty/wasmx/shm.lua Outdated Show resolved Hide resolved
lib/resty/wasmx/shm.lua Outdated Show resolved Hide resolved
lib/resty/wasmx/shm.lua Outdated Show resolved Hide resolved
lib/resty/wasmx/shm.lua Outdated Show resolved Hide resolved
lib/resty/wasmx/shm.lua Outdated Show resolved Hide resolved
lib/resty/wasmx/shm.lua Outdated Show resolved Hide resolved
lib/resty/wasmx/shm.lua Show resolved Hide resolved
lib/resty/wasmx/shm.lua Show resolved Hide resolved
lib/resty/wasmx/shm.lua Outdated Show resolved Hide resolved
lib/resty/wasmx/shm.lua Outdated Show resolved Hide resolved
src/common/lua/ngx_wasm_lua_ffi.c Outdated Show resolved Hide resolved
src/common/lua/ngx_wasm_lua_ffi.c Outdated Show resolved Hide resolved
src/common/lua/ngx_wasm_lua_ffi.c Outdated Show resolved Hide resolved
src/common/lua/ngx_wasm_lua_ffi.c Outdated Show resolved Hide resolved
src/common/metrics/ngx_wa_metrics.h Show resolved Hide resolved
src/common/metrics/ngx_wa_metrics.h Outdated Show resolved Hide resolved
@thibaultcha thibaultcha changed the title feat(shm/ffi) expose kv/metrics read-only operations to Lua land feat(ffi) expose kv & metrics shms through FFI Aug 13, 2024
@casimiro casimiro force-pushed the feat/metrics_lua_interface branch from dc68e4a to 698a670 Compare August 13, 2024 19:59
lib/resty/wasmx/shm.lua Outdated Show resolved Hide resolved
lib/resty/wasmx/shm.lua Outdated Show resolved Hide resolved
@casimiro casimiro force-pushed the feat/metrics_lua_interface branch from 698a670 to 3feee24 Compare August 14, 2024 14:29
lib/resty/wasmx/shm.lua Outdated Show resolved Hide resolved
lib/resty/wasmx/shm.lua Outdated Show resolved Hide resolved
lib/resty/wasmx/shm.lua Outdated Show resolved Hide resolved
lib/resty/wasmx/shm.lua Outdated Show resolved Hide resolved
@casimiro casimiro force-pushed the feat/metrics_lua_interface branch 5 times, most recently from f3cb5a7 to b53f649 Compare August 21, 2024 16:06
@casimiro casimiro force-pushed the feat/metrics_lua_interface branch 3 times, most recently from fa15534 to 090e322 Compare August 22, 2024 15:55
@casimiro casimiro force-pushed the feat/metrics_lua_interface branch 4 times, most recently from a288360 to eb0e552 Compare August 27, 2024 12:17
@casimiro casimiro removed the pr/work-in-progress PR: Work in progress label Aug 28, 2024
@casimiro casimiro force-pushed the feat/metrics_lua_interface branch 4 times, most recently from 3d4a233 to bdee12d Compare August 31, 2024 14:11
docs/PROXY_WASM.md Outdated Show resolved Hide resolved
@casimiro casimiro force-pushed the feat/metrics_lua_interface branch from bdee12d to 728c175 Compare September 3, 2024 11:32
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.
After changing the metrics library to rely on shm facilities to store its
shm zone, support for the IPC subsystem in shm became necessary.
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.
@casimiro casimiro force-pushed the feat/metrics_lua_interface branch from 728c175 to eef9838 Compare September 3, 2024 15:32
@thibaultcha thibaultcha added the pr/merge-in-progress PR: Merge in progress (do not push) label Sep 4, 2024
@thibaultcha thibaultcha deleted the feat/metrics_lua_interface branch September 20, 2024 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/merge-in-progress PR: Merge in progress (do not push)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants