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

Handle un-mappable filters more gracefully #13916

Closed
wants to merge 2 commits into from

Conversation

Bargs
Copy link
Contributor

@Bargs Bargs commented Sep 8, 2017

@lukasolson I started to tackle #7767 on low fruit friday but I'd like your opinion on how we should handle it. These "invisible filters" occur when the mapping chain throws an error. If a single filter throws an error, the entire chain rejects. As a result, not even the valid filters appear in the filter bar because they never get added to the scope.

So far in this PR I've changed the mapping chain to include the un-mappable filters in the result instead of rejecting, with the error assigned as a meta property. I'm thinking we can look for this meta property in the filter bar and filter editor code and display something helpful for the user.

Right now the pill for an un-mappable filter has a blank key/value:

screen shot 2017-09-08 at 5 14 55 pm

If you click the edit button, you get the ace editor with the correct DSL:

screen shot 2017-09-08 at 5 16 09 pm

So here's where I'm a little uncertain what to do. I don't know how to convey what's going on to the user in a way that will make sense. The filter isn't "invalid" exactly, because it's still affecting the search results just fine. We just can't map it, in this case because the filter's index doesn't exist. But we can't say "hey this filter's index doesn't exist" because we don't expose the index to the user so it'll make no sense. Maybe we should add the index as a user editable property next to the label input? What do you think?

I considered simply deleting the invalid filter, but there were a few problems with that.

  • We don't know there's a problem until the mapping chain executes, and at that point the initial search request as already been fired.
  • The user may be confused if the filter just disappears on them
  • The filter isn't exactly invalid and the user might want a chance to fix it

@Bargs Bargs requested a review from lukasolson September 8, 2017 21:28
@lukasolson
Copy link
Member

I think this is a good start. What if, if there is an error in the mapper, it just uses the notifier and shows the error?

@Bargs
Copy link
Contributor Author

Bargs commented Sep 13, 2017

Do you mean in addition to exposing the index as an editable property?

@lukasolson
Copy link
Member

Okay, I've given this a little more thought. I'm not sure how feasible this is, but what if, if we encountered errors in the map filter process, we showed a pop-down error that said something about how the index for the filter couldn't be found, with a link that, when clicked, would open up the filter editor (straight to the filter DSL) and we included not only the query portion of the filter, but also the index?

@Bargs
Copy link
Contributor Author

Bargs commented Sep 13, 2017

Lukas and I chatted about this some on zoom and realized it would be nice to allow users to select an index pattern the same way they select a field. Instead of three inputs in a row we'll have 4, with the index being the first choice. The selected index will then filter the field list, which will also improve the field selector on dashboards with multiple indices. Maybe on Discover and Visualize we should auto-select the index pattern that's currently in use. I'll work on this some more on friday.

@Bargs
Copy link
Contributor Author

Bargs commented Sep 15, 2017

After adding in the index pattern selector, it became apparent that this option will confuse users since the selected pattern doesn't actually limit the filter to only working against that pattern. Users won't really understand what their selection means.

Next week I'm going to try a different approach to solving the invisible filter issue. Since the meta.index property only seems to be used for getting a field formatter to display the value in the pill, in the case where there's an invalid index I'm just going to ignore it and display the raw value in the pill instead. This will avoid the error causing the invisible filters in the first place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants