feat: add events dimensions file generator in modern probe #2232
Conversation
poiana
added
release-note-none
kind/feature
ekoops
changed the title
Perf diff from master - unit testsHeap diff from master - unit testsHeap diff from master - scap fileBenchmarks diff from master |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2232 +/- ##
==========================================
+ Coverage 75.09% 75.12% +0.02%
==========================================
Files 276 276
Lines 34391 34400 +9
Branches 5927 5921 -6
==========================================
+ Hits 25826 25843 +17
+ Misses 8565 8557 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
/milestone 0.21.0 |
ekoops
force-pushed
the
ekoops/events-dimensions-gen
branch
from
5752742 to
95c1ee8
Compare
|
Please double check driver/API_VERSION file. See versioning. /hold |
ekoops
force-pushed
the
ekoops/events-dimensions-gen
branch
from
95c1ee8 to
6780255
Compare
ekoops
changed the title
There was a problem hiding this comment.
Shouldn't we generate and commit the new file in this PR?
There was a problem hiding this comment.
The new file is produced at build time, that's why I deleted it. In the previous attempt, I included the generated file in the source, but since it required to be properly formatted (as per clang-format configuration), it required clang-format to be installed on the system. I assume that we don't want to be dependent on a development tool at build time.
There was a problem hiding this comment.
what about disabling formatting on this file since it is autogenerated?
There was a problem hiding this comment.
Yes it works for me. I'll let clang ignore it
There was a problem hiding this comment.
Mmmh i'd completely drop the file really; it is really error prone to let it lingering in the repo since it is auto-generated. I can't see the point in keeping it.
There was a problem hiding this comment.
I think it can help to quickly detect and track variation in the generation behavior, as well as event size variations.
ekoops
force-pushed
the
ekoops/events-dimensions-gen
branch
from
6780255 to
3457a5a
Compare
|
LGTM! just a compilation error in CI https://github.com/falcosecurity/libs/actions/runs/12868886761/job/35876631053?pr=2232 |
Signed-off-by: Leonardo Di Giovanna <[email protected]>
ekoops
force-pushed
the
ekoops/events-dimensions-gen
branch
from
3457a5a to
344e6e1
Compare
|
LGTM label has been added. Git tree hash: 5999e26a71acfe740cf27b2d7c0f77d7441c44ba
|
|
/hold |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Andreagit97, ekoops, FedeDP 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 |
|
/unhold |
What type of PR is this?
/kind feature
Any specific area of the project related to this PR?
/area build
/area driver-modern-bpf
Does this PR require a change in the driver versions?
What this PR does / why we need it:
This PR enables automatic generation of
driver/modern_bpf/definitions/events_dimensions.h. The new generator, defined indriver/modern_bpf/definitions/generator/generator.cpp, is called by the build system if any modification to the event table is detected. The events dimensions file has been added as a separate dependency to the bpf object file targets.Which issue(s) this PR fixes:
#2227
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: