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

Feat geofilter from filterbar #145465

Closed

Conversation

desean1625
Copy link
Contributor

Summary

Add ability to create geospatial filters from the filter bar

#144064

For maintainers

@desean1625 desean1625 requested review from a team as code owners November 16, 2022 21:29
@kibanamachine
Copy link
Contributor

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@desean1625
Copy link
Contributor Author

image
image

@nreese
Copy link
Contributor

nreese commented Nov 21, 2022

Thanks for opening this PR. Spatial filtering has been an often talked about feature that has unfortunately not made a lot of forward progress. The Kibana team is excited to see community interest in spatial filtering.

#1511 from all the way back in 2014
#67905
#124757

There is even this closed PR #111897 from 2021 that attempted to fix some issues with filters created in Maps application and allow them to work better with the filter editor.

There are a lot of edge cases and complexity with how filters work throughout Kibana. In order to reduce complexity, how about simplifying the approach in this PR to support only the bare minimum of spatial filtering? Then once the edge cases have been resolved, future enhancements can build on top of that foundation.

Mainly, how about supporting only the following operators with this PR

  • exists
  • distance

That way the UI can be simplified, removing the map and just providing a lat input, lon input, and distance input. Simplification will allow this PR to focus on addressing spatial filter edge cases without the complexity of UI concerns.

@desean1625
Copy link
Contributor Author

@nreese The filter that I put in place only supports geo_shape queries and the associated spatial relations as the operators. https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-geo-shape-query.html#geo-shape-spatial-relations.
This is a bare minimum implementation of a geo_shape query.

However, looking at the linked issue #67905 I see the complexity concern because it breaks the DataGrid Expanded actions filter for/filter out buttons. I Have a bunch of meetings today and tomorrow but I can take a look at correcting that when I have free time.

@desean1625 desean1625 requested a review from a team as a code owner November 21, 2022 20:14
@desean1625
Copy link
Contributor Author

Was actually able to look at it during my meeting and fix it. Please let me know if there are any other edge cases that are broken and need to be fixed to accept this PR.

@nreese
Copy link
Contributor

nreese commented Nov 22, 2022

The filter that I put in place only supports geo_shape queries and the associated spatial relations as the operators. https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-geo-shape-query.html#geo-shape-spatial-relations.
This is a bare minimum implementation of a geo_shape query.

Geo distance filter supports geo_point and geo_shape and is a great starting point. The best thing about starting with geo distance is that it avoids needing a map in the UI. Adding in a map into the UI creates a lot of complex problems that are not easy to answer

  1. The map UI provided shows the world countries. I don't think this is the best choice, as once zoomed in, users loose context of where they are. The map should instead show EMS basemap. Using EMS basemap may provide problems for users in air-gapped environments lacking access to EMS. Should we require users to host EMS for offline environments? Should we fall back to kibana.yml map.tilemap.url setting?

  2. The provided PR duplicates some map code with regards to drawing shapes. We would like to avoid code duplication and instead use a package for importing drawing capabilities. Maps application is a monolith plugin and needs to be broken into multiple packages. What will this break out look like? These are not questions that can be answered at the current time.

  3. Overall design considerations. Adding a map the filter UI increases its screen real-estate. There are lots of questions around this and will need support from designers. No show stoppers, just requires more thought and time

These and other issues make it imperative that the first spatial filter implementation be as simple as possible and avoid a map in the UI. Using distance filter and simple UI provides the opportunity to move forward without being blocked by any of these concerns.

@desean1625
Copy link
Contributor Author

@nreese I agree with you.

  1. In an air gapped setting users are utilizing the existing solutions by configuring the kibana.yml for setting up the base map in the maps plugin this will be automatically corrected when we fix point 2.

  2. Code duplication is an issue and what I implemented is a temporary thing. I first attempted to utilize the existing maps package and ran into the following errors. Once the following errors can be resolved I will work with you guys to replace this temporary solution.

     // Want to use x-pack/plugins/maps but get error when using as a required bundle
     // Error: X-Pack plugin or bundle with id "maps" is required by OSS plugin "unifiedSearch", which is prohibited. Consider making this an optional dependency instead.
     // As an optional dependancy we get circular dependancy errors
     // Error: Topological ordering of plugins did not complete, these plugins have cyclic or missing dependencies
     // So just for now a temp solution
    
  3. Thought of that, that is why I hardcoded it to 200px. However, it might be better in a modal or some other ui component. I'm not strong on ui design so I would love the feedback from the designers.

@desean1625
Copy link
Contributor Author

@nreese would you and the other stakeholders have more interest in me making a plugin interface for the unified search that would function similar to the "edit query as DSL" button? I am envisioning a nav bar with the built in "Edit Filter values" and "Edit query as DSL", then button for each unified search plugin that was registered. This way users have the ability to add new filtering features to accommodate their needs without changing the baseline for everyone. I think this would solve the duplicated code worries since it isn't part of the baseline.

I do think that unified geofiltering would be a valuable feature for the everyone, and a plugin approach would also make it fairly simple to incorporate these additions into the baseline at a future date.

@nreese
Copy link
Contributor

nreese commented Dec 8, 2022

would you and the other stakeholders have more interest in me making a plugin interface for the unified search that would function similar to the "edit query as DSL" button? I am envisioning a nav bar with the built in "Edit Filter values" and "Edit query as DSL", then button for each unified search plugin that was registered. This way users have the ability to add new filtering features to accommodate their needs without changing the baseline for everyone. I think this would solve the duplicated code worries since it isn't part of the baseline.

We are discussing this internally and will get back to you in the next week or 2.

@nreese
Copy link
Contributor

nreese commented Dec 16, 2022

would you and the other stakeholders have more interest in me making a plugin interface for the unified search that would function similar to the "edit query as DSL" button? I am envisioning a nav bar with the built in "Edit Filter values" and "Edit query as DSL", then button for each unified search plugin that was registered. This way users have the ability to add new filtering features to accommodate their needs without changing the baseline for everyone. I think this would solve the duplicated code worries since it isn't part of the baseline.

Creating a plugin interface for filters is a large effort and unfortunately the Kibana team does not have the resources to work with community members to design, build, and test such an interface at this time. While we do see the value in the proposal, we are not able to support any efforts in the area of a pluggable filter interface.

@stratoula
Copy link
Contributor

I added an ER to track this needs #147741. Feel free to comment and add your thoughts about this idea there.

@lukeelmers lukeelmers added the Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas label Dec 21, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@kertal kertal marked this pull request as draft December 22, 2022 15:41
@legrego legrego closed this Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💝community Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants