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

[Maps] spatially filter by all geo fields #100735

Merged
merged 9 commits into from
Jun 2, 2021

Conversation

nreese
Copy link
Contributor

@nreese nreese commented May 26, 2021

When creating a spatial filter, users must select a geo field to filter against. This is problematic if your map contains multiple indices and multiple geo fields. The generated filter causes all layers not using the selected field to show no data.

This PR removes the geo field selection when creating spatial filters. The generated filter is a bool.should filter that contains nested bool.must filters that test to ensure each geoField exists prior to testing if the geoField is contained in the spatial feature. That way, the filter will work for each map layer.

Removing the field selection simplifies the UI and makes the application just work the way users think it should work.

To test the PR:

  1. create a map with layers from multiple indices
  2. Use draw tools to create filters.
  3. index some shapes and add as a layer
  4. open tooltip for shape and create filter for shape

Screen Shot 2021-05-26 at 1 49 05 PM

cc @maihde

@nreese nreese added release_note:enhancement [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 v7.14.0 auto-backport Deprecated: Automatically backport this PR after it's merged labels May 26, 2021
@nreese nreese requested a review from a team as a code owner May 26, 2021 19:55
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis (Team:Geo)

@nreese nreese mentioned this pull request May 26, 2021
8 tasks
@jsanz
Copy link
Member

jsanz commented May 27, 2021

This is a great UX improvement 💯 I tested it locally on the Chromium browser without issues.

Should we consider the case where new layers are added with a geo field name not used before? I understand this is a corner case so I'd be OK on not doing anything since the behavior is pretty understandable.

Peek 2021-05-27 15-10

@kmartastic
Copy link
Contributor

In the same scenario with current behavior, adding a new layer (based on the same field as the existing filter), will automatically filter the new layer. I like that behavior.

@jsanz what happens when you disable/enable the filter in your example above?

@nreese
Copy link
Contributor Author

nreese commented May 27, 2021

Should we consider the case where new layers are added with a geo field name not used before? I understand this is a corner case so I'd be OK on not doing anything since the behavior is pretty understandable.

@jsanz Thanks for raising this issue. I would consider this a separate issue that can be resolved in a its own PR.

@jsanz
Copy link
Member

jsanz commented May 27, 2021

@jsanz what happens when you disable/enable the filter in your example above?

The created filter is not affecting the new layer, you are supposed to create a new filter from scratch (@nreese correct me if I'm wrong).

The thing is that trying again I think I got into an issue. After adding two layers (sample logs and sample flights/origin) I added again the sample flights/destLocation and the filter is only working with the sample logs layer 🤔

Leaving below the requests for the three layers, in case it helps.

Peek 2021-05-27 17-25

Origin layer request
{
  "docvalue_fields": [
    "OriginLocation"
  ],
  "size": 10000,
  "track_total_hits": 10001,
  "_source": false,
  "script_fields": {},
  "stored_fields": [
    "OriginLocation"
  ],
  "runtime_mappings": {},
  "query": {
    "bool": {
      "must": [],
      "filter": [
        {
          "match_all": {}
        },
        {
          "bool": {
            "should": [
              {
                "bool": {
                  "must": [
                    {
                      "exists": {
                        "field": "geo.coordinates"
                      }
                    },
                    {
                      "geo_shape": {
                        "ignore_unmapped": true,
                        "geo.coordinates": {
                          "relation": "INTERSECTS",
                          "shape": {
                            "type": "Polygon",
                            "coordinates": [
                              [
                                [
                                  -115.96546,
                                  57.84821
                                ],
                                [
                                  -115.96546,
                                  10.79817
                                ],
                                [
                                  -94.13189,
                                  10.79817
                                ],
                                [
                                  -94.13189,
                                  57.84821
                                ],
                                [
                                  -115.96546,
                                  57.84821
                                ]
                              ]
                            ]
                          }
                        }
                      }
                    }
                  ]
                }
              },
              {
                "bool": {
                  "must": [
                    {
                      "exists": {
                        "field": "OriginLocation"
                      }
                    },
                    {
                      "geo_shape": {
                        "ignore_unmapped": true,
                        "OriginLocation": {
                          "relation": "INTERSECTS",
                          "shape": {
                            "type": "Polygon",
                            "coordinates": [
                              [
                                [
                                  -115.96546,
                                  57.84821
                                ],
                                [
                                  -115.96546,
                                  10.79817
                                ],
                                [
                                  -94.13189,
                                  10.79817
                                ],
                                [
                                  -94.13189,
                                  57.84821
                                ],
                                [
                                  -115.96546,
                                  57.84821
                                ]
                              ]
                            ]
                          }
                        }
                      }
                    }
                  ]
                }
              },
              {
                "bool": {
                  "must": [
                    {
                      "exists": {
                        "field": "DestLocation"
                      }
                    },
                    {
                      "geo_shape": {
                        "ignore_unmapped": true,
                        "DestLocation": {
                          "relation": "INTERSECTS",
                          "shape": {
                            "type": "Polygon",
                            "coordinates": [
                              [
                                [
                                  -115.96546,
                                  57.84821
                                ],
                                [
                                  -115.96546,
                                  10.79817
                                ],
                                [
                                  -94.13189,
                                  10.79817
                                ],
                                [
                                  -94.13189,
                                  57.84821
                                ],
                                [
                                  -115.96546,
                                  57.84821
                                ]
                              ]
                            ]
                          }
                        }
                      }
                    }
                  ]
                }
              }
            ]
          }
        },
        {
          "geo_bounding_box": {
            "OriginLocation": {
              "top_left": [
                -180,
                89
              ],
              "bottom_right": [
                180,
                -89
              ]
            }
          }
        },
        {
          "range": {
            "timestamp": {
              "gte": "2021-04-26T22:00:00.000Z",
              "lte": "2021-05-27T15:25:33.589Z",
              "format": "strict_date_optional_time"
            }
          }
        }
      ],
      "should": [],
      "must_not": []
    }
  }
}
Destination layer request
{
  "docvalue_fields": [
    "DestLocation"
  ],
  "size": 10000,
  "track_total_hits": 10001,
  "_source": false,
  "script_fields": {},
  "stored_fields": [
    "DestLocation"
  ],
  "runtime_mappings": {},
  "query": {
    "bool": {
      "must": [],
      "filter": [
        {
          "match_all": {}
        },
        {
          "bool": {
            "should": [
              {
                "bool": {
                  "must": [
                    {
                      "exists": {
                        "field": "geo.coordinates"
                      }
                    },
                    {
                      "geo_shape": {
                        "ignore_unmapped": true,
                        "geo.coordinates": {
                          "relation": "INTERSECTS",
                          "shape": {
                            "type": "Polygon",
                            "coordinates": [
                              [
                                [
                                  -115.96546,
                                  57.84821
                                ],
                                [
                                  -115.96546,
                                  10.79817
                                ],
                                [
                                  -94.13189,
                                  10.79817
                                ],
                                [
                                  -94.13189,
                                  57.84821
                                ],
                                [
                                  -115.96546,
                                  57.84821
                                ]
                              ]
                            ]
                          }
                        }
                      }
                    }
                  ]
                }
              },
              {
                "bool": {
                  "must": [
                    {
                      "exists": {
                        "field": "OriginLocation"
                      }
                    },
                    {
                      "geo_shape": {
                        "ignore_unmapped": true,
                        "OriginLocation": {
                          "relation": "INTERSECTS",
                          "shape": {
                            "type": "Polygon",
                            "coordinates": [
                              [
                                [
                                  -115.96546,
                                  57.84821
                                ],
                                [
                                  -115.96546,
                                  10.79817
                                ],
                                [
                                  -94.13189,
                                  10.79817
                                ],
                                [
                                  -94.13189,
                                  57.84821
                                ],
                                [
                                  -115.96546,
                                  57.84821
                                ]
                              ]
                            ]
                          }
                        }
                      }
                    }
                  ]
                }
              },
              {
                "bool": {
                  "must": [
                    {
                      "exists": {
                        "field": "DestLocation"
                      }
                    },
                    {
                      "geo_shape": {
                        "ignore_unmapped": true,
                        "DestLocation": {
                          "relation": "INTERSECTS",
                          "shape": {
                            "type": "Polygon",
                            "coordinates": [
                              [
                                [
                                  -115.96546,
                                  57.84821
                                ],
                                [
                                  -115.96546,
                                  10.79817
                                ],
                                [
                                  -94.13189,
                                  10.79817
                                ],
                                [
                                  -94.13189,
                                  57.84821
                                ],
                                [
                                  -115.96546,
                                  57.84821
                                ]
                              ]
                            ]
                          }
                        }
                      }
                    }
                  ]
                }
              }
            ]
          }
        },
        {
          "geo_bounding_box": {
            "DestLocation": {
              "top_left": [
                -180,
                89
              ],
              "bottom_right": [
                180,
                -89
              ]
            }
          }
        },
        {
          "range": {
            "timestamp": {
              "gte": "2021-04-26T22:00:00.000Z",
              "lte": "2021-05-27T15:25:33.709Z",
              "format": "strict_date_optional_time"
            }
          }
        }
      ],
      "should": [],
      "must_not": []
    }
  }
}
Logs layer request
{
  "docvalue_fields": [
    "geo.coordinates"
  ],
  "size": 10000,
  "track_total_hits": 10001,
  "_source": false,
  "script_fields": {},
  "stored_fields": [
    "geo.coordinates"
  ],
  "runtime_mappings": {},
  "query": {
    "bool": {
      "must": [],
      "filter": [
        {
          "match_all": {}
        },
        {
          "bool": {
            "should": [
              {
                "bool": {
                  "must": [
                    {
                      "exists": {
                        "field": "geo.coordinates"
                      }
                    },
                    {
                      "geo_shape": {
                        "ignore_unmapped": true,
                        "geo.coordinates": {
                          "relation": "INTERSECTS",
                          "shape": {
                            "type": "Polygon",
                            "coordinates": [
                              [
                                [
                                  -115.96546,
                                  57.84821
                                ],
                                [
                                  -115.96546,
                                  10.79817
                                ],
                                [
                                  -94.13189,
                                  10.79817
                                ],
                                [
                                  -94.13189,
                                  57.84821
                                ],
                                [
                                  -115.96546,
                                  57.84821
                                ]
                              ]
                            ]
                          }
                        }
                      }
                    }
                  ]
                }
              },
              {
                "bool": {
                  "must": [
                    {
                      "exists": {
                        "field": "OriginLocation"
                      }
                    },
                    {
                      "geo_shape": {
                        "ignore_unmapped": true,
                        "OriginLocation": {
                          "relation": "INTERSECTS",
                          "shape": {
                            "type": "Polygon",
                            "coordinates": [
                              [
                                [
                                  -115.96546,
                                  57.84821
                                ],
                                [
                                  -115.96546,
                                  10.79817
                                ],
                                [
                                  -94.13189,
                                  10.79817
                                ],
                                [
                                  -94.13189,
                                  57.84821
                                ],
                                [
                                  -115.96546,
                                  57.84821
                                ]
                              ]
                            ]
                          }
                        }
                      }
                    }
                  ]
                }
              },
              {
                "bool": {
                  "must": [
                    {
                      "exists": {
                        "field": "DestLocation"
                      }
                    },
                    {
                      "geo_shape": {
                        "ignore_unmapped": true,
                        "DestLocation": {
                          "relation": "INTERSECTS",
                          "shape": {
                            "type": "Polygon",
                            "coordinates": [
                              [
                                [
                                  -115.96546,
                                  57.84821
                                ],
                                [
                                  -115.96546,
                                  10.79817
                                ],
                                [
                                  -94.13189,
                                  10.79817
                                ],
                                [
                                  -94.13189,
                                  57.84821
                                ],
                                [
                                  -115.96546,
                                  57.84821
                                ]
                              ]
                            ]
                          }
                        }
                      }
                    }
                  ]
                }
              }
            ]
          }
        },
        {
          "geo_bounding_box": {
            "geo.coordinates": {
              "top_left": [
                -180,
                89
              ],
              "bottom_right": [
                180,
                -89
              ]
            }
          }
        },
        {
          "range": {
            "timestamp": {
              "gte": "2021-04-26T22:00:00.000Z",
              "lte": "2021-05-27T15:25:33.590Z",
              "format": "strict_date_optional_time"
            }
          }
        }
      ],
      "should": [],
      "must_not": []
    }
  }
}

@nreese
Copy link
Contributor Author

nreese commented May 27, 2021

The thing is that trying again I think I got into an issue. After adding two layers (sample logs and sample flights/origin) I added again the sample flights/destLocation and the filter is only working with the sample logs layer

This is caused by having 2 geo fields from the same index and same document. In your case, you added DestLocation and OriginLocation from flights index. So the filter is working, just that matching documents also have locations outside of filter bounds

@jsanz
Copy link
Member

jsanz commented May 27, 2021

Thanks @nreese I see now, it's clear when we use the exclude filter 😅. Mixing filters with documents with many geospatial filters fields is tricky!

image

@nreese
Copy link
Contributor Author

nreese commented Jun 1, 2021

@elasticmachine merge upstream

@nreese nreese requested review from nickpeihl and removed request for thomasneirynck June 1, 2021 12:46
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
maps 731 732 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
maps 2.6MB 2.6MB +4.5KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
data 820.8KB 821.2KB +344.0B
Unknown metric groups

References to deprecated APIs

id before after diff
canvas 29 25 -4
crossClusterReplication 8 6 -2
fleet 22 20 -2
globalSearch 4 2 -2
indexManagement 12 7 -5
infra 261 149 -112
lens 67 45 -22
licensing 18 15 -3
lists 239 236 -3
maps 286 208 -78
ml 121 115 -6
monitoring 109 56 -53
securitySolution 390 346 -44
stackAlerts 101 95 -6
total -342

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Member

@nickpeihl nickpeihl left a comment

Choose a reason for hiding this comment

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

Looking really good! I like the simplified interface.

Code changes lgtm.

A couple of nits:

  • If I already have a filter on a map and then add another layer, I would expect the new layer to also be filtered. Currently, the new layer shows nothing. Fine with me if this is in a new PR, but I think it should definitely be in the same release as this PR.
Jun-01-2021.15-52-25.mp4
  • When I edit a previously created filter, there is an option to choose an Index Pattern. Choosing an index pattern appears to have no effects on the layers. Can we get rid of this option?

Screen Shot 2021-06-01 at 3 29 46 PM

@nreese
Copy link
Contributor Author

nreese commented Jun 2, 2021

When I edit a previously created filter, there is an option to choose an Index Pattern. Choosing an index pattern appears to have no effects on the layers. Can we get rid of this option?

I can open an issue but we do not own that code.

@nreese nreese merged commit 14442b7 into elastic:master Jun 2, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Jun 2, 2021
* [Maps] spatial filter by all geo fields

* replace geoFields with geoFieldNames

* update mapSpatialFilter to be able to reconize multi field filters

* add check for geoFieldNames

* i18n fixes and fix GeometryFilterForm jest test

* tslint

* tslint

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Jun 2, 2021
* [Maps] spatial filter by all geo fields

* replace geoFields with geoFieldNames

* update mapSpatialFilter to be able to reconize multi field filters

* add check for geoFieldNames

* i18n fixes and fix GeometryFilterForm jest test

* tslint

* tslint

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Nathan Reese <reese.nathan@gmail.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jun 2, 2021
* master: (68 commits)
  Unskip advanced settings a11y test (elastic#100558)
  [App Search] Crawler Landing Page (elastic#100822)
  [DOCS] Clarify when to use kbn clean (elastic#101155)
  change label behavior (elastic#100991)
  skip flaky suite (elastic#101126)
  Fix cases plugin ownership (elastic#101073)
  [Home] Adding file upload to add data page (elastic#100863)
  [ML] Functional tests - reenable categorization tests (elastic#101137)
  [DOCS] Adds server.uuid to settings docs (elastic#101121)
  Fix newsfeed unread notifications always on when reloading Kibana (elastic#100357)
  [Lens] Time shift metrics (elastic#98781)
  [Deprecations service] make `correctiveActions.manualSteps` required (elastic#100997)
  Add "Risk Matrix" section to the PR template (elastic#100649)
  [Maps] spatially filter by all geo fields (elastic#100735)
  [Security Solution] Add Ransomware canary advanced policy option (elastic#101068)
  [Exploratory view] Core web vitals (elastic#100320)
  [Security solution][Endpoint] Add unit tests for fleet event filters/trusted apps cards (elastic#101034)
  [Lens] Use a setter function for the dimension panel (elastic#101123)
  [Index Patterns] Fix return saved index pattern object (elastic#101051)
  [CI] For PRs, build TS refs before public api docs check (elastic#100791)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated: Automatically backport this PR after it's merged [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation release_note:enhancement v7.14.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants