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] Improve rules exception flyout opening for the indices with huge amount of fields #159216

Merged

Conversation

e40pud
Copy link
Contributor

@e40pud e40pud commented Jun 7, 2023

Summary

Original ticket: #158751

These changes improve the rule's exceptions flyout opening experience. We had a few complaints that it is very slow to open it and sometimes it throws an exception about the limited response size.

To fix this, we decided to load extended field's data (conflicts and unmapped info) only when user selects some field instead of fetching this data for all fields on flyout opening.

NOTES:

After these changes we gonna do next steps related to fields loading when user creates/edits rule exceptions:

  1. We will call _fields_for_wildcard WITHOUT include_unmapped=true parameter to fetch all fields specs on exception flyout loading
  2. We will call _fields_for_wildcard WITH include_unmapped=true for only one field when user selects it from the dropdown menu

With these changes we will improve slow exception flyout opening when user has lots of fields which are unmapped in different indices. If for some reason user has a lot of (thousands) conflicting fields around indices then the loading is still might be slow as the _fields_for_wildcard call will return conflicts information even without include_unmapped=true parameter.

@e40pud e40pud added release_note:fix Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. ci:cloud-deploy Create or update a Cloud deployment Team:Detection Engine Security Solution Detection Engine Area labels Jun 7, 2023
@e40pud e40pud self-assigned this Jun 7, 2023
@e40pud e40pud requested review from a team as code owners June 7, 2023 14:30
@elasticmachine
Copy link
Contributor

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

@e40pud
Copy link
Contributor Author

e40pud commented Jun 7, 2023

@elasticmachine merge upstream

@e40pud
Copy link
Contributor Author

e40pud commented Jun 9, 2023

@elasticmachine merge upstream

@e40pud
Copy link
Contributor Author

e40pud commented Jun 12, 2023

@elasticmachine merge upstream

Copy link
Contributor

@vitaliidm vitaliidm left a comment

Choose a reason for hiding this comment

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

@e40pud , thanks for working on it. It's going to be a good improvement on performance.

I left 2 comments: one nit(up to you to decide what to do) and one observation

}, [memoDataViewId, data.dataViews, setDataViewIndexPatterns, activeSpaceId]);

// Fetch extended fields information
const getExtendedField = useCallback(
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: getExtendedField can retrieve array of fields, so probably name getExtendedFields would be more suitable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, will update the name

}
};
fetchExtendedField();
}, [entry.field?.name, getExtendedField]);
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not very familiar with this functionality, but it looks like there is an initial request that loads all the fields at the flyout's opening.

When I open flyout, I can see the following request


http://localhost:5601/kbn/api/index_patterns/_fields_for_wildcard?pattern=apm-*-transaction*%2Cauditbeat-*%2Cendgame-*%2Cfilebeat-*%2Clogs-*%2Cpacketbeat-*%2Ctraces-apm*%2Cwinlogbeat-*%2C-*elastic-cloud-logs-*%2Cnon-ecs&meta_fields=_source&meta_fields=_id&meta_fields=_index&meta_fields=_score&allow_no_index=true

Which returns 8000 fields

Within the list of the fields, there is a conflicting one, I created

{
   "name":"agent.id",
   "type":"conflict",
   "esTypes":[
      "keyword",
      "long"
   ],
   "searchable":true,
   "aggregatable":true,
   "readFromDocValues":false,
   "conflictDescriptions":{
      "keyword":[
         ".ds-auditbeat-8.4.2-2023.06.13-000001",
         ".ds-filebeat-8.5.2-2023.06.13-000001",
         ".ds-packetbeat-8.5.3-2023.06.13-000001"
      ],
      "long":[
         "non-ecs"
      ]
   },
   "metadata_field":false
}

When I select, this field in controller, another request is sent, to retrieve this field


http://localhost:5601/kbn/api/index_patterns/_fields_for_wildcard?pattern=apm-*-transaction*%2Cauditbeat-*%2Cendgame-*%2Cfilebeat-*%2Clogs-*%2Cpacketbeat-*%2Ctraces-apm*%2Cwinlogbeat-*%2C-*elastic-cloud-logs-*%2Cnon-ecs&meta_fields=_source&meta_fields=_id&meta_fields=_index&meta_fields=_score&allow_no_index=true&include_unmapped=true&fields=agent.id

The result is identical

{
   "name":"agent.id",
   "type":"conflict",
   "esTypes":[
      "keyword",
      "long"
   ],
   "searchable":true,
   "aggregatable":true,
   "readFromDocValues":false,
   "conflictDescriptions":{
      "keyword":[
         ".ds-auditbeat-8.4.2-2023.06.13-000001",
         ".ds-filebeat-8.5.2-2023.06.13-000001",
         ".ds-packetbeat-8.5.3-2023.06.13-000001"
      ],
      "long":[
         "non-ecs"
      ]
   },
   "metadata_field":false
}

It looks like, we still retrieve all the fields at the beginning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, conflicts are always returned if those exist.

Though, the unmapped information will be returned only when include_unmapped=true parameter specified. Users usually have lots of unmapped fields and this can produce huge responses when we call _fields_for_wildcard with include_unmapped=true for all fields.

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for explaining and adding to notes.
How comparably larger could be response when include_unmapped=true?
8k items seems quite large already

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can be a really huge difference. Cannot tell exactly in numbers, but what happens when we add include_unmapped info is:

  • For each field that is unmapped in some indices we would receive conflictDescriptions with the whole list of available indices (some will be in "unmapped" array and the rest in the "field_type" array).

Let's say we have 8k fields and we search through 100 indices. If half of the fields are unmapped in some of the indices for whatever reason (indices store similar but different type of data; we add with time new fields etc.), we will end up with 4k fields * 100 indices extra lines in the response.

@e40pud
Copy link
Contributor Author

e40pud commented Jun 13, 2023

@elasticmachine merge upstream

@e40pud e40pud requested a review from vitaliidm June 13, 2023 14:25
Copy link
Contributor

@vitaliidm vitaliidm left a comment

Choose a reason for hiding this comment

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

Thanks for addressing comments!

// Fetch extended fields information
const getExtendedFields = useCallback(
async (fields: string[]) => {
let extendedFields: DataViewField[] = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the response type of getFieldsForIndexPattern is FieldSpec[], not DataViewField[]

getFieldsForIndexPattern: (
indexPattern: DataView | DataViewSpec,
options?: GetFieldsOptions | undefined
) => Promise<FieldSpec[]>;

Suggested change
let extendedFields: DataViewField[] = [];
let extendedFields: FieldSpec[] = [];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will add a separate commit, as I also will need to update imports for these changes

Copy link
Contributor

@yctercero yctercero left a comment

Choose a reason for hiding this comment

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

Approving to remove security-solution-platform review request. Vitalii approved for team detection-engine.

@e40pud
Copy link
Contributor Author

e40pud commented Jun 14, 2023

@elasticmachine merge upstream

@e40pud
Copy link
Contributor Author

e40pud commented Jun 14, 2023

@elasticmachine merge upstream

@e40pud
Copy link
Contributor Author

e40pud commented Jun 14, 2023

@elasticmachine merge upstream

@e40pud e40pud enabled auto-merge (squash) June 14, 2023 23:08
@e40pud
Copy link
Contributor Author

e40pud commented Jun 15, 2023

@elasticmachine merge upstream

@e40pud
Copy link
Contributor Author

e40pud commented Jun 15, 2023

@elasticmachine merge upstream

@e40pud
Copy link
Contributor Author

e40pud commented Jun 15, 2023

@elasticmachine merge upstream

@e40pud
Copy link
Contributor Author

e40pud commented Jun 15, 2023

@elasticmachine merge upstream

@e40pud
Copy link
Contributor Author

e40pud commented Jun 15, 2023

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

kibana-ci commented Jun 15, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Explore - Security Solution Tests #2 / Hover actions Adds to timeline
  • [job] [logs] FTR Configs #13 / Index Management app Index template wizard Mappings step clearing up the Numeric subtype dropdown doesn't break the page

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
lists 159.1KB 159.5KB +446.0B
securitySolution 10.8MB 10.8MB +57.0B
total +503.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 13 15 +2
securitySolution 410 414 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 14 16 +2
securitySolution 493 497 +4
total +6

History

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

cc @e40pud

@e40pud e40pud merged commit 31b3477 into elastic:main Jun 15, 2023
27 checks passed
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 15, 2023
…dices with huge amount of fields (elastic#159216)

## Summary

Original ticket:
[elastic#158751](elastic#158751)

These changes improve the rule's exceptions flyout opening experience.
We had a few complaints that it is very slow to open it and sometimes it
throws an exception about the limited response size.

To fix this, we decided to load extended field's data (conflicts and
unmapped info) only when user selects some field instead of fetching
this data for all fields on flyout opening.

## NOTES:

After these changes we gonna do next steps related to fields loading
when user creates/edits rule exceptions:
1. We will call `_fields_for_wildcard` **WITHOUT**
`include_unmapped=true` parameter to fetch all fields specs on exception
flyout loading
2. We will call `_fields_for_wildcard` **WITH** `include_unmapped=true`
for only one field when user selects it from the dropdown menu

With these changes we will improve slow exception flyout opening when
user has lots of fields which are unmapped in different indices. If for
some reason user has a lot of (thousands) conflicting fields around
indices then the loading is still might be slow as the
`_fields_for_wildcard` call will return conflicts information even
without `include_unmapped=true` parameter.

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit 31b3477)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.8

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Jun 15, 2023
…the indices with huge amount of fields (#159216) (#159801)

# Backport

This will backport the following commits from `main` to `8.8`:
- [[Security Solution] Improve rules exception flyout opening for the
indices with huge amount of fields
(#159216)](#159216)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Ievgen
Sorokopud","email":"ievgen.sorokopud@elastic.co"},"sourceCommit":{"committedDate":"2023-06-15T12:57:15Z","message":"[Security
Solution] Improve rules exception flyout opening for the indices with
huge amount of fields (#159216)\n\n## Summary\r\n\r\nOriginal
ticket:\r\n[#158751](#158751
changes improve the rule's exceptions flyout opening experience.\r\nWe
had a few complaints that it is very slow to open it and sometimes
it\r\nthrows an exception about the limited response size.\r\n\r\nTo fix
this, we decided to load extended field's data (conflicts
and\r\nunmapped info) only when user selects some field instead of
fetching\r\nthis data for all fields on flyout opening.\r\n\r\n##
NOTES:\r\n\r\nAfter these changes we gonna do next steps related to
fields loading\r\nwhen user creates/edits rule exceptions:\r\n1. We will
call `_fields_for_wildcard` **WITHOUT**\r\n`include_unmapped=true`
parameter to fetch all fields specs on exception\r\nflyout loading\r\n2.
We will call `_fields_for_wildcard` **WITH**
`include_unmapped=true`\r\nfor only one field when user selects it from
the dropdown menu\r\n\r\nWith these changes we will improve slow
exception flyout opening when\r\nuser has lots of fields which are
unmapped in different indices. If for\r\nsome reason user has a lot of
(thousands) conflicting fields around\r\nindices then the loading is
still might be slow as the\r\n`_fields_for_wildcard` call will return
conflicts information even\r\nwithout `include_unmapped=true`
parameter.\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"31b34771c5e6f710858a7f617bbca04537cf5c1b","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:
SecuritySolution","ci:cloud-deploy","v8.9.0","Team:Detection
Engine","v8.8.2"],"number":159216,"url":"#159216
Solution] Improve rules exception flyout opening for the indices with
huge amount of fields (#159216)\n\n## Summary\r\n\r\nOriginal
ticket:\r\n[#158751](#158751
changes improve the rule's exceptions flyout opening experience.\r\nWe
had a few complaints that it is very slow to open it and sometimes
it\r\nthrows an exception about the limited response size.\r\n\r\nTo fix
this, we decided to load extended field's data (conflicts
and\r\nunmapped info) only when user selects some field instead of
fetching\r\nthis data for all fields on flyout opening.\r\n\r\n##
NOTES:\r\n\r\nAfter these changes we gonna do next steps related to
fields loading\r\nwhen user creates/edits rule exceptions:\r\n1. We will
call `_fields_for_wildcard` **WITHOUT**\r\n`include_unmapped=true`
parameter to fetch all fields specs on exception\r\nflyout loading\r\n2.
We will call `_fields_for_wildcard` **WITH**
`include_unmapped=true`\r\nfor only one field when user selects it from
the dropdown menu\r\n\r\nWith these changes we will improve slow
exception flyout opening when\r\nuser has lots of fields which are
unmapped in different indices. If for\r\nsome reason user has a lot of
(thousands) conflicting fields around\r\nindices then the loading is
still might be slow as the\r\n`_fields_for_wildcard` call will return
conflicts information even\r\nwithout `include_unmapped=true`
parameter.\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"31b34771c5e6f710858a7f617bbca04537cf5c1b"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"#159216
Solution] Improve rules exception flyout opening for the indices with
huge amount of fields (#159216)\n\n## Summary\r\n\r\nOriginal
ticket:\r\n[#158751](#158751
changes improve the rule's exceptions flyout opening experience.\r\nWe
had a few complaints that it is very slow to open it and sometimes
it\r\nthrows an exception about the limited response size.\r\n\r\nTo fix
this, we decided to load extended field's data (conflicts
and\r\nunmapped info) only when user selects some field instead of
fetching\r\nthis data for all fields on flyout opening.\r\n\r\n##
NOTES:\r\n\r\nAfter these changes we gonna do next steps related to
fields loading\r\nwhen user creates/edits rule exceptions:\r\n1. We will
call `_fields_for_wildcard` **WITHOUT**\r\n`include_unmapped=true`
parameter to fetch all fields specs on exception\r\nflyout loading\r\n2.
We will call `_fields_for_wildcard` **WITH**
`include_unmapped=true`\r\nfor only one field when user selects it from
the dropdown menu\r\n\r\nWith these changes we will improve slow
exception flyout opening when\r\nuser has lots of fields which are
unmapped in different indices. If for\r\nsome reason user has a lot of
(thousands) conflicting fields around\r\nindices then the loading is
still might be slow as the\r\n`_fields_for_wildcard` call will return
conflicts information even\r\nwithout `include_unmapped=true`
parameter.\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"31b34771c5e6f710858a7f617bbca04537cf5c1b"}},{"branch":"8.8","label":"v8.8.2","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Ievgen Sorokopud <ievgen.sorokopud@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:cloud-deploy Create or update a Cloud deployment release_note:fix Team:Detection Engine Security Solution Detection Engine Area Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.8.2 v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants