Skip to content

Don't fetch data view on store initialization#177007

Merged
nkhristinin merged 12 commits intoelastic:mainfrom
nkhristinin:redux-dataview
Apr 26, 2024
Merged

Don't fetch data view on store initialization#177007
nkhristinin merged 12 commits intoelastic:mainfrom
nkhristinin:redux-dataview

Conversation

@nkhristinin
Copy link
Copy Markdown
Contributor

@nkhristinin nkhristinin commented Feb 15, 2024

Sourcerer page load improvements

This PR contains 2 fixes:

  1. Don't fetch data view when we initialise redux store, which blocks page loading.
  2. Don't fetch the default data view, when we open the alerts page, and just use @timestamp

I added a 3-second delay for this API request, so you can see that the new version doesn't block page load

Before

Screen.Recording.2024-03-11.at.14.12.13.mov

After

Screen.Recording.2024-03-11.at.13.50.25.mov

Testing needed

As we change how we initialise sourcerer, additional testing is needed, as I maybe don't manually tests all corner cases

@nkhristinin
Copy link
Copy Markdown
Contributor Author

/ci

@nkhristinin
Copy link
Copy Markdown
Contributor Author

/ci

@nkhristinin
Copy link
Copy Markdown
Contributor Author

/ci

@nkhristinin
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@nkhristinin
Copy link
Copy Markdown
Contributor Author

/ci

@nkhristinin
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@nkhristinin nkhristinin marked this pull request as ready for review April 9, 2024 09:57
@nkhristinin nkhristinin requested a review from a team as a code owner April 9, 2024 09:57
@nkhristinin nkhristinin added the release_note:skip Skip the PR/issue when compiling release notes label Apr 9, 2024
@yctercero yctercero changed the title Don't fetch data view on store initialisation Don't fetch data view on store initialization Apr 10, 2024
Copy link
Copy Markdown
Contributor

@kqualters-elastic kqualters-elastic left a comment

Choose a reason for hiding this comment

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

tested locally and now see a loading indicator on initial page load while the sourcerer api calls run, as opposed to a blank page before, with everything else working the same. LGTM 👍

@nkhristinin
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@nkhristinin
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@nkhristinin nkhristinin enabled auto-merge (squash) April 15, 2024 14:12
@nkhristinin nkhristinin disabled auto-merge April 15, 2024 14:15
@nkhristinin
Copy link
Copy Markdown
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Copy Markdown
Contributor

merge conflict between base and head

@nkhristinin nkhristinin enabled auto-merge (squash) April 26, 2024 08:20
@nkhristinin nkhristinin merged commit aa0d52a into elastic:main Apr 26, 2024
@kibana-ci
Copy link
Copy Markdown

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #57 / endpoint "before all" hook in "endpoint"

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 17.3MB 17.4MB +6.9KB

History

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

@kibanamachine kibanamachine added v8.15.0 backport:skip This PR does not require backporting labels Apr 26, 2024
nkhristinin added a commit that referenced this pull request May 1, 2024
## Exceptions not created for mappings with conflicts

related: #177007


How to reproduce

- Execute request in order, and change timestamps to you actual dates
```
PUT test-1
{
  "mappings": {
    "properties": {
      "@timestamp": {"type": "date"},
      "host.name": { "type": "keyword" }
    }
  }
}

PUT auditbeat-1
{
  "mappings": {
    "properties": {
      "@timestamp": {"type": "date"},
      "host.name": { "type": "boolean" }
    }
  }
}

POST auditbeat-1/_doc
{
  "@timestamp": 1714121723789,
  "host.name":"host 11",
  "user.name": "123"
}

POST test-1/_doc
{
  "@timestamp": 1714121723789,
  "host.name":"host 11",
  "user.name": "123"
}

```

- Create a query rule, with index pattern `test-1` and the query: '*'.
Wait for the alert to be generated
- Go to the Alerts page (not from the Rule, but all alerts)
- Click add exception from the alert and observe that it says that
fields have conflict about auditbeat index. Which is incorrect as a rule
has only `test-1` index pattern.
- Check all checkboxes about closing alerts and creating exceptions. See
that there is no success toast, and the rule also has no exception.
- Open flyout exception again. Observe that there is no more message
about conflicts. If you create an exception it will be successful.




https://github.com/elastic/kibana/assets/7609147/d29981f7-5d6f-41b2-af46-2ade884e3563


### Reason

In the `AddExceptionFlyout` we do have this line

```
 const { isLoading, indexPatterns, getExtendedFields } = useFetchIndexPatterns(rules);
```

From `AlertContextMenuComponent` we are passing try to load the rule
info and first past `undefined` to rules and then some real info.

The problem is that in the `useFetchIndexPatterns` - if rules not
defined, we return `getExtendedFields` related to the default data view,
but no there rule index pattern.

And probably there some race conditions happen, and after we pass rules
props, exceptions don't rerender the info.

As a fix for this BC I just wait for rule info to be loaded, and then
render exception flyout, but it only masks the problem.

As we want to fix this bug faster, it's probably ok to merge it like
that. And there will be an issue to address this later.

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request May 1, 2024
## Exceptions not created for mappings with conflicts

related: elastic#177007

How to reproduce

- Execute request in order, and change timestamps to you actual dates
```
PUT test-1
{
  "mappings": {
    "properties": {
      "@timestamp": {"type": "date"},
      "host.name": { "type": "keyword" }
    }
  }
}

PUT auditbeat-1
{
  "mappings": {
    "properties": {
      "@timestamp": {"type": "date"},
      "host.name": { "type": "boolean" }
    }
  }
}

POST auditbeat-1/_doc
{
  "@timestamp": 1714121723789,
  "host.name":"host 11",
  "user.name": "123"
}

POST test-1/_doc
{
  "@timestamp": 1714121723789,
  "host.name":"host 11",
  "user.name": "123"
}

```

- Create a query rule, with index pattern `test-1` and the query: '*'.
Wait for the alert to be generated
- Go to the Alerts page (not from the Rule, but all alerts)
- Click add exception from the alert and observe that it says that
fields have conflict about auditbeat index. Which is incorrect as a rule
has only `test-1` index pattern.
- Check all checkboxes about closing alerts and creating exceptions. See
that there is no success toast, and the rule also has no exception.
- Open flyout exception again. Observe that there is no more message
about conflicts. If you create an exception it will be successful.

https://github.com/elastic/kibana/assets/7609147/d29981f7-5d6f-41b2-af46-2ade884e3563

### Reason

In the `AddExceptionFlyout` we do have this line

```
 const { isLoading, indexPatterns, getExtendedFields } = useFetchIndexPatterns(rules);
```

From `AlertContextMenuComponent` we are passing try to load the rule
info and first past `undefined` to rules and then some real info.

The problem is that in the `useFetchIndexPatterns` - if rules not
defined, we return `getExtendedFields` related to the default data view,
but no there rule index pattern.

And probably there some race conditions happen, and after we pass rules
props, exceptions don't rerender the info.

As a fix for this BC I just wait for rule info to be loaded, and then
render exception flyout, but it only masks the problem.

As we want to fix this bug faster, it's probably ok to merge it like
that. And there will be an issue to address this later.

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
(cherry picked from commit 4aabce5)
kibanamachine referenced this pull request May 3, 2024
… (#182286)

# Backport

This will backport the following commits from `main` to `8.14`:
- [Don&#x27;t render exceptions flyout if data is loading
(#181588)](#181588)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Khristinin
Nikita","email":"nikita.khristinin@elastic.co"},"sourceCommit":{"committedDate":"2024-05-01T18:58:38Z","message":"Don't
render exceptions flyout if data is loading (#181588)\n\n## Exceptions
not created for mappings with conflicts\r\n\r\nrelated:
https://github.com/elastic/kibana/pull/177007\r\n\r\n\r\nHow to
reproduce\r\n\r\n- Execute request in order, and change timestamps to
you actual dates\r\n```\r\nPUT test-1\r\n{\r\n \"mappings\": {\r\n
\"properties\": {\r\n \"@timestamp\": {\"type\": \"date\"},\r\n
\"host.name\": { \"type\": \"keyword\" }\r\n }\r\n }\r\n}\r\n\r\nPUT
auditbeat-1\r\n{\r\n \"mappings\": {\r\n \"properties\": {\r\n
\"@timestamp\": {\"type\": \"date\"},\r\n \"host.name\": { \"type\":
\"boolean\" }\r\n }\r\n }\r\n}\r\n\r\nPOST auditbeat-1/_doc\r\n{\r\n
\"@timestamp\": 1714121723789,\r\n \"host.name\":\"host 11\",\r\n
\"user.name\": \"123\"\r\n}\r\n\r\nPOST test-1/_doc\r\n{\r\n
\"@timestamp\": 1714121723789,\r\n \"host.name\":\"host 11\",\r\n
\"user.name\": \"123\"\r\n}\r\n\r\n```\r\n\r\n- Create a query rule,
with index pattern `test-1` and the query: '*'.\r\nWait for the alert to
be generated\r\n- Go to the Alerts page (not from the Rule, but all
alerts)\r\n- Click add exception from the alert and observe that it says
that\r\nfields have conflict about auditbeat index. Which is incorrect
as a rule\r\nhas only `test-1` index pattern.\r\n- Check all checkboxes
about closing alerts and creating exceptions. See\r\nthat there is no
success toast, and the rule also has no exception.\r\n- Open flyout
exception again. Observe that there is no more message\r\nabout
conflicts. If you create an exception it will be
successful.\r\n\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/7609147/d29981f7-5d6f-41b2-af46-2ade884e3563\r\n\r\n\r\n###
Reason\r\n\r\nIn the `AddExceptionFlyout` we do have this
line\r\n\r\n```\r\n const { isLoading, indexPatterns, getExtendedFields
} = useFetchIndexPatterns(rules);\r\n```\r\n\r\nFrom
`AlertContextMenuComponent` we are passing try to load the rule\r\ninfo
and first past `undefined` to rules and then some real info.\r\n\r\nThe
problem is that in the `useFetchIndexPatterns` - if rules
not\r\ndefined, we return `getExtendedFields` related to the default
data view,\r\nbut no there rule index pattern.\r\n\r\nAnd probably there
some race conditions happen, and after we pass rules\r\nprops,
exceptions don't rerender the info.\r\n\r\nAs a fix for this BC I just
wait for rule info to be loaded, and then\r\nrender exception flyout,
but it only masks the problem.\r\n\r\nAs we want to fix this bug faster,
it's probably ok to merge it like\r\nthat. And there will be an issue to
address this later.\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana
Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"4aabce5bcd9aa625c85af3f7dde9bc3b675d5344","branchLabelMapping":{"^v8.15.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport:prev-minor","v8.15.0"],"title":"Don't
render exceptions flyout if data is
loading","number":181588,"url":"https://github.com/elastic/kibana/pull/181588","mergeCommit":{"message":"Don't
render exceptions flyout if data is loading (#181588)\n\n## Exceptions
not created for mappings with conflicts\r\n\r\nrelated:
https://github.com/elastic/kibana/pull/177007\r\n\r\n\r\nHow to
reproduce\r\n\r\n- Execute request in order, and change timestamps to
you actual dates\r\n```\r\nPUT test-1\r\n{\r\n \"mappings\": {\r\n
\"properties\": {\r\n \"@timestamp\": {\"type\": \"date\"},\r\n
\"host.name\": { \"type\": \"keyword\" }\r\n }\r\n }\r\n}\r\n\r\nPUT
auditbeat-1\r\n{\r\n \"mappings\": {\r\n \"properties\": {\r\n
\"@timestamp\": {\"type\": \"date\"},\r\n \"host.name\": { \"type\":
\"boolean\" }\r\n }\r\n }\r\n}\r\n\r\nPOST auditbeat-1/_doc\r\n{\r\n
\"@timestamp\": 1714121723789,\r\n \"host.name\":\"host 11\",\r\n
\"user.name\": \"123\"\r\n}\r\n\r\nPOST test-1/_doc\r\n{\r\n
\"@timestamp\": 1714121723789,\r\n \"host.name\":\"host 11\",\r\n
\"user.name\": \"123\"\r\n}\r\n\r\n```\r\n\r\n- Create a query rule,
with index pattern `test-1` and the query: '*'.\r\nWait for the alert to
be generated\r\n- Go to the Alerts page (not from the Rule, but all
alerts)\r\n- Click add exception from the alert and observe that it says
that\r\nfields have conflict about auditbeat index. Which is incorrect
as a rule\r\nhas only `test-1` index pattern.\r\n- Check all checkboxes
about closing alerts and creating exceptions. See\r\nthat there is no
success toast, and the rule also has no exception.\r\n- Open flyout
exception again. Observe that there is no more message\r\nabout
conflicts. If you create an exception it will be
successful.\r\n\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/7609147/d29981f7-5d6f-41b2-af46-2ade884e3563\r\n\r\n\r\n###
Reason\r\n\r\nIn the `AddExceptionFlyout` we do have this
line\r\n\r\n```\r\n const { isLoading, indexPatterns, getExtendedFields
} = useFetchIndexPatterns(rules);\r\n```\r\n\r\nFrom
`AlertContextMenuComponent` we are passing try to load the rule\r\ninfo
and first past `undefined` to rules and then some real info.\r\n\r\nThe
problem is that in the `useFetchIndexPatterns` - if rules
not\r\ndefined, we return `getExtendedFields` related to the default
data view,\r\nbut no there rule index pattern.\r\n\r\nAnd probably there
some race conditions happen, and after we pass rules\r\nprops,
exceptions don't rerender the info.\r\n\r\nAs a fix for this BC I just
wait for rule info to be loaded, and then\r\nrender exception flyout,
but it only masks the problem.\r\n\r\nAs we want to fix this bug faster,
it's probably ok to merge it like\r\nthat. And there will be an issue to
address this later.\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana
Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"4aabce5bcd9aa625c85af3f7dde9bc3b675d5344"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.15.0","branchLabelMappingKey":"^v8.15.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/181588","number":181588,"mergeCommit":{"message":"Don't
render exceptions flyout if data is loading (#181588)\n\n## Exceptions
not created for mappings with conflicts\r\n\r\nrelated:
https://github.com/elastic/kibana/pull/177007\r\n\r\n\r\nHow to
reproduce\r\n\r\n- Execute request in order, and change timestamps to
you actual dates\r\n```\r\nPUT test-1\r\n{\r\n \"mappings\": {\r\n
\"properties\": {\r\n \"@timestamp\": {\"type\": \"date\"},\r\n
\"host.name\": { \"type\": \"keyword\" }\r\n }\r\n }\r\n}\r\n\r\nPUT
auditbeat-1\r\n{\r\n \"mappings\": {\r\n \"properties\": {\r\n
\"@timestamp\": {\"type\": \"date\"},\r\n \"host.name\": { \"type\":
\"boolean\" }\r\n }\r\n }\r\n}\r\n\r\nPOST auditbeat-1/_doc\r\n{\r\n
\"@timestamp\": 1714121723789,\r\n \"host.name\":\"host 11\",\r\n
\"user.name\": \"123\"\r\n}\r\n\r\nPOST test-1/_doc\r\n{\r\n
\"@timestamp\": 1714121723789,\r\n \"host.name\":\"host 11\",\r\n
\"user.name\": \"123\"\r\n}\r\n\r\n```\r\n\r\n- Create a query rule,
with index pattern `test-1` and the query: '*'.\r\nWait for the alert to
be generated\r\n- Go to the Alerts page (not from the Rule, but all
alerts)\r\n- Click add exception from the alert and observe that it says
that\r\nfields have conflict about auditbeat index. Which is incorrect
as a rule\r\nhas only `test-1` index pattern.\r\n- Check all checkboxes
about closing alerts and creating exceptions. See\r\nthat there is no
success toast, and the rule also has no exception.\r\n- Open flyout
exception again. Observe that there is no more message\r\nabout
conflicts. If you create an exception it will be
successful.\r\n\r\n\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/7609147/d29981f7-5d6f-41b2-af46-2ade884e3563\r\n\r\n\r\n###
Reason\r\n\r\nIn the `AddExceptionFlyout` we do have this
line\r\n\r\n```\r\n const { isLoading, indexPatterns, getExtendedFields
} = useFetchIndexPatterns(rules);\r\n```\r\n\r\nFrom
`AlertContextMenuComponent` we are passing try to load the rule\r\ninfo
and first past `undefined` to rules and then some real info.\r\n\r\nThe
problem is that in the `useFetchIndexPatterns` - if rules
not\r\ndefined, we return `getExtendedFields` related to the default
data view,\r\nbut no there rule index pattern.\r\n\r\nAnd probably there
some race conditions happen, and after we pass rules\r\nprops,
exceptions don't rerender the info.\r\n\r\nAs a fix for this BC I just
wait for rule info to be loaded, and then\r\nrender exception flyout,
but it only masks the problem.\r\n\r\nAs we want to fix this bug faster,
it's probably ok to merge it like\r\nthat. And there will be an issue to
address this later.\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana
Machine
<42973632+kibanamachine@users.noreply.github.com>","sha":"4aabce5bcd9aa625c85af3f7dde9bc3b675d5344"}}]}]
BACKPORT-->

Co-authored-by: Khristinin Nikita <nikita.khristinin@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes v8.15.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants