Report: Add infrastructure for Perf Stat rules #244
Conversation
|
I recommend thinking about how to do this such that it doesn't require explicit on-boarding for each data type to support analytics rules. The data they output should be self-describing enough that aggregates can be formed generically. |
I prefer things are explicit. It will help when others are looking to contribute rules. |
| let all_rules: Rules[] = [ | ||
| system_info_rules, | ||
| cpu_utilization_rules, | ||
| perf_stat_rules, |
There was a problem hiding this comment.
Are we planning to have all the rules defined in the code or give also the user the possibility to add/configure/change them?
There was a problem hiding this comment.
Currently we want all the rules to be in code.
There was a problem hiding this comment.
It'd be beneficial if this was somehow discoverable instead of needing it to be added explicitly for every data source.
| name: "ipc", | ||
| per_run_rule: function* (ruleOpts: RuleOpts) : Generator<Finding, void, any> { | ||
| let diff = percent_difference(ruleOpts.base_run_data, ruleOpts.this_run_data); | ||
| if (diff > 10) { |
There was a problem hiding this comment.
do we want this % to be fixed?
There was a problem hiding this comment.
Currently yes. Maybe once folks use it and get back with details about other units we can change it.
| return key_value_map; | ||
| } | ||
|
|
||
| function percent_difference(v1, v2) { |
There was a problem hiding this comment.
v1 and v2 should be typed as number correct? This function should also return a number.
| } | ||
|
|
||
| fn get_values(values: Vec<PerfStat>, key: String) -> Result<String> { | ||
| fn get_values(values: Vec<PerfStat>, key: String, metrics: &mut DataMetrics) -> Result<String> { |
There was a problem hiding this comment.
Since every data source is going to implement something like this, shouldn't we make a rust trait to hold some of the assumed common functionality every data source needs?
| let all_rules: Rules[] = [ | ||
| system_info_rules, | ||
| cpu_utilization_rules, | ||
| perf_stat_rules, |
There was a problem hiding this comment.
It'd be beneficial if this was somehow discoverable instead of needing it to be added explicitly for every data source.
4 participants
This will be followed by infrastructure for other data types.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.