Skip to content

[Synthetics] Exp view reload state on missing data#152286

Merged
shahzad31 merged 9 commits intoelastic:mainfrom
shahzad31:update-data-view
Mar 2, 2023
Merged

[Synthetics] Exp view reload state on missing data#152286
shahzad31 merged 9 commits intoelastic:mainfrom
shahzad31:update-data-view

Conversation

@shahzad31
Copy link
Copy Markdown
Contributor

@shahzad31 shahzad31 commented Feb 27, 2023

Summary

Exp view reload state on missing data

When there is no data present , earlier it wasn't loading the state auto. this fix will make sure to retry for loading dataview in case of failure.

On auto refresh or user refresh, it will update the state to resolve it. if data is present by the time.
image

@shahzad31 shahzad31 marked this pull request as ready for review February 28, 2023 12:13
@shahzad31 shahzad31 requested a review from a team as a code owner February 28, 2023 12:13
@shahzad31 shahzad31 requested a review from a team February 28, 2023 12:13
@shahzad31 shahzad31 requested a review from a team as a code owner February 28, 2023 12:13
@shahzad31 shahzad31 added v8.7.0 v8.8.0 release_note:skip Skip the PR/issue when compiling release notes and removed v8.7.0 labels Feb 28, 2023
@dominiqueclarke dominiqueclarke added the ci:cloud-deploy Create or update a Cloud deployment label Feb 28, 2023
@mattkime
Copy link
Copy Markdown
Contributor

mattkime commented Mar 1, 2023

@shahzad31 The changes look good, but I'm curious about the problem you're trying to solve. Is it common that someone is looking at this view as data is loaded? I'm wondering if this is something I should be supporting in the data views api.

@shahzad31
Copy link
Copy Markdown
Contributor Author

@shahzad31 The changes look good, but I'm curious about the problem you're trying to solve. Is it common that someone is looking at this view as data is loaded? I'm wondering if this is something I should be supporting in the data views api.

@mattkime yes so essentially when user adds monitor via ui, we wait for the data being loaded and when monitors start populating data. we refresh silently and show the data at it comes.

@botelastic botelastic bot added the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Mar 1, 2023
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/uptime (Team:uptime)

return await this.createAndSavedDataView(app, appIndices);
}
if (e instanceof DataViewMissingIndices) {
this.dataViews.clearInstanceCache();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of clearing the cache you could refresh the field list for a given data view - dataViews.refreshFields( dataView)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

that sounds better. let me try that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

actually that won't work here because in this case we don't know if the it has already tried fetching or not.
So we clear cache, and when the next refersh happens. it won't reuse the cache.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you pass the data view id to clearInstanceCache?

shahzad31 and others added 2 commits March 2, 2023 10:00
Co-authored-by: Abdul Wahab Zahid <awahab07@yahoo.com>
@shahzad31 shahzad31 requested a review from awahab07 March 2, 2023 11:58
@shahzad31 shahzad31 requested a review from mattkime March 2, 2023 16:06
Copy link
Copy Markdown
Contributor

@mattkime mattkime left a comment

Choose a reason for hiding this comment

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

data view code lgtm

@kibana-ci
Copy link
Copy Markdown

kibana-ci commented Mar 2, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
dataViews 243 245 +2

Page load bundle

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

id before after diff
dataViews 44.0KB 44.2KB +130.0B
observability 89.1KB 89.2KB +154.0B
total +284.0B
Unknown metric groups

API count

id before after diff
dataViews 1028 1031 +3

ESLint disabled line counts

id before after diff
observability 52 53 +1
securitySolution 428 430 +2
total +3

Total ESLint disabled count

id before after diff
observability 58 59 +1
securitySolution 506 508 +2
total +3

History

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

Copy link
Copy Markdown
Contributor

@awahab07 awahab07 left a comment

Choose a reason for hiding this comment

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

LGTM.

Tested and works as described.

@shahzad31 shahzad31 merged commit 525638d into elastic:main Mar 2, 2023
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Mar 2, 2023
@shahzad31 shahzad31 deleted the update-data-view branch March 2, 2023 19:34
@shahzad31 shahzad31 added v8.7.0 auto-backport Deprecated - use backport:version if exact versions are needed and removed backport:skip This PR does not require backporting labels Mar 2, 2023
@kibanamachine
Copy link
Copy Markdown
Contributor

💔 All backports failed

Status Branch Result
8.7 Backport failed because of merge conflicts

You might need to backport the following PRs to 8.7:
- [Lens][Embeddable] Reset removable error list on search/context update (#152489)
- [Fleet] Disable FQDN selector by default (#152592)

Manual backport

To create the backport manually run:

node scripts/backport --pr 152286

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

@shahzad31
Copy link
Copy Markdown
Contributor Author

💚 All backports created successfully

Status Branch Result
8.7

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

Questions ?

Please refer to the Backport tool documentation

shahzad31 added a commit to shahzad31/kibana that referenced this pull request Mar 2, 2023
Co-authored-by: Abdul Wahab Zahid <awahab07@yahoo.com>
(cherry picked from commit 525638d)
shahzad31 added a commit that referenced this pull request Mar 2, 2023
…152616)

# Backport

This will backport the following commits from `main` to `8.7`:
- [[Synthetics] Exp view reload state on missing data
(#152286)](#152286)

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

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

<!--BACKPORT
[{"author":{"name":"Shahzad","email":"shahzad31comp@gmail.com"},"sourceCommit":{"committedDate":"2023-03-02T19:33:56Z","message":"[Synthetics]
Exp view reload state on missing data (#152286)\n\nCo-authored-by: Abdul
Wahab Zahid
<awahab07@yahoo.com>","sha":"525638d5e73e27bbf3d9f34ec8b0649930c964ec","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:uptime","release_note:skip","auto-backport","ci:cloud-deploy","v8.7.0","v8.8.0"],"number":152286,"url":"https://github.com/elastic/kibana/pull/152286","mergeCommit":{"message":"[Synthetics]
Exp view reload state on missing data (#152286)\n\nCo-authored-by: Abdul
Wahab Zahid
<awahab07@yahoo.com>","sha":"525638d5e73e27bbf3d9f34ec8b0649930c964ec"}},"sourceBranch":"main","suggestedTargetBranches":["8.7"],"targetPullRequestStates":[{"branch":"8.7","label":"v8.7.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/152286","number":152286,"mergeCommit":{"message":"[Synthetics]
Exp view reload state on missing data (#152286)\n\nCo-authored-by: Abdul
Wahab Zahid
<awahab07@yahoo.com>","sha":"525638d5e73e27bbf3d9f34ec8b0649930c964ec"}}]}]
BACKPORT-->
bmorelli25 pushed a commit to bmorelli25/kibana that referenced this pull request Mar 10, 2023
Co-authored-by: Abdul Wahab Zahid <awahab07@yahoo.com>
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 ci:cloud-deploy Create or update a Cloud deployment release_note:skip Skip the PR/issue when compiling release notes Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v8.7.0 v8.8.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants