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.

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

Improve error logs for task manager poller #197635

Merged
merged 3 commits into from
Oct 25, 2024

Conversation

mikecote
Copy link
Contributor

@mikecote mikecote commented Oct 24, 2024

I noticed some scenarios we see error logs from the task poller like Failed to poll for work: undefined making me think err.message is empty in some situations. I'm modifying the code to handle string situations if ever they occur by performing err.message || err and to also include a stack trace when strings are passed-in.

@mikecote mikecote added Feature:Task Manager Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v9.0.0 backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) v8.16.0 v8.17.0 labels Oct 24, 2024
@mikecote mikecote self-assigned this Oct 24, 2024
@mikecote mikecote requested a review from a team as a code owner October 24, 2024 13:08
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@mikecote mikecote added the release_note:skip Skip the PR/issue when compiling release notes label Oct 24, 2024
Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

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

LGTM

Happened to think, once the new Error(\${err}`)` gets thrown, the stack trace will lead back to this function, rather than whoever sent the error (which the other case will do). But, we'd never be able to get THAT stack trace anyway, since apparently it's not an Error. Hoping 🤞🏻 that the "new" message in there will help us pinpoint where it's coming from though. Think it's be best we can do for now ...

@mikecote
Copy link
Contributor Author

Happened to think, once the new Error(${err}) gets thrown, the stack trace will lead back to this function, rather than whoever sent the error (which the other case will do). But, we'd never be able to get THAT stack trace anyway, since apparently it's not an Error. Hoping 🤞🏻 that the "new" message in there will help us pinpoint where it's coming from though. Think it's be best we can do for now ...

@pmuellr yeah hopefully it can help a bit knowing the stack trace and where it was dropped so we can improve on that 🤞

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

cc @mikecote

@mikecote mikecote merged commit 81b63c6 into elastic:main Oct 25, 2024
40 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.16, 8.x

https://github.com/elastic/kibana/actions/runs/11517132514

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 25, 2024
I noticed some scenarios we see error logs from the task poller like
`Failed to poll for work: undefined` making me think `err.message` is
empty in some situations. I'm modifying the code to handle string
situations if ever they occur by performing `err.message || err` and to
also include a stack trace when strings are passed-in.

---------

Co-authored-by: Patrick Mueller <[email protected]>
(cherry picked from commit 81b63c6)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 25, 2024
I noticed some scenarios we see error logs from the task poller like
`Failed to poll for work: undefined` making me think `err.message` is
empty in some situations. I'm modifying the code to handle string
situations if ever they occur by performing `err.message || err` and to
also include a stack trace when strings are passed-in.

---------

Co-authored-by: Patrick Mueller <[email protected]>
(cherry picked from commit 81b63c6)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.16
8.x

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 25, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [Improve error logs for task manager poller
(#197635)](#197635)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Mike
Côté","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-25T11:12:44Z","message":"Improve
error logs for task manager poller (#197635)\n\nI noticed some scenarios
we see error logs from the task poller like\r\n`Failed to poll for work:
undefined` making me think `err.message` is\r\nempty in some situations.
I'm modifying the code to handle string\r\nsituations if ever they occur
by performing `err.message || err` and to\r\nalso include a stack trace
when strings are passed-in.\r\n\r\n---------\r\n\r\nCo-authored-by:
Patrick Mueller
<[email protected]>","sha":"81b63c60eb6d1fe623f2e177cd55d2f285f79590","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Feature:Task
Manager","Team:ResponseOps","v9.0.0","backport:prev-minor","v8.16.0","v8.17.0"],"title":"Improve
error logs for task manager
poller","number":197635,"url":"https://github.com/elastic/kibana/pull/197635","mergeCommit":{"message":"Improve
error logs for task manager poller (#197635)\n\nI noticed some scenarios
we see error logs from the task poller like\r\n`Failed to poll for work:
undefined` making me think `err.message` is\r\nempty in some situations.
I'm modifying the code to handle string\r\nsituations if ever they occur
by performing `err.message || err` and to\r\nalso include a stack trace
when strings are passed-in.\r\n\r\n---------\r\n\r\nCo-authored-by:
Patrick Mueller
<[email protected]>","sha":"81b63c60eb6d1fe623f2e177cd55d2f285f79590"}},"sourceBranch":"main","suggestedTargetBranches":["8.16","8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/197635","number":197635,"mergeCommit":{"message":"Improve
error logs for task manager poller (#197635)\n\nI noticed some scenarios
we see error logs from the task poller like\r\n`Failed to poll for work:
undefined` making me think `err.message` is\r\nempty in some situations.
I'm modifying the code to handle string\r\nsituations if ever they occur
by performing `err.message || err` and to\r\nalso include a stack trace
when strings are passed-in.\r\n\r\n---------\r\n\r\nCo-authored-by:
Patrick Mueller
<[email protected]>","sha":"81b63c60eb6d1fe623f2e177cd55d2f285f79590"}},{"branch":"8.16","label":"v8.16.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Mike Côté <[email protected]>
kibanamachine added a commit that referenced this pull request Oct 25, 2024
# Backport

This will backport the following commits from `main` to `8.16`:
- [Improve error logs for task manager poller
(#197635)](#197635)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Mike
Côté","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-25T11:12:44Z","message":"Improve
error logs for task manager poller (#197635)\n\nI noticed some scenarios
we see error logs from the task poller like\r\n`Failed to poll for work:
undefined` making me think `err.message` is\r\nempty in some situations.
I'm modifying the code to handle string\r\nsituations if ever they occur
by performing `err.message || err` and to\r\nalso include a stack trace
when strings are passed-in.\r\n\r\n---------\r\n\r\nCo-authored-by:
Patrick Mueller
<[email protected]>","sha":"81b63c60eb6d1fe623f2e177cd55d2f285f79590","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Feature:Task
Manager","Team:ResponseOps","v9.0.0","backport:prev-minor","v8.16.0","v8.17.0"],"title":"Improve
error logs for task manager
poller","number":197635,"url":"https://github.com/elastic/kibana/pull/197635","mergeCommit":{"message":"Improve
error logs for task manager poller (#197635)\n\nI noticed some scenarios
we see error logs from the task poller like\r\n`Failed to poll for work:
undefined` making me think `err.message` is\r\nempty in some situations.
I'm modifying the code to handle string\r\nsituations if ever they occur
by performing `err.message || err` and to\r\nalso include a stack trace
when strings are passed-in.\r\n\r\n---------\r\n\r\nCo-authored-by:
Patrick Mueller
<[email protected]>","sha":"81b63c60eb6d1fe623f2e177cd55d2f285f79590"}},"sourceBranch":"main","suggestedTargetBranches":["8.16","8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/197635","number":197635,"mergeCommit":{"message":"Improve
error logs for task manager poller (#197635)\n\nI noticed some scenarios
we see error logs from the task poller like\r\n`Failed to poll for work:
undefined` making me think `err.message` is\r\nempty in some situations.
I'm modifying the code to handle string\r\nsituations if ever they occur
by performing `err.message || err` and to\r\nalso include a stack trace
when strings are passed-in.\r\n\r\n---------\r\n\r\nCo-authored-by:
Patrick Mueller
<[email protected]>","sha":"81b63c60eb6d1fe623f2e177cd55d2f285f79590"}},{"branch":"8.16","label":"v8.16.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Mike Côté <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) Feature:Task Manager release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.16.0 v8.17.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants