-
Notifications
You must be signed in to change notification settings - Fork 165
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
Clean up the table API #2193
base: master
Are you sure you want to change the base?
Clean up the table API #2193
Conversation
Perf diff from master - unit tests
Heap diff from master - unit tests
Heap diff from master - scap file
Benchmarks diff from master
|
702c074
to
e3b0851
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2193 +/- ##
==========================================
+ Coverage 75.19% 75.37% +0.18%
==========================================
Files 261 262 +1
Lines 33878 33774 -104
Branches 5801 5752 -49
==========================================
- Hits 25475 25458 -17
+ Misses 8403 8316 -87
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
4e6ec6a
to
a5989b6
Compare
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
This is an intermediate class in the hierarchy, that built-in sinsp tables will inherit from, but plugin-provided table wrappers won't. Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
The API defined by libsinsp::state::base_table is about to be removed (moved to libsinsp::state::built_in_table), so we introduce a new API that's available for every table (including plugin-provided tables), using the plugin table API underneath. Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
Avoid a naming conflict with sinsp_thread_manager::clear. Signed-off-by: Grzegorz Nosek <[email protected]>
Signed-off-by: Grzegorz Nosek <[email protected]>
a5989b6
to
7989c78
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
I went commit by commit as suggested, everything makes sense and LGTM!
Great job @gnosek !
LGTM label has been added. Git tree hash: 3498cc14f4ef2e91e04cb145cfed7caa7e5da69f
|
/milestone 0.20.0 |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: FedeDP, gnosek 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 |
What type of PR is this?
/kind cleanup
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:
Today we have two APIs to access tables from sinsp (core or plugin) code:
Unfortunately, adapting plugin tables to the C++ API is awkward and complex, which makes extending either API harder than it could be.
The goal of this PR is to separate the C++ API as implemented by the built in tables from the C++ API as used by table consumers, so that the consumer API is based only on the C-style plugin API.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
There's quite a bit of churn here, so it's probably best to review commit by commit. Some of the later commits are semi-related cleanups (like the typeinfo changes) that let us remove some code duplication. I understand we wanted to keep state support sort of separate from plugin support, but in the end the two are intertwined enough that any real separation between them is fictional.
In the grand tradition of libs, the C++ api is left undocumented ;) We might want to pull it out to a more separate place too (the C++ SDK would be perfect IMHO, except it would introduce a circular dependency with sinsp)
Does this PR introduce a user-facing change?: