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 unknown product test flaky behavior #148616

Merged
merged 3 commits into from
Jan 11, 2023
Merged

Fix unknown product test flaky behavior #148616

merged 3 commits into from
Jan 11, 2023

Conversation

gsoldevila
Copy link
Contributor

Fixes #129754

The failing integration test is querying the /api/status endpoint before Kibana and ES are completely initialized, and thus, it is getting a unavailable: Waiting for Elasticsearch instead of the expected critical.

The proposed fix consists in awaiting a few milliseconds, in order to give the debounceTime operators enough time to propagate the right status.

@gsoldevila gsoldevila added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc release_note:skip Skip the PR/issue when compiling release notes test-failure-flaky backport:skip This commit does not require backporting v8.7.0 labels Jan 10, 2023
@gsoldevila gsoldevila requested a review from a team as a code owner January 10, 2023 09:45
jloleysens
jloleysens approved these changes Jan 10, 2023
View reviewed changes
Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

LGTM, but how many times do we run it before we no longer consider it flaky 😅 ?

@kibanamachine kibanamachine requested a review from a team as a code owner January 10, 2023 09:51
@gsoldevila
Copy link
Contributor Author

gsoldevila commented Jan 10, 2023 •
edited
Loading

LGTM, but how many times do we run it before we no longer consider it flaky 😅 ?

I managed to reproduce the test failure locally, commenting out the following line:

kibana/packages/core/status/core-status-server-internal/src/status_service.ts

Line 91 in 12466d8

debounceTime(80),

This line used to be a debounceTime(500) and it changed to debounceTime(80) with this PR.

A few days after it was merged, the first CI failure was observed.

I also confirmed locally that the introduced delay fixes the issue, when the debounceTime(80) is commented out.

So all in all, I'm pretty confident this will fix the flaky behavior.

@gsoldevila
Copy link
Contributor Author

gsoldevila commented Jan 10, 2023 •
edited
Loading

Now, that being said, perhaps it would be interesting to make the test more robust, having some sort of await on the status$ observables to be completely initialised somehow (instead of awaiting some random amount of time).

@gsoldevila
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@gsoldevila gsoldevila merged commit 9f39b78 into elastic:main Jan 11, 2023
jennypavlova pushed a commit to jennypavlova/kibana that referenced this pull request Jan 13, 2023
@gsoldevila @kibanamachine @jennypavlova
Fix unknown product test flaky behavior (elastic#148616)
948be95 
Fixes elastic#129754

The failing integration test is querying the `/api/status` endpoint
before Kibana and ES are completely initialized, and thus, it is getting
a _unavailable: Waiting for Elasticsearch_ instead of the expected
_critical_.

The proposed fix consists in awaiting a few milliseconds, in order to
give the `debounceTime` operators enough time to propagate the right
status.

Co-authored-by: kibanamachine <[email protected]>
@afharo afharo mentioned this pull request Aug 8, 2024
Closed
gsoldevila added a commit to gsoldevila/kibana that referenced this pull request Aug 8, 2024
@gsoldevila
Fix unknown product test flaky behavior (elastic#148616)
ece815c 
Fixes elastic#129754

The failing integration test is querying the `/api/status` endpoint
before Kibana and ES are completely initialized, and thus, it is getting
a _unavailable: Waiting for Elasticsearch_ instead of the expected
_critical_.

The proposed fix consists in awaiting a few milliseconds, in order to
give the `debounceTime` operators enough time to propagate the right
status.

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit 9f39b78)
@gsoldevila gsoldevila mentioned this pull request Aug 8, 2024
Merged
@gsoldevila
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
✅ 7.17

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

Questions ?

Please refer to the Backport tool documentation

gsoldevila added a commit that referenced this pull request Aug 8, 2024
@gsoldevila
[7.17] Fix unknown product test flaky behavior (#148616) (#190158)
68492d5 
# Backport

This will backport the following commits from `main` to `7.17`:
- [Fix unknown product test flaky behavior
(#148616)](#148616)

<!--- Backport version: 8.9.8 -->

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

<!--BACKPORT [{"author":{"name":"Gerard
Soldevila","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-01-11T14:20:37Z","message":"Fix
unknown product test flaky behavior (#148616)\n\nFixes
https://github.com/elastic/kibana/issues/129754\r\n\r\nThe failing
integration test is querying the `/api/status` endpoint\r\nbefore Kibana
and ES are completely initialized, and thus, it is getting\r\na
_unavailable: Waiting for Elasticsearch_ instead of the
expected\r\n_critical_.\r\n\r\nThe proposed fix consists in awaiting a
few milliseconds, in order to\r\ngive the `debounceTime` operators
enough time to propagate the right\r\nstatus.\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"9f39b785620fbc42e30aea774e875bb3048eb2ae","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Core","release_note:skip","test-failure-flaky","backport:skip","v8.7.0"],"number":148616,"url":"https://github.com/elastic/kibana/pull/148616","mergeCommit":{"message":"Fix
unknown product test flaky behavior (#148616)\n\nFixes
https://github.com/elastic/kibana/issues/129754\r\n\r\nThe failing
integration test is querying the `/api/status` endpoint\r\nbefore Kibana
and ES are completely initialized, and thus, it is getting\r\na
_unavailable: Waiting for Elasticsearch_ instead of the
expected\r\n_critical_.\r\n\r\nThe proposed fix consists in awaiting a
few milliseconds, in order to\r\ngive the `debounceTime` operators
enough time to propagate the right\r\nstatus.\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"9f39b785620fbc42e30aea774e875bb3048eb2ae"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/148616","number":148616,"mergeCommit":{"message":"Fix
unknown product test flaky behavior (#148616)\n\nFixes
https://github.com/elastic/kibana/issues/129754\r\n\r\nThe failing
integration test is querying the `/api/status` endpoint\r\nbefore Kibana
and ES are completely initialized, and thus, it is getting\r\na
_unavailable: Waiting for Elasticsearch_ instead of the
expected\r\n_critical_.\r\n\r\nThe proposed fix consists in awaiting a
few milliseconds, in order to\r\ngive the `debounceTime` operators
enough time to propagate the right\r\nstatus.\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"9f39b785620fbc42e30aea774e875bb3048eb2ae"}}]}]
BACKPORT-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jloleysens jloleysens jloleysens approved these changes

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:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc test-failure-flaky v7.17.24 v8.7.0
Projects
None yet
Milestone
No milestone
4 participants
@gsoldevila @kibana-ci @jloleysens @kibanamachine