[task manager] provide better diagnostics when task manager performance is degraded #109741
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
pmuellr
changed the title
mikecote
reviewed
Aug 24, 2021
mikecote
reviewed
Aug 24, 2021
pmuellr
added
Feature:Task Manager
release_note:enhancement
Team:ResponseOps
pmuellr
changed the title
|
Pinging @elastic/kibana-alerting-services (Team:Alerting Services) |
mikecote
approved these changes
Aug 24, 2021
| if (statusWithoutCapacity === HealthStatus.Warning) { | ||
| logLevel = LogLevel.Warn; | ||
| } else if (statusWithoutCapacity === HealthStatus.Error && !isEmpty(monitoredHealth.stats)) { | ||
| logLevel = LogLevel.Error; | ||
| } | ||
|
|
||
| const message = `Latest Monitored Stats: ${JSON.stringify(monitoredHealth)}`; | ||
| const detectedProblemMessage = `Task Manager detected a degradation in performance. This is usually temporary, and Kibana can recover automatically. If the problem persists, check the docs for troubleshooting information: https://www.elastic.co/guide/en/kibana/current/task-manager-health-monitoring.html .`; |
There was a problem hiding this comment.
We should probably generate the URL using the docLinks plugin. (example PR https://github.com/elastic/kibana/pull/92953/files).
There was a problem hiding this comment.
woops - I meant to leave a TODO for this (which would be easy to miss anyway), but there is no server-side docLinks plugin, yet - I did just add it as a TODO though :-)
To Do
open issue to change the new performance warning error to use server-side docLinks instead of the
currentreference, once we have server-side doclinks via this issue: [core] docLinks server-side service #95389 - the new issue will be blocked on that
Open for other ideas, felt like hard-coding current was going to be the best story for now ...
There was a problem hiding this comment.
Ahh that's true, it's only public.
What about something like the following?
import { kibanaPackageJson } from '@kbn/utils';
https://www.elastic.co/guide/en/kibana/${kibanaPackageJson.branch}/task-manager-health-monitoring.html
There was a problem hiding this comment.
I opened #109937 to track this (and deleted the TODO in the top comment).
There was a problem hiding this comment.
Ya, it does look like the existing docLink stuff makes use of the branch property, so seems doable. Up to you - we'll need to thread it through all the calls ... :-) I think long-term we still want docLinks for this, to handle cases when the URLs change (I assume it deals with that).
There was a problem hiding this comment.
Do we have a url shortening service? I vaguely remember using one in the past
There was a problem hiding this comment.
A URL shortener to make the doc link shorter? I'm not sure I want that - I'd prefer the user see that we're pointing them to doc, vs pointing them to the great unknown :-).
There was a problem hiding this comment.
Turns out using @kbn/utils to get the patch version is absurdly easy: in commit e8c9b08
Thanks for the tip, Mike! I want to use more packages like this :-)
ymao1
approved these changes
Aug 24, 2021
| logger.warn( | ||
| `Detected potential performance issue with Task Manager. Set 'xpack.task_manager.monitored_stats_health_verbose_log.enabled: true' in your Kibana.yml to enable debug logging` | ||
| ); | ||
| logger.debug(detectedProblemMessage); |
There was a problem hiding this comment.
I understand why we're changing it back to debug for this PR, but we're effectively undoing this issue that was done for O11y of Alerting, where the intent was to try to have task manager self-introspect when it might be having problems and let the user know so they could turn on debug logging easily. Are we officially giving up on that part of O11y of Alerting because we've determined it's too noisy and too many false positives or will there be another follow up issue to try to achieve it in a less noisy way?
There was a problem hiding this comment.
Yeah, that's it in a nutshell - it's too noisy and too many false positives. I'll open a new issue to track this, note some of the issues we've already seen. Thanks!
There was a problem hiding this comment.
opened #109941 to track this
|
@elasticmachine merge upstream |
This was referenced Aug 26, 2021
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
pmuellr
added a commit
to pmuellr/kibana
that referenced
this pull request
Sep 1, 2021
…ce is degraded (elastic#109741) resolves elastic#109095 resolves elastic#106854 Changes the way task manager and alerting perform their health / status checks: - no longer sets an `unavailable` status; now uses `degraded` instead - change task manager "hot stats freshness" calculation to allow for staler data before signalling a problem - Changed the "Detected potential performance issue" message to sound less scary, include a doc link to task manager health monitoring, and log a debug instead of warning level - add additional debug logging when task manager sets a status that's not `available`, indicating why it's setting that status (in the code, it's when task manager uses HealthStatus.Warning or Error)
pmuellr
added a commit
to pmuellr/kibana
that referenced
this pull request
Sep 1, 2021
…ce is degraded (elastic#109741) resolves elastic#109095 resolves elastic#106854 Changes the way task manager and alerting perform their health / status checks: - no longer sets an `unavailable` status; now uses `degraded` instead - change task manager "hot stats freshness" calculation to allow for staler data before signalling a problem - Changed the "Detected potential performance issue" message to sound less scary, include a doc link to task manager health monitoring, and log a debug instead of warning level - add additional debug logging when task manager sets a status that's not `available`, indicating why it's setting that status (in the code, it's when task manager uses HealthStatus.Warning or Error)
pmuellr
added a commit
to pmuellr/kibana
that referenced
this pull request
Sep 1, 2021
…ce is degraded (elastic#109741) resolves elastic#109095 resolves elastic#106854 Changes the way task manager and alerting perform their health / status checks: - no longer sets an `unavailable` status; now uses `degraded` instead - change task manager "hot stats freshness" calculation to allow for staler data before signalling a problem - Changed the "Detected potential performance issue" message to sound less scary, include a doc link to task manager health monitoring, and log a debug instead of warning level - add additional debug logging when task manager sets a status that's not `available`, indicating why it's setting that status (in the code, it's when task manager uses HealthStatus.Warning or Error) # Conflicts: # x-pack/plugins/task_manager/server/monitoring/capacity_estimation.ts # x-pack/plugins/task_manager/server/monitoring/task_run_statistics.test.ts # x-pack/plugins/task_manager/server/routes/health.test.ts
pmuellr
added a commit
that referenced
this pull request
Sep 1, 2021
…ce is degraded (#109741) (#110870) resolves #109095 resolves #106854 Changes the way task manager and alerting perform their health / status checks: - no longer sets an `unavailable` status; now uses `degraded` instead - change task manager "hot stats freshness" calculation to allow for staler data before signalling a problem - Changed the "Detected potential performance issue" message to sound less scary, include a doc link to task manager health monitoring, and log a debug instead of warning level - add additional debug logging when task manager sets a status that's not `available`, indicating why it's setting that status (in the code, it's when task manager uses HealthStatus.Warning or Error)
pmuellr
added a commit
that referenced
this pull request
Sep 1, 2021
…ce is degraded (#109741) (#110869) resolves #109095 resolves #106854 Changes the way task manager and alerting perform their health / status checks: - no longer sets an `unavailable` status; now uses `degraded` instead - change task manager "hot stats freshness" calculation to allow for staler data before signalling a problem - Changed the "Detected potential performance issue" message to sound less scary, include a doc link to task manager health monitoring, and log a debug instead of warning level - add additional debug logging when task manager sets a status that's not `available`, indicating why it's setting that status (in the code, it's when task manager uses HealthStatus.Warning or Error)
pmuellr
added a commit
that referenced
this pull request
Sep 1, 2021
…rformance is degraded (#109741) (#110875) * [task manager] provide better diagnostics when task manager performance is degraded (#109741) resolves #109095 resolves #106854 Changes the way task manager and alerting perform their health / status checks: - no longer sets an `unavailable` status; now uses `degraded` instead - change task manager "hot stats freshness" calculation to allow for staler data before signalling a problem - Changed the "Detected potential performance issue" message to sound less scary, include a doc link to task manager health monitoring, and log a debug instead of warning level - add additional debug logging when task manager sets a status that's not `available`, indicating why it's setting that status (in the code, it's when task manager uses HealthStatus.Warning or Error) # Conflicts: # x-pack/plugins/task_manager/server/monitoring/capacity_estimation.ts # x-pack/plugins/task_manager/server/monitoring/task_run_statistics.test.ts # x-pack/plugins/task_manager/server/routes/health.test.ts * fix backport to remove post-7.14 stuff
💔 Build Failed
Failed CI Steps
Test FailuresKibana Pipeline / general / task-queue-process-15 / X-Pack Endpoint API Integration Tests.x-pack/test/security_solution_endpoint_api_int/apis/metadata·ts.Endpoint plugin test metadata api POST /api/endpoint/metadata when index is not empty metadata api should return one entry for each host with default pagingStandard OutStack TraceKibana Pipeline / general / Plugin Functional Tests.test/plugin_functional/test_suites/data_plugin/session·ts.data plugin Session management Dashboard "after each" hook for "on load there is a single session"Standard OutStack TraceKibana Pipeline / general / Chrome UI Functional Tests.test/functional/apps/visualize/_heatmap_chart·ts.visualize app visualize ciGroup10 heatmap chart should show 6 color ranges if changed on optionsStandard OutStack Traceand 24 more failures, only showing the first 3. Metrics [docs]
History
To update your PR or re-run it, just comment with: |
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
resolves #109095
resolves #106854
resolves #110332
changes to task manager:
unavailableif it found a Health status of error, which caused Kibana itself to claim it wasunavailable. It no longer does this, and will instead set it's status todegraded, which is also reflected as the Kibana status. There have been cases of users seeing theunavailablestatus and misunderstanding that Kibana crashed, which it did not.changes to alerting:
unavailableplugin status, changing all references of that todegradedChecklist
Delete any items that are not applicable to this PR.
[ ] Documentation was added for features that require explanation or tutorialsAny 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 listThis renders correctly on smaller devices using a responsive layout. (You can test this in your browser)This was checked for cross-browser compatibilityRelease note
Improves Task Manager performance monitoring to log more diagnostic data in debug logs, provide a more accurate logged message when performance degradation is noticed, and never set the plugin status of "unavailable", using "degraded" instead.