adds apm instrumentation to task manager's task runner #55356
Merged
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
added
Feature:Alerting
release_note:skip
pmuellr
commented
Feb 10, 2020
| const trans = apm.startTransaction( | ||
| `taskManager run ${this.instance.taskType}`, | ||
| 'taskManager', | ||
| this.instance.taskType, |
There was a problem hiding this comment.
The name and type (first two parms ^^^) of apm.startTransaction() seem useful, in terms of how the UI ends up making use of them. The subtype and action (last two parms) I'm less sure of, as I'm not sure they come up directly in the UI. Since these end up being other plugins tasks running, seems unlikely we can consistently get more specific per-task info. And alerting and actions already have alertType and actionType specific task types, which makes for nice grouping.
Thinking of just removing the last two.
pmuellr
commented
Feb 10, 2020
| return this.processResult(validatedResult); | ||
| } catch (err) { | ||
| this.logger.error(`Task ${this} failed: ${err}`); | ||
| // in error scenario, we can not get the RunResult | ||
| // re-use modifiedContext's state, which is correct as of beforeRun | ||
| if (trans) trans.end(); |
There was a problem hiding this comment.
probably want to use trans.end('success') and one for failure as well
pmuellr
commented
Feb 10, 2020
| @@ -252,6 +253,7 @@ export class TaskStore { | |||
| ) | |||
| ); | |||
|
|
|||
| const trans = apm.startTransaction(`taskManager markAvailableTasksAsClaimed`, 'taskManager'); | |||
There was a problem hiding this comment.
I think for this call, we should look into using transaction.setLabel() to add the queue size. Hopefully it will show up somewhere in the UI, but ideally I'd like some sort of specialzed ui for this anyway, so while it may not be immediately useful, at least it's getting written somewhere.
There was a problem hiding this comment.
label values must be strings, so not as useful as I thought
There was a problem hiding this comment.
also, we'd need to actually get the queue size, but I wouldn't want the cost of getting that to be included here, so we'll have to figure out some other story for the queue size ...
Adds some apm transaction boundaries for parts of task manager, so that they will show up in APM as new types of transactions. Should provide some visibility into the ES calls made by task manager for alerting and actions, especially under stress testing scenarios.
pmuellr
force-pushed
the
taskManager/apm-ize
branch
from
bd09593 to
76451dd
Compare
pmuellr
added
v7.7.0
v8.0.0
Team:ResponseOps
|
Pinging @elastic/kibana-alerting-services (Team:Alerting Services) |
sorenlouv
approved these changes
Feb 11, 2020
|
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/advanced_settings/feature_controls/advanced_settings_spaces·ts.Advanced Settings spaces feature controls space with Advanced Settings disabled redirects to management homeStandard OutStack TraceHistory
To update your PR or re-run it, just comment with: |
pmuellr
added a commit
to pmuellr/kibana
that referenced
this pull request
Feb 11, 2020
Adds some apm transaction boundaries for parts of task manager, so that they will show up in APM as new types of transactions. Should provide some visibility into the ES calls made by task manager for alerting and actions, especially under stress testing scenarios.
pmuellr
added a commit
that referenced
this pull request
Feb 12, 2020
Adds some apm transaction boundaries for parts of task manager, so that they will show up in APM as new types of transactions. Should provide some visibility into the ES calls made by task manager for alerting and actions, especially under stress testing scenarios.
gmmorris
added a commit
to gmmorris/kibana
that referenced
this pull request
Feb 12, 2020
* master: [Canvas] Move sample data and feature registration to canvas np plugin (elastic#56564) instrument task manager with apm transactions (elastic#55356) displays Alert Instance state on Alert Details page (elastic#56842) Adding the Accessibility Statement to docs (elastic#57153) [Uptime] Remove redundant adapter function (elastic#56980) [SIEM][Detection Engine] Backend end-to-end tests [Uptime] Added tests for pages (elastic#56736) Updating to kind-of@6.0.3 (elastic#57367)
mikecote
added
release_note:enhancement
and removed
release_note:skip
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
This adds APM support to task manager by instrumenting task manager's task runner with apm transactions.
To use in Kibana with
yarn start, create a fileconfig/apm.dev.jswith the following contents:You'll then need an APM server running. For running Kibana off master, I was able to run a production cloud 7.5.1 deployment, with apm enabled (the default), and use the token and url provided by the deployment. Start your local Kibana via
yarn start, and you'll start seeing numbers in your APM instance.Here's an example of what a pretty simple instrumentation (the first commit) can provide: