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

[Security Solution] Add nested field inside of data provider #93721

Merged
merged 15 commits into from
Mar 9, 2021

Conversation

XavierM
Copy link
Contributor

@XavierM XavierM commented Mar 5, 2021

Summary

This PR is to fix #89784, we will be able to drag/add nested query inside of the data provider.

To validate this PR id created this index like below and was able to drag and drop this field inside of the data provider

PUT test2
PUT test2/_mapping
{
  "dynamic": false,
  "properties": {
    "myNestedUserField": {
      "type": "nested",
      "properties": {
        "first": {
          "type": "text"
        },
        "last": {
          "type": "text"
        },
        "children": {
          "type": "nested",
          "properties": {
            "name": {
              "type": "text"
            }
          }
        },
        "married": {
          "properties": {
            "wife": {
              "type": "text"
            }
          }
        }
      }
    }
  }
}
POST test2/_doc
{
  "@timestamp": "2021-03-03T12:10:30Z",
  "myNestedUserField": [
    {
      "first" : "Xavier7",
      "last" :  "M",
      "children": [ { "name": "flore"}, { "name": "ingrid"}],
      "married": [{ "wife": "andrea"}]
    },
    {
      "first" : "Kevin6",
      "last" :  "Q"
    }
  ]
}

Checklist

@XavierM XavierM added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes impact:critical This issue should be addressed immediately due to a critical level of impact on the product. v7.12.0 Team:Threat Hunting Security Solution Threat Hunting Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. auto-backport Deprecated - use backport:version if exact versions are needed labels Mar 5, 2021
@XavierM XavierM requested a review from a team as a code owner March 5, 2021 01:49
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

Copy link
Contributor

@rylnd rylnd left a comment

Choose a reason for hiding this comment

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

I did not do a code review, but rather reviewed behavior.

  1. Generated enriched alerts via these steps:
    a558b6a0-8047-11eb-b3b4-3b97d83dbead_-_Kibana

  2. Send e.g. matched.atomic to the timeline, and observe that timeline is now filtered to only those events:
    a558b6a0-8047-11eb-b3b4-3b97d83dbead_-_Kibana

  3. Add a non-overlapping field and see results increase:
    a558b6a0-8047-11eb-b3b4-3b97d83dbead_-_Kibana

  4. Move the non-overlapping field to AND and see results drop to 0:
    a558b6a0-8047-11eb-b3b4-3b97d83dbead_-_Kibana

So far so good; the one thing that did not work was sending a nested date field to timeline; it seems not to generate any filter at all:

a558b6a0-8047-11eb-b3b4-3b97d83dbead_-_Kibana

So: one minor bug with date fields, hopefully; otherwise things look great!

@XavierM
Copy link
Contributor Author

XavierM commented Mar 8, 2021

It looks like that nested date type are not supported in discover too

image

I am going to create a ticket in Kibana for that

@rylnd rylnd self-requested a review March 8, 2021 23:39
@rylnd
Copy link
Contributor

rylnd commented Mar 8, 2021

Approving as nested date fields appears to be a separate issue, and this PR still improves current behavior.

@rylnd
Copy link
Contributor

rylnd commented Mar 9, 2021

@XavierM @kqualters-elastic I'm not seeing an issue with dates in Discover with the mapped threat.indicator.first_seen field:

Exact value:
Discover_-_Elastic

Range:

Discover_-_Elastic

@rylnd
Copy link
Contributor

rylnd commented Mar 9, 2021

Just pulled down the latest (1ad9be5) and confirmed that the date bug has been fixed:

Detections_-_Kibana

The above screenshot demonstrates that the date filtering works in conjunction with a normal filter; previously the number of events returned was in the thousands. Moving either filter to an OR also correctly expands the number of results.

LGTM

@XavierM XavierM enabled auto-merge (squash) March 9, 2021 03:55
@elastic elastic deleted a comment from kibanamachine Mar 9, 2021
@cnasikas
Copy link
Member

cnasikas commented Mar 9, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
securitySolution 7.8MB 7.8MB +1.5KB

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

@XavierM XavierM merged commit af8d5eb into elastic:master Mar 9, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 9, 2021
…#93721)

* add nested field inside of dataprovider

* make sure to get nested

* fix elq server side

* add mock for nested attributes

* Add basic tests for nested fields query generation

* Update snapshots and failing tests with nestedFields

* fix nested date

Co-authored-by: Kevin Qualters <[email protected]>
@kibanamachine
Copy link
Contributor

💚 Backport successful

7.12 / #94080

Successful backport PRs will be merged automatically after passing CI.

XavierM added a commit to XavierM/kibana that referenced this pull request Mar 9, 2021
…#93721)

* add nested field inside of dataprovider

* make sure to get nested

* fix elq server side

* add mock for nested attributes

* Add basic tests for nested fields query generation

* Update snapshots and failing tests with nestedFields

* fix nested date

Co-authored-by: Kevin Qualters <[email protected]>
XavierM added a commit that referenced this pull request Mar 9, 2021
…#94121)

* add nested field inside of dataprovider

* make sure to get nested

* fix elq server side

* add mock for nested attributes

* Add basic tests for nested fields query generation

* Update snapshots and failing tests with nestedFields

* fix nested date

Co-authored-by: Kevin Qualters <[email protected]>

Co-authored-by: Kevin Qualters <[email protected]>
kibanamachine added a commit that referenced this pull request Mar 10, 2021
…#94080)

* add nested field inside of dataprovider

* make sure to get nested

* fix elq server side

* add mock for nested attributes

* Add basic tests for nested fields query generation

* Update snapshots and failing tests with nestedFields

* fix nested date

Co-authored-by: Kevin Qualters <[email protected]>

Co-authored-by: Xavier Mouligneau <[email protected]>
Co-authored-by: Kevin Qualters <[email protected]>
Co-authored-by: MadameSheema <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed impact:critical This issue should be addressed immediately due to a critical level of impact on the product. release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting Security Solution Threat Hunting Team v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Security Solution][Timeline] Dragging a nested field to timeline does not generate the correct query
7 participants