[ftr/remote] stop WebDriver logs polling when its session no longer exists #171672
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dmlemeshko
force-pushed
the
ftr/improve-pollForLogEntry
branch
from
2b30dd1
to
d156067
Compare
dmlemeshko
added
FTR
v8.12.0
v8.11.2
release_note:skip
dmlemeshko
changed the title
dmlemeshko
changed the title
|
Adding Operations team to be aware of the change. |
pheyos
reviewed
Nov 22, 2023
…ibana into ftr/improve-pollForLogEntry
|
@pheyos, I think it is ready for another look |
…ibana into ftr/improve-pollForLogEntry
dmlemeshko
commented
Jan 12, 2024
| }); | ||
| // pause 10 seconds then resubscribe | ||
| Rx.of(1).pipe( | ||
| delay(10 * 1000), |
There was a problem hiding this comment.
Short delay (e.g. 10 ms) will help to reproduce logs duplication in main. With this change you won't see multiple messages like
warn browser[SEVERE] ERROR FETCHING BROWSR LOGS: This driver instance does not have a valid session ID (did you call WebDriver.quit()?) and may no longer be used.
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: |
pheyos
approved these changes
Jan 12, 2024
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Jan 12, 2024
…xists (elastic#171672) ## Summary Part of elastic#171743 Since we no longer collecting code coverage for functional tests, I removed flushing on stop signal. We also noticed a failure on test completion and it is probably due to resubscribing for logs polling when WebDriver session is already destroyed. See elastic#171743 for actual logs example My guess, our current approach to keep fetching WebDriver/Browser logs based on FTR lifecycle is not very robust: `takeUntil(lifecycle.cleanup.after$)` keeps emitting until the `cleanup` stage is finished, and internally we keep resubscribing on WebDriver failures due to `NoSuchSessionError` and keep trying to fetch logs again & again. Since we don't re-create WebDriver session, it simply doesn't make sense. Resubscribing seems useless. I'm replacing `takeUntil(lifecycle.cleanup.after$)` with `takeWhile((loggingEntry: logging.Entry) =>!loggingEntry.message.startsWith(FINAL_LOG_ENTRY_PREFIX))` so that we stop as soon as we get NoSuchSessionError even if it is before cleanup.after stage in FTR lifecycle (e.g. browser crashed during the test run) For **reviewers**: please give it a try locally and check that you are able to see browser logs, but no errors/delay on FTR teardown. How to test: - start servers & test runner - let the browser window to be opened by WebDriver and then close it manually - check console logs: `browser` logs should be stopped immediately, you don't see any browser/webdriver logs other than general FTR error (cherry picked from commit d34915b)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
semd
pushed a commit
to semd/kibana
that referenced
this pull request
Jan 12, 2024
…xists (elastic#171672) ## Summary Part of elastic#171743 Since we no longer collecting code coverage for functional tests, I removed flushing on stop signal. We also noticed a failure on test completion and it is probably due to resubscribing for logs polling when WebDriver session is already destroyed. See elastic#171743 for actual logs example My guess, our current approach to keep fetching WebDriver/Browser logs based on FTR lifecycle is not very robust: `takeUntil(lifecycle.cleanup.after$)` keeps emitting until the `cleanup` stage is finished, and internally we keep resubscribing on WebDriver failures due to `NoSuchSessionError` and keep trying to fetch logs again & again. Since we don't re-create WebDriver session, it simply doesn't make sense. Resubscribing seems useless. I'm replacing `takeUntil(lifecycle.cleanup.after$)` with `takeWhile((loggingEntry: logging.Entry) =>!loggingEntry.message.startsWith(FINAL_LOG_ENTRY_PREFIX))` so that we stop as soon as we get NoSuchSessionError even if it is before cleanup.after stage in FTR lifecycle (e.g. browser crashed during the test run) For **reviewers**: please give it a try locally and check that you are able to see browser logs, but no errors/delay on FTR teardown. How to test: - start servers & test runner - let the browser window to be opened by WebDriver and then close it manually - check console logs: `browser` logs should be stopped immediately, you don't see any browser/webdriver logs other than general FTR error
kibanamachine
added a commit
that referenced
this pull request
Jan 12, 2024
…onger exists (#171672) (#174770) # Backport This will backport the following commits from `main` to `8.12`: - [[ftr/remote] stop WebDriver logs polling when its session no longer exists (#171672)](#171672) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Dzmitry Lemechko","email":"dzmitry.lemechko@elastic.co"},"sourceCommit":{"committedDate":"2024-01-12T15:12:35Z","message":"[ftr/remote] stop WebDriver logs polling when its session no longer exists (#171672)\n\n## Summary\r\n\r\nPart of #171743\r\n\r\nSince we no longer collecting code coverage for functional tests, I\r\nremoved flushing on stop signal.\r\n\r\nWe also noticed a failure on test completion and it is probably due to\r\nresubscribing for logs polling when WebDriver session is already\r\ndestroyed. See #171743 for actual logs example\r\n\r\nMy guess, our current approach to keep fetching WebDriver/Browser logs\r\nbased on FTR lifecycle is not very robust:\r\n`takeUntil(lifecycle.cleanup.after$)` keeps emitting until the `cleanup`\r\nstage is finished, and internally we keep resubscribing on WebDriver\r\nfailures due to `NoSuchSessionError` and keep trying to fetch logs again\r\n& again. Since we don't re-create WebDriver session, it simply doesn't\r\nmake sense. Resubscribing seems useless.\r\n\r\nI'm replacing `takeUntil(lifecycle.cleanup.after$)` with\r\n`takeWhile((loggingEntry: logging.Entry)\r\n=>!loggingEntry.message.startsWith(FINAL_LOG_ENTRY_PREFIX))` so that we\r\nstop as soon as we get NoSuchSessionError even if it is before\r\ncleanup.after stage in FTR lifecycle (e.g. browser crashed during the\r\ntest run)\r\n\r\nFor **reviewers**: please give it a try locally and check that you are\r\nable to see browser logs, but no errors/delay on FTR teardown.\r\n\r\nHow to test:\r\n- start servers & test runner\r\n- let the browser window to be opened by WebDriver and then close it\r\nmanually\r\n- check console logs: `browser` logs should be stopped immediately, you\r\ndon't see any browser/webdriver logs other than general FTR error","sha":"d34915bec78de95af02e775924d83ef95e6ea5bb","branchLabelMapping":{"^v8.13.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","FTR","v8.12.1","v8.13.0"],"title":"[ftr/remote] stop WebDriver logs polling when its session no longer exists","number":171672,"url":"#171672 stop WebDriver logs polling when its session no longer exists (#171672)\n\n## Summary\r\n\r\nPart of #171743\r\n\r\nSince we no longer collecting code coverage for functional tests, I\r\nremoved flushing on stop signal.\r\n\r\nWe also noticed a failure on test completion and it is probably due to\r\nresubscribing for logs polling when WebDriver session is already\r\ndestroyed. See #171743 for actual logs example\r\n\r\nMy guess, our current approach to keep fetching WebDriver/Browser logs\r\nbased on FTR lifecycle is not very robust:\r\n`takeUntil(lifecycle.cleanup.after$)` keeps emitting until the `cleanup`\r\nstage is finished, and internally we keep resubscribing on WebDriver\r\nfailures due to `NoSuchSessionError` and keep trying to fetch logs again\r\n& again. Since we don't re-create WebDriver session, it simply doesn't\r\nmake sense. Resubscribing seems useless.\r\n\r\nI'm replacing `takeUntil(lifecycle.cleanup.after$)` with\r\n`takeWhile((loggingEntry: logging.Entry)\r\n=>!loggingEntry.message.startsWith(FINAL_LOG_ENTRY_PREFIX))` so that we\r\nstop as soon as we get NoSuchSessionError even if it is before\r\ncleanup.after stage in FTR lifecycle (e.g. browser crashed during the\r\ntest run)\r\n\r\nFor **reviewers**: please give it a try locally and check that you are\r\nable to see browser logs, but no errors/delay on FTR teardown.\r\n\r\nHow to test:\r\n- start servers & test runner\r\n- let the browser window to be opened by WebDriver and then close it\r\nmanually\r\n- check console logs: `browser` logs should be stopped immediately, you\r\ndon't see any browser/webdriver logs other than general FTR error","sha":"d34915bec78de95af02e775924d83ef95e6ea5bb"}},"sourceBranch":"main","suggestedTargetBranches":["8.12"],"targetPullRequestStates":[{"branch":"8.12","label":"v8.12.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.13.0","branchLabelMappingKey":"^v8.13.0$","isSourceBranch":true,"state":"MERGED","url":"#171672 stop WebDriver logs polling when its session no longer exists (#171672)\n\n## Summary\r\n\r\nPart of #171743\r\n\r\nSince we no longer collecting code coverage for functional tests, I\r\nremoved flushing on stop signal.\r\n\r\nWe also noticed a failure on test completion and it is probably due to\r\nresubscribing for logs polling when WebDriver session is already\r\ndestroyed. See #171743 for actual logs example\r\n\r\nMy guess, our current approach to keep fetching WebDriver/Browser logs\r\nbased on FTR lifecycle is not very robust:\r\n`takeUntil(lifecycle.cleanup.after$)` keeps emitting until the `cleanup`\r\nstage is finished, and internally we keep resubscribing on WebDriver\r\nfailures due to `NoSuchSessionError` and keep trying to fetch logs again\r\n& again. Since we don't re-create WebDriver session, it simply doesn't\r\nmake sense. Resubscribing seems useless.\r\n\r\nI'm replacing `takeUntil(lifecycle.cleanup.after$)` with\r\n`takeWhile((loggingEntry: logging.Entry)\r\n=>!loggingEntry.message.startsWith(FINAL_LOG_ENTRY_PREFIX))` so that we\r\nstop as soon as we get NoSuchSessionError even if it is before\r\ncleanup.after stage in FTR lifecycle (e.g. browser crashed during the\r\ntest run)\r\n\r\nFor **reviewers**: please give it a try locally and check that you are\r\nable to see browser logs, but no errors/delay on FTR teardown.\r\n\r\nHow to test:\r\n- start servers & test runner\r\n- let the browser window to be opened by WebDriver and then close it\r\nmanually\r\n- check console logs: `browser` logs should be stopped immediately, you\r\ndon't see any browser/webdriver logs other than general FTR error","sha":"d34915bec78de95af02e775924d83ef95e6ea5bb"}}]}] BACKPORT--> Co-authored-by: Dzmitry Lemechko <dzmitry.lemechko@elastic.co>
CoenWarmer
pushed a commit
to CoenWarmer/kibana
that referenced
this pull request
Feb 15, 2024
…xists (elastic#171672) ## Summary Part of elastic#171743 Since we no longer collecting code coverage for functional tests, I removed flushing on stop signal. We also noticed a failure on test completion and it is probably due to resubscribing for logs polling when WebDriver session is already destroyed. See elastic#171743 for actual logs example My guess, our current approach to keep fetching WebDriver/Browser logs based on FTR lifecycle is not very robust: `takeUntil(lifecycle.cleanup.after$)` keeps emitting until the `cleanup` stage is finished, and internally we keep resubscribing on WebDriver failures due to `NoSuchSessionError` and keep trying to fetch logs again & again. Since we don't re-create WebDriver session, it simply doesn't make sense. Resubscribing seems useless. I'm replacing `takeUntil(lifecycle.cleanup.after$)` with `takeWhile((loggingEntry: logging.Entry) =>!loggingEntry.message.startsWith(FINAL_LOG_ENTRY_PREFIX))` so that we stop as soon as we get NoSuchSessionError even if it is before cleanup.after stage in FTR lifecycle (e.g. browser crashed during the test run) For **reviewers**: please give it a try locally and check that you are able to see browser logs, but no errors/delay on FTR teardown. How to test: - start servers & test runner - let the browser window to be opened by WebDriver and then close it manually - check console logs: `browser` logs should be stopped immediately, you don't see any browser/webdriver logs other than general FTR error
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Part of #171743
Since we no longer collecting code coverage for functional tests, I removed flushing on stop signal.
We also noticed a failure on test completion and it is probably due to resubscribing for logs polling when WebDriver session is already destroyed. See #171743 for actual logs example
My guess, our current approach to keep fetching WebDriver/Browser logs based on FTR lifecycle is not very robust:
takeUntil(lifecycle.cleanup.after$)keeps emitting until thecleanupstage is finished, and internally we keep resubscribing on WebDriver failures due toNoSuchSessionErrorand keep trying to fetch logs again & again. Since we don't re-create WebDriver session, it simply doesn't make sense. Resubscribing seems useless.I'm replacing
takeUntil(lifecycle.cleanup.after$)withtakeWhile((loggingEntry: logging.Entry) =>!loggingEntry.message.startsWith(FINAL_LOG_ENTRY_PREFIX))so that we stop as soon as we get NoSuchSessionError even if it is before cleanup.after stage in FTR lifecycle (e.g. browser crashed during the test run)For reviewers: please give it a try locally and check that you are able to see browser logs, but no errors/delay on FTR teardown.
How to test:
browserlogs should be stopped immediately, you don't see any browser/webdriver logs other than general FTR error