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.

Sign up for GitHub

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

Jump to bottom

Fix: Consider all summary pings to determine if monitor is only fleet managed. #142004

Merged
merged 4 commits into from
Oct 4, 2022
Merged

Fix: Consider all summary pings to determine if monitor is only fleet managed. #142004

merged 4 commits into from
Oct 4, 2022

Conversation

awahab07
Copy link
Contributor

@awahab07 awahab07 commented Sep 27, 2022 •
edited by kibanamachine
Loading

Fixes #138862
Fixes #140768

Summary

  • Fixes the behavior where Run now button on Uptime -> Monitor Management -> Add/Edit page would try to initiate a pre-flight (run_once) test for both public and private locations, in which case, the test for private location never finishes. Also some tweaks are applied to the Run now tooltip (popover).
  • Fixes the behavior on Uptime Overview page where running an on-demand test (Test now, the button with play icon) for a monitor configured for both public and private locations would disable the Test now button on on-demand test completion.

How to test

  1. Set up a cluster so that you are able to create and run a monitor on at least one public and at least one private location (Only a shallow private location with no connected agent may also work for this test but a thorough e2e test is recommended). Or you may use the deployment if it's still live.
  2. Create the monitor via Uptime -> Monitor Management and test the Test now behavior before saving and make sure the pre-flight test is only ran for public locations.
  3. Save the monitor and wait for the results to get ingested.
  4. Goto Uptime -> Overview page and make sure the monitor appears there.
  5. Test the on-demand test functionality via the Test now button (the button with play icon) and make sure the button stays enabled after the test completes.

Screenshot 2022-09-28 at 20 23 42

Screenshot 2022-09-28 at 21 40 38

Screenshot 2022-09-28 at 21 43 46

@awahab07 awahab07 added bug Fixes for quality problems that affect the customer experience Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v8.5.0 ci:cloud-deploy Create or update a Cloud deployment labels Sep 27, 2022
@awahab07 awahab07 changed the title Consider all ping to determine if monitor is only fleet managed. Consider all summary pings to determine if monitor is only fleet managed. Sep 28, 2022
@awahab07 awahab07 force-pushed the fix/Test-Run-behavior-for-Private-Locations branch from 2bf833e to 7d18fd7 Compare September 28, 2022 16:06
@awahab07 awahab07 marked this pull request as ready for review September 28, 2022 16:07
@awahab07 awahab07 requested a review from a team as a code owner September 28, 2022 16:07
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@awahab07 awahab07 added the release_note:skip Skip the PR/issue when compiling release notes label Sep 28, 2022
@awahab07 awahab07 force-pushed the fix/Test-Run-behavior-for-Private-Locations branch from 7d18fd7 to 11eb12f Compare September 28, 2022 19:19
@awahab07 awahab07 changed the title Consider all summary pings to determine if monitor is only fleet managed. Fix: Consider all summary pings to determine if monitor is only fleet managed. Sep 28, 2022
@andrewvc
Copy link
Contributor

As I read the description this looks at result data to determine if the "run once" functionality should work and where it should work. Is that right? Isn't there a way to interrogate the saved objects instead?

@awahab07
Copy link
Contributor Author

awahab07 commented Oct 4, 2022

As I read the description this looks at result data to determine if the "run once" functionality should work and where it should work. Is that right? Isn't there a way to interrogate the saved objects instead?

Yes, it'd be neat to fetch the Saved Object to determine the status. Just that, on the Uptime overview page, we don't query the saved objects at all for anything. But I think, it's fine to query the saved object and disable "run once" if:

  1. Saved Object tells the monitor is only fleet_managed.
  2. Saved Object is not available against monitor.config_id (for legacy monitors).
  3. User doesn't have permission to access Saved Object or in case of any other server error against the fetch endpoint.

@dominiqueclarke @shahzad31 any thoughts on this?

@awahab07
Copy link
Contributor Author

awahab07 commented Oct 4, 2022

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

kibana-ci commented Oct 4, 2022 •
edited
Loading

💚 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
synthetics 1.0MB 1.0MB +201.0B

History

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

justinkambic
justinkambic approved these changes Oct 4, 2022
View reviewed changes
Copy link
Contributor

@justinkambic justinkambic 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 this using the deployment and the behavior was anticipated. Code looks good, too.

@awahab07 awahab07 merged commit 1eb059d into elastic:main Oct 4, 2022
@kibanamachine kibanamachine mentioned this pull request Oct 4, 2022
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 4, 2022
@awahab07
Fix: Consider all summary pings to determine if monitor is only fleet…
92be062 
… managed. (elastic#142004)

* Consider all ping to determine if monitor is only fleet managed.

* Close popover on outside click as the built-in functionality is buggy.

* Handle the case where only private locations are selected among a mix of locations.

(cherry picked from commit 1eb059d)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.5

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 Oct 4, 2022
@kibanamachine @awahab07
Fix: Consider all summary pings to determine if monitor is only fleet…
3b7c409 
… managed. (#142004) (#142659)

* Consider all ping to determine if monitor is only fleet managed.

* Close popover on outside click as the built-in functionality is buggy.

* Handle the case where only private locations are selected among a mix of locations.

(cherry picked from commit 1eb059d)

Co-authored-by: Abdul Wahab Zahid <[email protected]>
WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 11, 2022
@awahab07 @WafaaNasr
Fix: Consider all summary pings to determine if monitor is only fleet…
f17be6f 
… managed. (elastic#142004)

* Consider all ping to determine if monitor is only fleet managed.

* Close popover on outside click as the built-in functionality is buggy.

* Handle the case where only private locations are selected among a mix of locations.
WafaaNasr pushed a commit to WafaaNasr/kibana that referenced this pull request Oct 14, 2022
@awahab07 @WafaaNasr
Fix: Consider all summary pings to determine if monitor is only fleet…
04da951 
… managed. (elastic#142004)

* Consider all ping to determine if monitor is only fleet managed.

* Close popover on outside click as the built-in functionality is buggy.

* Handle the case where only private locations are selected among a mix of locations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justinkambic justinkambic justinkambic approved these changes

Assignees
No one assigned
Labels
bug Fixes for quality problems that affect the customer experience 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.5.0 v8.6.0
Projects
None yet
Milestone
No milestone
6 participants
@awahab07 @elasticmachine @andrewvc @kibana-ci @kibanamachine @justinkambic