[Synthetics] Apply faster backoff for non-`test-now` screenshot results #152874

justinkambic · 2023-03-07T23:44:45Z

Summary

It was noted in #152754 that we're getting some negative side effects from the exponential backoff we added to the getter for journey screenshots. When a screenshot is legitimately not available for an older journey (as opposed to not yet fully indexed for a fresh journey), the backoff will cause the screenshot never to render.

Testing

Follow a flow similar to what's illustrated in the GIF below.

Create a monitor with some screenshots that show up.
Delete the screenshot data stream. A good way is to use the Index Management > Data Streams view in Kibana.
Refresh your monitor's detail page when new photos are coming in. It should give up on finding the images very fast, and as they aren't there, you should see the missing image placeholder.
Start creating a new monitor. Make it have multiple steps.
Ensure that you aren't seeing any "infinite" loading states across the screenshot UIs.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

dominiqueclarke

LGTM. Screenshots are now working as expected both in test now/run once and on monitor details page, test run details page, and step details page.

elasticmachine · 2023-03-08T20:49:49Z

Pinging @elastic/uptime (Team:uptime)

shahzad31 · 2023-03-21T14:09:35Z

...k/plugins/synthetics/public/apps/synthetics/components/common/screenshot/empty_thumbnail.tsx

+        <EuiSkeletonRectangle
+          data-test-subj="stepScreenshotPlaceholderLoading"
+          isLoading={isLoading}
+          height={height}
+          width={width}
+        >
+          {/* `children` is required by this component, even though we'll replace it. */}
+          <span />
+        </EuiSkeletonRectangle>


you can't do this for now. since this needs to backported. in 8.7 this doesn't exists.

Not intended for 8.7 anymore 😃

paulb-elastic · 2023-03-22T10:11:16Z

Moving over to 8.8

…hether the backoff is in progress.

… aren\'t using the result of that hook anymore.

dominiqueclarke

LGTM!

kibana-ci · 2023-04-18T16:29:39Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 56b212d

Failed CI Steps

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.2MB	1.2MB	+375.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`securitySolution`	432	435	+3

Total ESLint disabled count

id	before	after	diff
`securitySolution`	512	515	+3

History

💔 Build #119592 failed 6f55ba3
💛 Build #116564 was flaky d1fa765
💔 Build #116197 failed b725b92
💛 Build #116159 was flaky e53a554
💔 Build #115209 failed 59f2502177ad118aee273c88933093095c87092d

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

cc @justinkambic

justinkambic added bug Fixes for quality problems that affect the customer experience Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability release_note:skip Skip the PR/issue when compiling release notes v8.7.0 labels Mar 7, 2023

justinkambic self-assigned this Mar 7, 2023

dominiqueclarke self-requested a review March 8, 2023 14:45

dominiqueclarke approved these changes Mar 8, 2023

View reviewed changes

justinkambic force-pushed the 152754/update-exponential-backoff-usage branch from 645b0d2 to c1fec9a Compare March 8, 2023 20:49

justinkambic marked this pull request as ready for review March 8, 2023 20:49

justinkambic requested a review from a team as a code owner March 8, 2023 20:49

justinkambic mentioned this pull request Mar 8, 2023

[Synthetics] Adjust screenshot backoff logic for older runs #152710

Merged

justinkambic force-pushed the 152754/update-exponential-backoff-usage branch from c1fec9a to 0805ad0 Compare March 9, 2023 13:40

dominiqueclarke self-requested a review March 9, 2023 14:45

justinkambic force-pushed the 152754/update-exponential-backoff-usage branch from 0805ad0 to 3d2bb9e Compare March 9, 2023 17:36

justinkambic force-pushed the 152754/update-exponential-backoff-usage branch from 0bcae4d to 8b29aec Compare March 16, 2023 20:49

shahzad31 reviewed Mar 21, 2023

View reviewed changes

paulb-elastic added v8.8.0 and removed v8.7.0 labels Mar 22, 2023

justinkambic force-pushed the 152754/update-exponential-backoff-usage branch 2 times, most recently from 814a81d to f75443c Compare March 23, 2023 20:11

justinkambic added 8 commits March 28, 2023 10:20

Apply faster backoff for non-test-now screenshot results.

382cdc0

Add a separate loading state to the step retrieval hook to indicate w…

63cb360

…hether the backoff is in progress.

Fix loading animation and use EUI version instead of custom animation.

d10f52e

Update screenshot usage.

a81199a

Fix broken unit tests.

3d2fe36

Mend unit tests.

e141e89

Only backoff when using run-once.

e0488e4

Remove unused useFetcher call in favor of useEffect.

a31f3de

justinkambic added 4 commits March 28, 2023 10:20

Remove unneeded loading flag.

66988b2

Unwrap return object.

1b8ff55

Fix mocks and remove test that depended on useFetcher status, as we…

e109abd

… aren\'t using the result of that hook anymore.

Fix broken unit tests.

e53a554

justinkambic force-pushed the 152754/update-exponential-backoff-usage branch from 59f2502 to e53a554 Compare March 28, 2023 16:05

Simplify effect in hook.

42b891a

justinkambic force-pushed the 152754/update-exponential-backoff-usage branch from 29da0a1 to 42b891a Compare March 28, 2023 18:08

justinkambic added 2 commits March 28, 2023 14:17

Revert a change to the loading state for screenshot previews.

b725b92

Merge branch 'main' into 152754/update-exponential-backoff-usage

d1fa765

dominiqueclarke approved these changes Apr 5, 2023

View reviewed changes

justinkambic added 2 commits April 12, 2023 16:47

Merge branch 'main' into 152754/update-exponential-backoff-usage

6f55ba3

Merge branch 'main' into 152754/update-exponential-backoff-usage

56b212d

justinkambic merged commit 5af2fd4 into elastic:main Apr 18, 2023

kibanamachine added the backport:skip This commit does not require backporting label Apr 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Synthetics] Apply faster backoff for non-`test-now` screenshot results #152874

[Synthetics] Apply faster backoff for non-`test-now` screenshot results #152874

justinkambic commented Mar 7, 2023 •

edited

Loading

dominiqueclarke left a comment

elasticmachine commented Mar 8, 2023

shahzad31 Mar 21, 2023

justinkambic Mar 22, 2023

paulb-elastic commented Mar 22, 2023

dominiqueclarke left a comment

kibana-ci commented Apr 18, 2023

ESLint disabled line counts

Total ESLint disabled count

[Synthetics] Apply faster backoff for non-test-now screenshot results #152874

[Synthetics] Apply faster backoff for non-test-now screenshot results #152874

Conversation

justinkambic commented Mar 7, 2023 • edited Loading

Summary

Testing

Checklist

Risk Matrix

For maintainers

dominiqueclarke left a comment

Choose a reason for hiding this comment

elasticmachine commented Mar 8, 2023

shahzad31 Mar 21, 2023

Choose a reason for hiding this comment

justinkambic Mar 22, 2023

Choose a reason for hiding this comment

paulb-elastic commented Mar 22, 2023

dominiqueclarke left a comment

Choose a reason for hiding this comment

kibana-ci commented Apr 18, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

ESLint disabled line counts

Total ESLint disabled count

History

[Synthetics] Apply faster backoff for non-`test-now` screenshot results #152874

[Synthetics] Apply faster backoff for non-`test-now` screenshot results #152874

justinkambic commented Mar 7, 2023 •

edited

Loading