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.

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

Only filter by app & title keys by default #100

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
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
14 changes: 10 additions & 4 deletions aw_transform/classify.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class Rule:
ignore_case: bool

def __init__(self, rules: Dict[str, Any]):
self.select_keys = rules.get("select_keys", None)
self.select_keys = rules.get("select_keys", ["app", "title"])
Copy link
Member

Choose a reason for hiding this comment

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

This would be better as a default when creating a new rule in the frontend, not made an implicit default in the transform imo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't look like there is an existing component we can use for a multiselect/array form input right now. Am I missing something?

Here's what I propose here:

  • Merge this PR to avoid accidental categorization (matching against url could yield bad results)
  • Having an explicit all magic key will be helpful regardless
  • Work on getting a array input + select_keys available on the frontend

Copy link
Member

Choose a reason for hiding this comment

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

It doesn't look like there is an existing component we can use for a multiselect/array form input right now.

No, but should be easy to add. Could even just put a text field, and ask users to comma-separate.

self.ignore_case = rules.get("ignore_case", False)

# NOTE: Also checks that the regex isn't an empty string (which would erroneously match everything)
Expand All @@ -29,10 +29,15 @@ def __init__(self, rules: Dict[str, Any]):
)

def match(self, e: Event) -> bool:
if self.select_keys:
values = [e.data.get(key, None) for key in self.select_keys]
else:
# `data` contains keys like 'app', 'title'
# by default, the rule regex is matched against all values

if self.select_keys == "all":
values = list(e.data.values())
elif self.select_keys:
values = [e.data.get(key, None) for key in self.select_keys]

# although there is a `type` field on the rule name, right now the only valid type is regex
if self.regex:
for val in values:
if isinstance(val, str) and self.regex.search(val):
Expand All @@ -45,6 +50,7 @@ def categorize(events: List[Event], classes: List[Tuple[Category, Rule]]):


def _categorize_one(e: Event, classes: List[Tuple[Category, Rule]]) -> Event:
# coloring is intentionally left to the frontend in order to keep the backend as simple as possible
e.data["$category"] = _pick_category(
[_cls for _cls, rule in classes if rule.match(e)]
)
Expand Down