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

Implement filtering on tags in Filter model #117

Closed
ddabble opened this issue Aug 11, 2020 · 18 comments
Closed

Implement filtering on tags in Filter model #117

ddabble opened this issue Aug 11, 2020 · 18 comments
Labels
discussion Requires developer feedback/discussion before implementation task

Comments

@ddabble
Copy link
Contributor

ddabble commented Aug 11, 2020

A couple ideas:

  • The filter strings could take inspiration from Django's query syntax with double underscores, e.g.:
    • "tags__key": ["nav_problem", "zabbix_problem"]
    • "tags__value": ["Netbox 4", "boxDown"]
      No double underscores would filter on both key AND value:
    • "tags": ["object=Netbox 4", "problem_type=boxDown"]
      And we could potentially also add support for e.g. regex:
    • "tags__value__regex": ["Netbox [0-9]+", "^https://argus\..*"]
  • The filter strings could consist of a dictionary, e.g.:
    • "tags": [
          {
              "key": "nav_problem"
          },
          {
              "key": "zabbix_problem"
          },
      ]
      
    • "tags": [
          {
              "key": "object"
              "value": "Netbox 4"
          },
          {
              "value": "boxDown"
          },
      ]
      
    Or a string instead of a dictionary, which would filter on both key AND value:
    • "tags": ["object=Netbox 4", "problem_type=boxDown"]

Both of these ideas could benefit from being more fleshed out.

@ddabble ddabble added discussion Requires developer feedback/discussion before implementation task labels Aug 11, 2020
@ddabble ddabble added this to To do in Argus Sprint #2 via automation Aug 11, 2020
@hmpf
Copy link
Contributor

hmpf commented Aug 11, 2020

Unless we add a safe regex-editor, we shouldn't allow regexes. It's too easy to create a DoS.

@hmpf
Copy link
Contributor

hmpf commented Aug 11, 2020

Hmmm, something to show how serious the source system considers the problem (severity/priority/whetever) will need to be filterable as well. If that is just single-digit integers it should be easy to make a filter: a list of all levels wanted :)

@hmpf
Copy link
Contributor

hmpf commented Aug 13, 2020

I think the third type is the most viable and what most closely resembles the old system. We could also support:

"tags": ["object=", "=boxDown"]

I'll check the old code for how things worked with explicit object/parent_object/problemtype regarding boolean combinations.

@hmpf
Copy link
Contributor

hmpf commented Aug 13, 2020

What I'd really like to have is that every filter in the same type is an OR while filters of different types is an AND. So that:

{
  "sourceSystemIds": [45, 56, 78],
  "tags": ["object=Netbox 4", "problem_type=boxDown", "object=Switch 1", "problem_type=linkDown"]
}

is interpreted as

a = Q(source__in=[45, 56, 78]) 
b = Q(tags__key="object", tags__value__in=["Netbox 4", "Switch 1"])
c = Q(tags__key="problem_type", tags__value__in=["boxDown", "linkDown"])

qs = Incident.objects.filter(a & b & c)

Thinking some more, being able to filter on tags with ["object=Netbox 4", "problem_type=boxDown", "object=Switch 1", "problem_type=linkDown"] is useful regardless: in admin, in CLI, maybe a management command. It needs a queryset-method.
We might as well have one for source as well.

@ddabble
Copy link
Contributor Author

ddabble commented Aug 13, 2020

I think the third type is the most viable

Wait, which third type? This one? "tags": ["object=Netbox 4", "problem_type=boxDown"] In that case, the original idea was that it was an alternative to the dictionary format.

"tags": ["object=", "=boxDown"]

I think I'd prefer the dictionary format over this one, as it's more verbose, and therefore easier to read and harder to get wrong 😅

What I'd really like to have is that every filter in the same type is an OR while filters of different types is an AND.

Yes, this is already implemented 🙂

@hmpf
Copy link
Contributor

hmpf commented Aug 13, 2020

Not fun sending a dictionary in a get-query though :) Anyway, just being able to filter on complete tags would be great.

@hmpf
Copy link
Contributor

hmpf commented Aug 13, 2020

.. and we need to be able to filter on source type!

@hmpf
Copy link
Contributor

hmpf commented Aug 13, 2020

Source type: On creation of a source type, we create a tag for that source type, with no connected incidents.

It strikes me, we could have a "source" tag as well, hiding that part of the data model from the end user. It would then be a matter of just sending a list of tags when wanting to filter, an array, not a dict.

@ddabble
Copy link
Contributor Author

ddabble commented Aug 13, 2020

Source type: On creation of a source type, we create a tag for that source type, with no connected incidents.

Hm, I'm not really a fan of that idea.. The frontend might very well choose to display it that way, but I think the backend should use what is already implemented, without having to create "symbolic" tags.

When it comes to sending arrays vs dicts, I think it shouldn't really be an argument that it's a "hassle" to send dicts, as the request parameters and bodies are not meant to be manually written anyway.

