update(userspace): define subtable field type in plugin API and sinsp state API #1849
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…API version Co-authored-by: Gianmatteo Palmieri <[email protected]> Signed-off-by: Jason Dellaluce <[email protected]>
…dapt to new plugin API Co-authored-by: Gianmatteo Palmieri <[email protected]> Signed-off-by: Jason Dellaluce <[email protected]>
poiana
added
kind/feature
incertum
approved these changes
May 7, 2024
There was a problem hiding this comment.
/approve
Thanks @mrgian and @jasondellaluce. Yes ok to add the tests in the next PR.
|
LGTM label has been added. Git tree hash: 8c23de69c48c28cd2595de76cb86214148949db1
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: FedeDP, incertum, jasondellaluce The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind feature
Any specific area of the project related to this PR?
/area libsinsp
Does this PR require a change in the driver versions?
What this PR does / why we need it:
Second contribution step of #1712 (comment). This PR introduces the notion of "substable" in the state field types supported by libsinsp and the plugin API.
By subtable, here we mean a state tables that are owned by the entries of another table. The first use case we have is the file descriptor tables, which in our model are owned by individual thread infos (that are part of the threads table). File descriptor tables will be tables with an
int64_tkey and entries represented bysinsp_fdinfo. Imitating other weakly-typed languages (e.g. JavaScript), subtables also give support to representing map types (e.g. unordered_map/map) and arrays (e.g. std::array, std::vector, std::list) by representing them as maps with non-sparse integer key representing the array index. This is already achieved in my personal branches, and allows sharing threadinfo data such as the env variables and proc args (bothstd::vector<std::string>).The approach consists in adding (both in sinsp and in the plugin API) a new state field type representing a pointer to another table, in sinsp represented as
libsinsp::state::base_table*and in the plugin API as the opaquess_plugin_table_t*. Just like any other field type of the state API, the table entry that owns it is responsible of owning its allocated memory and managing its lifecycle. In order to obtain accessors to a subtable at initialization time (specially in plugins), users might need to create an entry of a table (e.g. a threadinfo) and access its subtable (e.g. the fdtable) just for the purpose of listing/accessing its fields. Once an accessor is obtained, it can be reused for future accesses to the same kind of tables as the API already allows. For the plugin API, this involves passing the table reader and writer vtables at initialization time. The complex part is at the plumbing level, in which we make the libsinsp and plugin API layer communicate -- this part has been put under stress to avoid any unnecessary allocation and/or memory leaks.Due to the lack of use case, and the inherent complexity of supporting it, at the moment it's not possible to define subtables as write-mode dynamic fields (but that's a possible future use case). For similar reasons, the initial shared subtables (e.g. the fd tables) will not be substitutable and will be made read only (their content will be modifiable though).
Which issue(s) this PR fixes:
Special notes for your reviewer:
The PR does not include tests due to it being large already. I implemented well-covering tests using the file descriptor subtable as a example use case, both for the sinsp API an the plugin API. However, given the size of that part too, I thought it would be overkill for reviewer to include everything here. So if reviewers are ok with it, tests on this part will be added in the follow-up PR I have ready for after this one.
Since work was needed on it, this also polishes a little the contents of
plugin_table_api.cpp. Also, this solves a minor bug in therelease_table_entryfunctionality -- the plugin is now owner of shared pointers passed by sinsp and respect the reference counting model.As for the benchmarks and tests I've run, performance in the hot path should not be impacted for all code not using this feature. Also, the plugin API should not have any breaking change introduced and should keep being backward-compatible.
/milestone 0.17.0
Does this PR introduce a user-facing change?: