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

[APM] Remove ui filters #84526

Closed
6 of 8 tasks
sorenlouv opened this issue Nov 30, 2020 · 17 comments
Closed
6 of 8 tasks

[APM] Remove ui filters #84526

sorenlouv opened this issue Nov 30, 2020 · 17 comments
Assignees
Labels
Team:APM All issues that need APM UI Team support v7.12.0

Comments

@sorenlouv
Copy link
Member

sorenlouv commented Nov 30, 2020

Related

TODO:

7.12

7.13

UI filters were released 1.5 years ago in 7.4. Since then we haven't iterated on them due to concerns about their direction. Implementing them introduced a high degree of complexity and technical debt and in their current state this cost is not worth it.

UI Filters doesn't introduce any new capabilities that cannot be gained from the search bar. Additionally, ui filters is unique to APM and are as a concept inconsistent with the rest of Kibana.

To align with the rest of Kibana, and reduce complexity I suggest we remove the UI filters.

UI Filters are located on the left, under the heading "Filters"

@sorenlouv sorenlouv added [zube]: Inbox discuss Team:APM All issues that need APM UI Team support labels Nov 30, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

@dgieselaar
Copy link
Member

++, this will also allow us to aggregate on less fields when aggregation transaction metrics, which will make them more efficient.

@alex-fedotyev
Copy link

I think this would be a good first step in improving filtering experience.

Based on my interactions with customers so far, our customer try to filter a dataset using following steps:

  1. Try to find attributes which have multiple values (for example, user finds that there are cloud availability zone and service version which have multiple values during selected time frame).
  2. Identify whether any of those value stand out from the rest in a one or more measurements, typically number of calls vs. latency vs. error rates:
    1. This could be done by either splitting by the attribute and showing top N values and their measurements (for example, compare service performance across all availability zones).
    2. Another option could be to compare original dataset with a subset defined by a specific value (for example, compare latest canary deployment performance vs entire service).
  3. In case one or more values are showing significant impact, apply the filter using that value to perform further troubleshooting.

I included this workflow to highlight that users will need filtering capabilities in the end.

Will we be able to introduce filters from Kibana into APM when we remove UI filters?
image

@sorenlouv
Copy link
Member Author

Will we be able to introduce filters from Kibana into APM when we remove UI filters?

Yes, we can have a similar experience to how the rest of Kibana does this.

@alex-fedotyev
Copy link

I think this would be perfect.

@nehaduggal - how are you expecting transaction significant tags to work? Would you need filters for that?

@nehaduggal
Copy link

We don't need filters for significant terms. The plan is to surface the sig terms for transactions with high latency, errors or throughput regardless of transaction type. Taking away filters shouldn't affect that functionality in any way.

@gimmic
Copy link

gimmic commented Dec 1, 2020

Forgive my ignorance on this, but how will a user be able to quickly toggle a field query quickly between 'and', 'not', 'disabled'?

I understand that it can be changed manually in the query, but for high level dashboards UI filter elements are nice as toggles. Is this what is being referenced?
(It's possible I am misunderstanding the context of "remove ui filters")

@sorenlouv
Copy link
Member Author

@iankoetter Thanks for chiming in. I've updated the issue with an screenshot to make it more clear.
This is how you could can achieve the same thing in the future:

Using a filter

Using kql

This should be more recognisable for people familiar with Kibana. Do you prefer the old ui filters approach?

@formgeist
Copy link
Contributor

From a product design and UX point of view, removing the UI filters seems like a hasty decision based on performance issues, without much thought to what we're removing from the experience in APM.

The query bar filtering experience is not meant for the average user, because it requires them to go through a lot of steps for even the simplest filter and they need to know what they're looking for. The most important feature of the UI filters was the discoverability of those contextual values.

I understand that with the metadata and significant tags, we will eventually allow the user to instantly filter by those values and we would otherwise present contextual information such as service version and cloud information in the header.

Let's at least have a more formal plan for what we want our filtering experience to look like before we go and rip out the existing feature.

@gimmic
Copy link

gimmic commented Dec 2, 2020

Edit: I realize I have overlooked the primary use here, APM. If this is not a planned change for Kibana globally, then my input should be discarded!

So for junior analysts who may not be familiar with KQL syntax, or even know what fields are useful to filter against, how do we provide a frame of reference? Today this is one of the functions provided by UI filter elements on views. As a simple example we might have a saved discover search with that comes pre-enabled with some exclusion filters. The idea is to make it visible to the analyst their view is modified by those filters, but also give them the opportunity to quickly toggle it to an enabled or disabled state as well as inverse the filter(s). Those pre-canned UI elements are useful.

Example:
image

Removing filters also adds to the length of KQL queries in the search bar, which could also become an editing challenge. If I've got a long enough query to add horizontal scroll to KQL input, then when I want to "toggle" a field inclusion/exclusion, I'll have to find it in the text and manually change the text. Think of editing a very long URL in your browser's address bar..

With the filter options gone as an UI element, what's the alternative there? It seems like this would be removing knobs in favor of manual KQL typing which is more ephemeral in nature. Generally I am a fan of "less clicks", but not when removal of clicks adds complexity. Pivoting around in a dataset with a collection of "common knobs" makes much more sense than having to type those same knobs out over and over for each query/pivot.

The workflow generally goes as follows:
Is this a one time query filter? -> KQL
Is this a field/value we might often want to filter as an AND/NOT, but sometimes disable in general? -> UI filter element.

@formgeist
Copy link
Contributor

@iankoetter Thanks for providing some more insights. Indeed, the UI filters removal is only related to the APM implementation. As discussed we're intending to make the UI filters experience consistent with what Kibana does today (e.g. what you see in Discover or Dashboards); that's allowing you to add saved searches, change how filters are applied (e.g. NOT), and more.

More specifically, I was referring to the user experience of adding a new filter in that KQL search bar. The user is going to get a full list of possible fields, whether they relate to APM/Observability or not, and have to remember which field applies to which part of the dataset. It's definitely something that can be improved upon.

@formgeist
Copy link
Contributor

I've opened an enhancement issue to improve filtering capabilities in the UI possibly as part of the removal of the UI filters. #87537

@formgeist
Copy link
Contributor

I've been discussing with the (RUM) UX app team about how this affects the UI filters in that application and I imagine we'd need to move over the UI filters code as they will continue to use them in lieu of not having a search bar and there's no replacement for the filtering experience at this time. Any thoughts @sqren ? cc @shahzad31

@sorenlouv
Copy link
Member Author

I think it should be doable to only remove ui filters from the APM codebase.

@sorenlouv
Copy link
Member Author

@smith Are you still working on this or should I move it back?

@smith
Copy link
Contributor

smith commented Feb 10, 2021

@sqren definitely still working on it. See the linked issues in the description.

smith added a commit to smith/kibana that referenced this issue Feb 18, 2021
For a non-Java service, the previous link was like:

```
http://localhost:5601/kbn/app/apm/services/opbeans-python/metrics?rangeFrom=now-15m&rangeTo=now
```

which did not filter by the `service.node.name`.

It now is:

```
http://localhost:5601/kbn/app/apm/services/opbeans-python/metrics?kuery=service.node.name:%226a7f116fe344aee7e92fceeb426cbfdf6a534a8e3ba6345c16a47793eba6daf5%22&rangeFrom=now-15m&rangeTo=now
````

Which links to the metrics page with the filter applied.

The component is using a `MetricOverviewLink` which was using a `EuiLink` and passing throught the props, including `mergeQuery`, which includes the `kuery` parameter.

Replace the `EuiLink` with an `APMLink` which does use the `mergeQuery` prop and does pass the parameters through correctly.

Looks like this was changed to an `EuiLink` by a refactor in elastic#86986.

Since we'll be making some further changes to how `kuery` is handled in elastic#84526, I'm just making the minimal change to fix this bug at this time.
smith added a commit that referenced this issue Feb 18, 2021
For a non-Java service, the previous link was like:

```
http://localhost:5601/kbn/app/apm/services/opbeans-python/metrics?rangeFrom=now-15m&rangeTo=now
```

which did not filter by the `service.node.name`.

It now is:

```
http://localhost:5601/kbn/app/apm/services/opbeans-python/metrics?kuery=service.node.name:%226a7f116fe344aee7e92fceeb426cbfdf6a534a8e3ba6345c16a47793eba6daf5%22&rangeFrom=now-15m&rangeTo=now
````

Which links to the metrics page with the filter applied.

The component is using a `MetricOverviewLink` which was using a `EuiLink` and passing throught the props, including `mergeQuery`, which includes the `kuery` parameter.

Replace the `EuiLink` with an `APMLink` which does use the `mergeQuery` prop and does pass the parameters through correctly.

Looks like this was changed to an `EuiLink` by a refactor in #86986.

Since we'll be making some further changes to how `kuery` is handled in #84526, I'm just making the minimal change to fix this bug at this time.
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Feb 18, 2021
For a non-Java service, the previous link was like:

```
http://localhost:5601/kbn/app/apm/services/opbeans-python/metrics?rangeFrom=now-15m&rangeTo=now
```

which did not filter by the `service.node.name`.

It now is:

```
http://localhost:5601/kbn/app/apm/services/opbeans-python/metrics?kuery=service.node.name:%226a7f116fe344aee7e92fceeb426cbfdf6a534a8e3ba6345c16a47793eba6daf5%22&rangeFrom=now-15m&rangeTo=now
````

Which links to the metrics page with the filter applied.

The component is using a `MetricOverviewLink` which was using a `EuiLink` and passing throught the props, including `mergeQuery`, which includes the `kuery` parameter.

Replace the `EuiLink` with an `APMLink` which does use the `mergeQuery` prop and does pass the parameters through correctly.

Looks like this was changed to an `EuiLink` by a refactor in elastic#86986.

Since we'll be making some further changes to how `kuery` is handled in elastic#84526, I'm just making the minimal change to fix this bug at this time.
kibanamachine added a commit that referenced this issue Feb 18, 2021
…1842)

For a non-Java service, the previous link was like:

```
http://localhost:5601/kbn/app/apm/services/opbeans-python/metrics?rangeFrom=now-15m&rangeTo=now
```

which did not filter by the `service.node.name`.

It now is:

```
http://localhost:5601/kbn/app/apm/services/opbeans-python/metrics?kuery=service.node.name:%226a7f116fe344aee7e92fceeb426cbfdf6a534a8e3ba6345c16a47793eba6daf5%22&rangeFrom=now-15m&rangeTo=now
````

Which links to the metrics page with the filter applied.

The component is using a `MetricOverviewLink` which was using a `EuiLink` and passing throught the props, including `mergeQuery`, which includes the `kuery` parameter.

Replace the `EuiLink` with an `APMLink` which does use the `mergeQuery` prop and does pass the parameters through correctly.

Looks like this was changed to an `EuiLink` by a refactor in #86986.

Since we'll be making some further changes to how `kuery` is handled in #84526, I'm just making the minimal change to fix this bug at this time.

Co-authored-by: Nathan L Smith <nathan.smith@elastic.co>
smith added a commit that referenced this issue Feb 23, 2021
* Make kuery a standalone query parameter instead of part of uiFilters
* Move getEsFilters helper to RUM
* Move query utils from "common" to "server" (it uses esKuery from the data plugin, which is exported from server, not common.)
* Move uiFiltersRt to RUM routes

References #84526.
smith added a commit to smith/kibana that referenced this issue Feb 24, 2021
* Make kuery a standalone query parameter instead of part of uiFilters
* Move getEsFilters helper to RUM
* Move query utils from "common" to "server" (it uses esKuery from the data plugin, which is exported from server, not common.)
* Move uiFiltersRt to RUM routes

References elastic#84526.
# Conflicts:
#	x-pack/plugins/apm/public/context/annotations/annotations_context.tsx
smith added a commit that referenced this issue Mar 2, 2021
* Make kuery a standalone query parameter instead of part of uiFilters
* Move getEsFilters helper to RUM
* Move query utils from "common" to "server" (it uses esKuery from the data plugin, which is exported from server, not common.)
* Move uiFiltersRt to RUM routes

References #84526.
# Conflicts:
#	x-pack/plugins/apm/public/context/annotations/annotations_context.tsx
smith added a commit to smith/kibana that referenced this issue Mar 2, 2021
For a non-Java service, the previous link was like:

```
http://localhost:5601/kbn/app/apm/services/opbeans-python/metrics?rangeFrom=now-15m&rangeTo=now
```

which did not filter by the `service.node.name`.

It now is:

```
http://localhost:5601/kbn/app/apm/services/opbeans-python/metrics?kuery=service.node.name:%226a7f116fe344aee7e92fceeb426cbfdf6a534a8e3ba6345c16a47793eba6daf5%22&rangeFrom=now-15m&rangeTo=now
````

Which links to the metrics page with the filter applied.

The component is using a `MetricOverviewLink` which was using a `EuiLink` and passing throught the props, including `mergeQuery`, which includes the `kuery` parameter.

Replace the `EuiLink` with an `APMLink` which does use the `mergeQuery` prop and does pass the parameters through correctly.

Looks like this was changed to an `EuiLink` by a refactor in elastic#86986.

Since we'll be making some further changes to how `kuery` is handled in elastic#84526, I'm just making the minimal change to fix this bug at this time.
smith added a commit that referenced this issue Mar 2, 2021
…3240)

For a non-Java service, the previous link was like:

```
http://localhost:5601/kbn/app/apm/services/opbeans-python/metrics?rangeFrom=now-15m&rangeTo=now
```

which did not filter by the `service.node.name`.

It now is:

```
http://localhost:5601/kbn/app/apm/services/opbeans-python/metrics?kuery=service.node.name:%226a7f116fe344aee7e92fceeb426cbfdf6a534a8e3ba6345c16a47793eba6daf5%22&rangeFrom=now-15m&rangeTo=now
````

Which links to the metrics page with the filter applied.

The component is using a `MetricOverviewLink` which was using a `EuiLink` and passing throught the props, including `mergeQuery`, which includes the `kuery` parameter.

Replace the `EuiLink` with an `APMLink` which does use the `mergeQuery` prop and does pass the parameters through correctly.

Looks like this was changed to an `EuiLink` by a refactor in #86986.

Since we'll be making some further changes to how `kuery` is handled in #84526, I'm just making the minimal change to fix this bug at this time.
@smith
Copy link
Contributor

smith commented Mar 9, 2021

Closing this. Opened #94095 to remove projections. #82751 is open to move to the standard search bar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:APM All issues that need APM UI Team support v7.12.0
Projects
None yet
Development

No branches or pull requests

9 participants