@hmpf
Copy link
Contributor

hmpf commented Aug 13, 2020

Thinking out loud: There will be very few SourceSystemTypes. There can be potentially very many SourceSystems, objects, parent_objects, problem_types. This will affect the UI. If there is a "tags" column in the UI, the simplest way to make a filter is to click on the tags of a row. It might be relevant to give a subset of tags (like source system type, source?) their own column. This needs some mockups I think.

@hmpf
Copy link
Contributor

hmpf commented Aug 13, 2020

When it comes to sending arrays vs dicts, I think it shouldn't really be an argument that it's a "hassle" to send dicts, as the request parameters and bodies are not meant to be manually written anyway.

Why not allow writing it manually? In a GET I mean.

@hmpf
Copy link
Contributor

hmpf commented Aug 13, 2020

Source type: On creation of a source type, we create a tag for that source type, with no connected incidents.

Hm, I'm not really a fan of that idea.. The frontend might very well choose to display it that way, but I think the backend should use what is already implemented, without having to create "symbolic" tags.

It's a matter of consistency, and using one system for filtering instead of combining several.

Say you have ?tags=sourcetype=mjam,foo=b,bar=c,xux=d,foo=1. You could turn all those into tag querysets and look up which incidents have them in one fell swoop, or you could pop sourcetype from the tags-list, get the tags, get the source type and then recombine. The latter is more complex, everywhere: querysets, api, ui, cli. Querystrings, APIS, class hierarchies are also UIs.

(Or, hm, tags=sourcetype=mjam;foo=b,1;bar=c;xux=d, but the former is trivially easy with a <select multiple>)

@ddabble
Copy link
Contributor Author

ddabble commented Aug 13, 2020

Why not allow writing it manually? In a GET I mean.

Well sure, we can absolutely allow that! I just don't think we should design it around being easy to write manually, as that should only be for manual testing. The design that makes the backend (and frontend) code and preferably the (prettified) request data most readable should be chosen, I think. We can add multiple endpoints that are "manual testing-friendly" as well, though, if the need is high enough.

Say you have ?tags=sourcetype=mjam,foo=b,bar=c,xux=d,foo=1. You could turn all those into tag querysets and look up which incidents have them in one fell swoop, or you could pop sourcetype from the tags-list, get the tags, get the source type and then recombine. The latter is more complex, everywhere: querysets, api, ui, cli. Querystrings, APIS, class hierarchies are also UIs.

In this case, I would have preferred the query string to look like this: ?tags=foo=b,bar=c,xux=d,foo=1&sourcetype=mjam. As far as I can see, with your suggested design, we would also have to pop sourcetype and get the SourceSystemType object, as there are no incidents with that tag, as per your initial suggestion:

On creation of a source type, we create a tag for that source type, with no connected incidents.

@ddabble
Copy link
Contributor Author

ddabble commented Aug 13, 2020

Thinking out loud: There will be very few SourceSystemTypes. There can be potentially very many SourceSystems, objects, parent_objects, problem_types. This will affect the UI. If there is a "tags" column in the UI, the simplest way to make a filter is to click on the tags of a row. It might be relevant to give a subset of tags (like source system type, source?) their own column. This needs some mockups I think.

I'm not sure I'm following.. Could you explain where these columns and rows would be in the UI, and how they're used?

Regarding the design of how a user filters on tags in the frontend, @jorgenbele and I had a discussion and came up with the following proposal:

When the user starts writing in a text input field, the frontend queries the backend with which tags start with / contain the text input so far, and displays the matches below the input field - like a code completion program would do.

@hmpf
Copy link
Contributor

hmpf commented Aug 13, 2020

explain where these columns and rows would be in the UI

We'd need mockups. Better to draw something in a hurry, check with end users in a user test, then implement, than implement first and have to make endless adjustments.

@hmpf
Copy link
Contributor

hmpf commented Aug 13, 2020

Regarding the design of how a user filters on tags in the frontend, @jorgenbele and I had a discussion and came up with the following proposal:

When the user starts writing in a text input field, the frontend queries the backend with which tags start with / contain the text input so far, and displays the matches below the input field - like a code completion program would do.

So in sum: one form-field for sources, one for tags, both autocomplete? Another form field for source type?

@ddabble
Copy link
Contributor Author

ddabble commented Aug 13, 2020

So in sum: one form-field for sources, one for tags, both autocomplete? Another form field for source type?

We didn't discuss that specifically - other than that tags would have its own form field, but yes, that sounds like a viable solution to me 🙂

@hmpf hmpf moved this from To do to In progress in Argus Sprint #2 Aug 14, 2020
@hmpf hmpf moved this from In progress to Done in Argus Sprint #2 Aug 17, 2020
@hmpf
Copy link
Contributor

hmpf commented Aug 27, 2020

We still need to be able to filter in the API, but not in this issue.Closing...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Requires developer feedback/discussion before implementation task
Projects
No open projects
Development

No branches or pull requests

2 participants