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

[Synthetics] Adds retesting on failure #165626

Merged
merged 21 commits into from
Oct 3, 2023
Merged

[Synthetics] Adds retesting on failure #165626

merged 21 commits into from
Oct 3, 2023

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Sep 4, 2023 •
edited
Loading

Summary

Fixes #162399

Adds ability to retest on failure

image

In the Ping List
image

Smoke testing scope

  • Make sure alerting status/tls works as expected
  • Ping list shows both test and retest in list
  • Rest of the views in synthetics continue to work as expected

@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • /oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@shahzad31 shahzad31 added release_note:skip Skip the PR/issue when compiling release notes v8.11.0 labels Sep 5, 2023
@shahzad31 shahzad31 marked this pull request as ready for review September 5, 2023 11:45
@shahzad31 shahzad31 requested a review from a team as a code owner September 5, 2023 11:45
@botelastic botelastic bot added the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Sep 14, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

awahab07
awahab07 previously requested changes Sep 20, 2023
View reviewed changes
Copy link
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.

There's some behavior which is off:

  1. Status is always "Pending" for monitor.

  2. Retests results keep coming in despite monitor is consistently down. as opposed to the following AC:

    If the monitor was previously DOWN, a RETEST won't occur (this is to stop repeatedly retesting when we know the site is DOWN)

    localhost_5602_riv_app_synthetics

  3. Error count on both the Errors page and Overview page is incorrect.
    Screenshot 2023-09-20 at 17 47 19
    Screenshot 2023-09-20 at 17 33 24

  4. While Run once, the status is shown as COMPLETED instead of FAILED.
    Screenshot 2023-09-20 at 11 36 39

@awahab07
Copy link
Contributor

This is for the reminder that we'll need to adjust the responsiveness of the added UI elements. Preferably after the functionality is finalized.
Screenshot 2023-09-20 at 11 35 48

@shahzad31
Copy link
Contributor Author

There's some behavior which is off:

  1. Status is always "Pending" for monitor.

  2. Retests results keep coming in despite monitor is consistently down. as opposed to the following AC:

    If the monitor was previously DOWN, a RETEST won't occur (this is to stop repeatedly retesting when we know the site is DOWN)

  3. Error count on both the Errors page and Overview page is incorrect.
    images.githubusercontent.com/2748376/269346567-3309f805-419d-4510-92a6-9b22a53c6462.png">

  4. While Run once, the status is shown as COMPLETED instead of FAILED.

all the reported issues have been fixed ^^^

justinkambic
justinkambic reviewed Oct 2, 2023
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.

Had a few comments, but looking pretty good. LMK when ready and I will go through one more time.

.../public/components/shared/exploratory_view/configurations/synthetics/single_metric_config.ts Outdated Show resolved Hide resolved
...components/shared/exploratory_view/configurations/test_data/test_formula_metric_attribute.ts Outdated Show resolved Hide resolved
...components/shared/exploratory_view/configurations/test_data/test_formula_metric_attribute.ts Outdated Show resolved Hide resolved
...components/shared/exploratory_view/configurations/test_data/test_formula_metric_attribute.ts Outdated Show resolved Hide resolved
x-pack/plugins/uptime/server/legacy_uptime/routes/uptime_route_wrapper.ts Outdated Show resolved Hide resolved
...ck/plugins/synthetics/server/synthetics_service/project_monitor/normalizers/common_fields.ts Outdated Show resolved Hide resolved
...k/plugins/synthetics/public/apps/synthetics/utils/testing/__mocks__/synthetics_store.mock.ts Show resolved Hide resolved
...hetics/public/apps/synthetics/components/monitor_details/monitor_summary/test_runs_table.tsx
@@ -309,7 +323,7 @@ export const TestRunsTable = ({
pageIndex: page.index,
pageSize: page.size,
totalItemCount: total,
pageSizeOptions: [10, 20, 50], // TODO Confirm with Henry,
Copy link
Contributor

Choose a reason for hiding this comment

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

++, not possible

shahzad31 and others added 3 commits October 2, 2023 20:49
@shahzad31 @justinkambic
Update x-pack/plugins/exploratory_view/public/components/shared/explo…
351cdd8 
…ratory_view/configurations/synthetics/single_metric_config.ts

Co-authored-by: Justin Kambic <[email protected]>
@shahzad31 @justinkambic
Update x-pack/plugins/exploratory_view/public/components/shared/explo…
06df3cd 
…ratory_view/configurations/test_data/test_formula_metric_attribute.ts

Co-authored-by: Justin Kambic <[email protected]>
@shahzad31 @justinkambic
Update x-pack/plugins/exploratory_view/public/components/shared/explo…
0333de8 
…ratory_view/configurations/test_data/test_formula_metric_attribute.ts

Co-authored-by: Justin Kambic <[email protected]>
@shahzad31 @justinkambic
Update x-pack/plugins/exploratory_view/public/components/shared/explo…
e6537f1 
…ratory_view/configurations/test_data/test_formula_metric_attribute.ts

Co-authored-by: Justin Kambic <[email protected]>
justinkambic
justinkambic approved these changes Oct 2, 2023
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 WFG

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
synthetics 872 874 +2

Async chunks

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

id before after diff
exploratoryView 200.9KB 201.0KB +98.0B
synthetics 861.1KB 864.5KB +3.5KB
total +3.5KB

History

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

@shahzad31 shahzad31 dismissed awahab07’s stale review October 3, 2023 10:04

everything has been addressed

@shahzad31 shahzad31 merged commit db009f1 into elastic:main Oct 3, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Oct 3, 2023
@shahzad31 shahzad31 deleted the retries branch October 3, 2023 10:05
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

@awahab07 awahab07 awahab07 left review comments

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v8.11.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Synthetics] Implement the Kibana requirements for double testing
7 participants
@shahzad31 @apmmachine @elasticmachine @awahab07 @kibana-ci @justinkambic @kibanamachine