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.

Sign up for GitHub

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

Jump to bottom

refactor!(libsinsp): coherent metrics interface metrics_collector class + text-based Prometheus exposition format support #1652

Merged
merged 22 commits into from
Mar 13, 2024
Merged

refactor!(libsinsp): coherent metrics interface metrics_collector class + text-based Prometheus exposition format support #1652

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
09f01ea
update(scap): update scap_stats_v2 schema
incertum Jan 26, 2024
9522254
refactor(libsinsp): new light weight metrics_collector class
incertum Jan 26, 2024
535d592
refactor: naming change stats -> metrics if applicable + complete met…
incertum Jan 26, 2024
3fc1828
refactor(libsinsp): native memory conversion in metrics_v2
incertum Jan 27, 2024
172d958
cleanup(libsinsp): improve code clarity and adopt best practices
incertum Jan 28, 2024
68f0554
new(libsinsp): add convert_metric_to_prometheus_text to metrics_colle…
incertum Jan 29, 2024
40ebff3
cleanup(libsinsp): apply reviewers suggestions
incertum Jan 30, 2024
8000c0c
cleanup(libsinsp): metrics text - pass by string_view
incertum Jan 30, 2024
94b1d22
cleanup(libsinsp): expand convert_metric_to_prometheus_text
incertum Feb 2, 2024
99c6ebb
update: introduce new rule counters metrics category macro
incertum Feb 2, 2024
44b1b3c
refactor(libsinsp): convert_metric_to_prom_text follow some best prac…
incertum Feb 7, 2024
289c63b
chore: revert some changes to get_sinsp_stats_v2
incertum Feb 7, 2024
24d01f4
cleanup(libsinp): improve prometheus format conversion correctness
incertum Feb 8, 2024
a5e9b50
refactor!(libsinsp/metrics): new metrics_converter subclasses complem…
incertum Feb 11, 2024
bcb3368
cleanup(libsinsp/metrics_collector): const correction
incertum Feb 22, 2024
a2b4e1c
chore: adopt new get thread_manager style in metrics
incertum Feb 22, 2024
b5a4831
new(libsinsp/metrics): implement Prometheus convert_metric_to_unit_co…
incertum Mar 7, 2024
f034066
cleanup(libsinsp/metrics): apply reviewers suggestions + cleanup
incertum Mar 7, 2024
9c482ff
cleanup(libsinsp/metrics): apply reviewers suggestions
incertum Mar 7, 2024
dbfcfad
chore: use cerr information prints in metrics unit tests
incertum Mar 8, 2024
33a34ca
cleanup(metrics): code deduplication and simplification
incertum Mar 8, 2024
2c3092e
cleanup(metrics): apply reviewers suggestions
incertum Mar 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions userspace/libpman/src/stats.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ struct metrics_v2 *pman_get_metrics_v2(uint32_t flags, uint32_t *nstats, int32_t
g_state.stats[stat].type = METRIC_VALUE_TYPE_U64;
g_state.stats[stat].flags = METRICS_V2_KERNEL_COUNTERS;
g_state.stats[stat].unit = METRIC_VALUE_UNIT_COUNT;
g_state.stats[stat].metric_type = METRIC_VALUE_MONOTONIC;
g_state.stats[stat].metric_type = METRIC_VALUE_METRIC_TYPE_MONOTONIC;
g_state.stats[stat].value.u64 = 0;
strlcpy(g_state.stats[stat].name, modern_bpf_kernel_counters_stats_names[stat], METRIC_NAME_MAX);
}
Expand Down Expand Up @@ -258,26 +258,27 @@ struct metrics_v2 *pman_get_metrics_v2(uint32_t flags, uint32_t *nstats, int32_t
strlcat(g_state.stats[offset].name, modern_bpf_libbpf_stats_names[RUN_CNT], sizeof(g_state.stats[offset].name));
g_state.stats[stat].flags = METRICS_V2_KERNEL_COUNTERS;
FedeDP marked this conversation as resolved.
Show resolved Hide resolved
g_state.stats[stat].unit = METRIC_VALUE_UNIT_COUNT;
g_state.stats[stat].metric_type = METRIC_VALUE_MONOTONIC;
g_state.stats[stat].metric_type = METRIC_VALUE_METRIC_TYPE_MONOTONIC;
g_state.stats[offset].value.u64 = info.run_cnt;
break;
case RUN_TIME_NS:
strlcat(g_state.stats[offset].name, modern_bpf_libbpf_stats_names[RUN_TIME_NS], sizeof(g_state.stats[offset].name));
g_state.stats[stat].unit = METRIC_VALUE_UNIT_TIME_NS_COUNT;
g_state.stats[stat].metric_type = METRIC_VALUE_MONOTONIC;
g_state.stats[stat].metric_type = METRIC_VALUE_METRIC_TYPE_MONOTONIC;
g_state.stats[offset].value.u64 = info.run_time_ns;
break;
case AVG_TIME_NS:
strlcat(g_state.stats[offset].name, modern_bpf_libbpf_stats_names[AVG_TIME_NS], sizeof(g_state.stats[offset].name));
g_state.stats[stat].unit = METRIC_VALUE_UNIT_TIME_NS;
g_state.stats[stat].metric_type = METRIC_VALUE_NON_MONOTONIC_CURRENT;
g_state.stats[stat].metric_type = METRIC_VALUE_METRIC_TYPE_NON_MONOTONIC_CURRENT;
g_state.stats[offset].value.u64 = 0;
if(info.run_cnt > 0)
{
g_state.stats[offset].value.u64 = info.run_time_ns / info.run_cnt;
}
break;
default:
ASSERT(false);
break;
}
offset++;
Expand Down
9 changes: 5 additions & 4 deletions userspace/libscap/engine/bpf/scap_bpf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1731,7 +1731,7 @@ const struct metrics_v2* scap_bpf_get_stats_v2(struct scap_engine_handle engine,
{
stats[stat].type = METRIC_VALUE_TYPE_U64;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I start to think that an helper method to fill stats would be a good addition (perhaps even in a followup PR), like:

fill_stats(&stats[stat], type, value, unit, metric_type, flags);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that there is the new_metric method, perhaps we could use it in other places too, outside of metrics_collector?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracking it here #1463 (comment)

The new_metric method is a template in CPP. In addition for the scap stats for example we have inner loops where we only update the value, e.g. see

libs/userspace/libscap/engine/bpf/scap_bpf.c

Line 1738 in 5dc692e

for(int cpu = 0; cpu < handle->m_ncpus; cpu++)
when we iterate over the CPUs. Therefore a bit on the fence as I see the value of it being nicer from a software organization point of view, but the current approach seems faster.

Let's think more about it? I believe the real dilemma of metrics is that one part is in C while the other part is in CPP, so it's difficult to find a shared approach that would follow best practices either way. I'll repeat this in my next response below. That's also why I use the arrays approach with the metrics names as that's how we do it in scap / C. I am still not sure, how a really great metrics framework that spans scap and sinsp + pending falco rules counts metrics category should look like frankly.

Copy link
Contributor

@FedeDP FedeDP Mar 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the real dilemma of metrics is that one part is in C while the other part is in CPP

Yep, you are right, i thought about this thing later on. I am not sure how (and if we can) to proceed, i agree to "save the idea" for later; let's see if anyone comes with a good solution.

stats[stat].flags = METRICS_V2_KERNEL_COUNTERS;
stats[stat].metric_type = METRIC_VALUE_MONOTONIC;
stats[stat].metric_type = METRIC_VALUE_METRIC_TYPE_MONOTONIC;
stats[stat].unit = METRIC_VALUE_UNIT_COUNT;
stats[stat].value.u64 = 0;
strlcpy(stats[stat].name, bpf_kernel_counters_stats_names[stat], METRIC_NAME_MAX);
Expand Down Expand Up @@ -1828,25 +1828,26 @@ const struct metrics_v2* scap_bpf_get_stats_v2(struct scap_engine_handle engine,
strlcat(stats[offset].name, bpf_libbpf_stats_names[RUN_CNT], sizeof(stats[offset].name));
stats[offset].value.u64 = info.run_cnt;
stats[offset].unit = METRIC_VALUE_UNIT_COUNT;
stats[offset].metric_type = METRIC_VALUE_MONOTONIC;
stats[offset].metric_type = METRIC_VALUE_METRIC_TYPE_MONOTONIC;
break;
case RUN_TIME_NS:
strlcat(stats[offset].name, bpf_libbpf_stats_names[RUN_TIME_NS], sizeof(stats[offset].name));
stats[offset].value.u64 = info.run_time_ns;
stats[offset].unit = METRIC_VALUE_UNIT_TIME_NS_COUNT;
stats[offset].metric_type = METRIC_VALUE_MONOTONIC;
stats[offset].metric_type = METRIC_VALUE_METRIC_TYPE_MONOTONIC;
break;
case AVG_TIME_NS:
strlcat(stats[offset].name, bpf_libbpf_stats_names[AVG_TIME_NS], sizeof(stats[offset].name));
stats[offset].value.u64 = 0;
stats[offset].unit = METRIC_VALUE_UNIT_TIME_NS;
stats[offset].metric_type = METRIC_VALUE_NON_MONOTONIC_CURRENT;
stats[offset].metric_type = METRIC_VALUE_METRIC_TYPE_NON_MONOTONIC_CURRENT;
if (info.run_cnt > 0)
{
stats[offset].value.u64 = info.run_time_ns / info.run_cnt;
}
break;
default:
ASSERT(false);
break;
}
offset++;
Expand Down
2 changes: 1 addition & 1 deletion userspace/libscap/engine/gvisor/scap_gvisor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ const metrics_v2* engine::get_stats_v2(uint32_t flags, uint32_t* nstats, int32_t
{
stats[stat].type = METRIC_VALUE_TYPE_U64;
stats[stat].unit = METRIC_VALUE_UNIT_COUNT;
stats[stat].metric_type = METRIC_VALUE_MONOTONIC;
stats[stat].metric_type = METRIC_VALUE_METRIC_TYPE_MONOTONIC;
stats[stat].value.u64 = 0;
strlcpy(stats[stat].name, gvisor_counters_stats_names[stat], METRIC_NAME_MAX);
}
Expand Down
2 changes: 1 addition & 1 deletion userspace/libscap/engine/kmod/scap_kmod.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ const struct metrics_v2* scap_kmod_get_stats_v2(struct scap_engine_handle engine
stats[stat].type = METRIC_VALUE_TYPE_U64;
stats[stat].flags = METRICS_V2_KERNEL_COUNTERS;
stats[stat].unit = METRIC_VALUE_UNIT_COUNT;
stats[stat].metric_type = METRIC_VALUE_MONOTONIC;
stats[stat].metric_type = METRIC_VALUE_METRIC_TYPE_MONOTONIC;
stats[stat].value.u64 = 0;
strlcpy(stats[stat].name, kmod_kernel_counters_stats_names[stat], METRIC_NAME_MAX);
}
Expand Down
2 changes: 1 addition & 1 deletion userspace/libscap/engine/source_plugin/source_plugin.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ const struct metrics_v2* get_source_plugin_stats_v2(struct scap_engine_handle en
stats[stat].type = METRIC_VALUE_TYPE_U64;
stats[stat].value.u64 = 0;
stats[stat].unit = METRIC_VALUE_UNIT_COUNT;
stats[stat].metric_type = METRIC_VALUE_MONOTONIC;
stats[stat].metric_type = METRIC_VALUE_METRIC_TYPE_MONOTONIC;
strlcpy(stats[stat].name, source_plugin_counters_stats_names[stat], METRIC_NAME_MAX);
}
stats[N_EVTS].value.u64 = handle->m_nevts;
Expand Down
7 changes: 5 additions & 2 deletions userspace/libscap/metrics_v2.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ typedef enum metrics_v2_value_type{
METRIC_VALUE_TYPE_D,
METRIC_VALUE_TYPE_F,
METRIC_VALUE_TYPE_I,
METRIC_VALUE_TYPE_MAX,
} metrics_v2_value_type;

typedef enum metrics_v2_value_unit{
Expand All @@ -72,11 +73,13 @@ typedef enum metrics_v2_value_unit{
METRIC_VALUE_UNIT_TIME_NS_COUNT,
METRIC_VALUE_UNIT_TIME_S_COUNT,
METRIC_VALUE_UNIT_TIME_TIMESTAMP_NS,
METRIC_VALUE_UNIT_MAX,
} metrics_v2_value_unit;

typedef enum metrics_v2_metric_type{
METRIC_VALUE_MONOTONIC,
METRIC_VALUE_NON_MONOTONIC_CURRENT,
METRIC_VALUE_METRIC_TYPE_MONOTONIC,
METRIC_VALUE_METRIC_TYPE_NON_MONOTONIC_CURRENT,
METRIC_VALUE_METRIC_TYPE_MAX,
} metrics_v2_metric_type;

/*!
Expand Down
Loading
Loading