Improve match logging - #387
Conversation
vinckama
left a comment
vinckama
left a comment
There was a problem hiding this comment.
I added a few comments on the approach. I think the labels could stay generic and be associated to the rule when created. The business logic to select the label can be done upstream. This will allow to limit the impact on the metric cardinality.
| /// Precomputed `"{sensitive_data_category}/{sensitive_data}"` tag value derived from | ||
| /// the rule's `tags`, used to tag `scanning.match_count`. `None` when the rule has no | ||
| /// `sensitive_data` tag. | ||
| pub sds_rule_name: Option<String>, |
There was a problem hiding this comment.
could it be kept generic with "labels" similarly to how these telemetry tags are attached to scanner?
| "sds_rule_name", | ||
| sds_rule_name.to_string(), | ||
| )])); | ||
| counter!("scanning.match_count", labels).increment(count); |
There was a problem hiding this comment.
The cardinality of this metric is already high and it will increase even more with this new tag.
To avoid cardinality issues, these additional labels must be enabled as opt-in when required, for instance for correctness validation.
If labels are forwarded when the rule is instantiated, the business logic to decide if they should be enabled or not could be done outside of the library.
Such logic could be defined in the SDS context, enabled per target and propagated to downstream consumers.
| use metrics::{Counter, counter}; | ||
|
|
||
| /// Looks up the value of a `"key:value"` tag entry by key, matching the format used by | ||
| /// standard rule definitions (e.g. `sensitive_data:travis_ci_access_token`). |
There was a problem hiding this comment.
Minor: there is no concept of standard rules here, and in general the concept of sensitive_data tag is not associated to standard rule but is generic for all kind of rules.
2 participants
The current logging emit a metric for every match, but with almost no metadata.
This makes fleet monitoring challenging since an increase in activity can't be tied to a precise rule, and the performance of a given rule can't easily be isolated.
This change introduce a new
sds_rule_nametag with the category and name of the rule