fix(plugin_api): wrong metric type enums #1885
Conversation
poiana
added
release-note
kind/bug
|
LGTM label has been added. Git tree hash: f189d657c285f57d24903f6f96932698dd4f29bf
|
|
/milestone 0.18.0 |
| SS_PLUGIN_METRIC_VALUE_TYPE_U32 = 0, | ||
| SS_PLUGIN_METRIC_VALUE_TYPE_S32 = 1, | ||
| SS_PLUGIN_METRIC_VALUE_TYPE_U64 = 2, | ||
| SS_PLUGIN_METRIC_VALUE_TYPE_S64 = 3, | ||
| SS_PLUGIN_METRIC_VALUE_TYPE_D = 4, | ||
| SS_PLUGIN_METRIC_VALUE_TYPE_F = 5, | ||
| SS_PLUGIN_METRIC_VALUE_TYPE_I = 6, |
There was a problem hiding this comment.
Technically, this is a backward incompatible change, isn't it?
So, unfortunately, I believe we have to bump the PLUGIN_API_VERSION_MAJOR number.
cc @jasondellaluce
/hold a bit for second opinions
There was a problem hiding this comment.
Another option would be:
Since this couldn't be used before (because of the missing registered symbol), we assume the 3.5.0 did not have this feature at all. Thus, just bumping the PLUGIN_API_VERSION_MINOR would be enough.
There was a problem hiding this comment.
agree @leogr we can consider this feature not yet usable in the current release.
There was a problem hiding this comment.
Yet another option:
Forcing the enum values in metrics_v2.h (https://github.com/falcosecurity/libs/blob/master/userspace/libscap/metrics_v2.h) to match the enums in plugin_types.h
This should work given that these enums are only casted here https://github.com/falcosecurity/libs/blob/master/userspace/libsinsp/plugin.cpp#L947-L948
There was a problem hiding this comment.
Agree with Leo, since it was unusable in 0.17.1, we can consider a minor bump.
Signed-off-by: Gianmatteo Palmieri <mail@gian.im>
Signed-off-by: Gianmatteo Palmieri <mail@gian.im>
Signed-off-by: Gianmatteo Palmieri <mail@gian.im>
mrgian
force-pushed
the
fix-plugin-api-metrics
branch
from
50805d1
to
e9cd426
Compare
|
Bumped |
| * plugins metrics | ||
| */ | ||
|
|
||
| if(m_metrics_flags & METRICS_V2_PLUGINS) |
There was a problem hiding this comment.
If we insert plugin metrics first in the vector, they then get overwritten by scap/sinsp metrics.
I didn't look closely at it, but the cause may be: https://github.com/falcosecurity/libs/blob/master/userspace/libsinsp/metrics_collector.cpp#L487
|
LGTM label has been added. Git tree hash: 8226f3453afceed9b4598a717941899147df60ab
|
|
/milestone 0.17.2 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: FedeDP, incertum, jasondellaluce, mrgian 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 bug
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:
Fixes wrong metric type enums, which are used to convert plugin metrics to libsinsp metrics.
It also resolves
get_metricssymbol in plugins.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: