-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[APM] Query transaction metrics for alerts #108167
[APM] Query transaction metrics for alerts #108167
Conversation
Pinging @elastic/apm-ui (Team:apm) |
This makes sense but could be confusing to users. Documenting the behaviour might help although I'm not a fan of documenting what feel like implementation details. |
const searchAggregatedTransactions = | ||
config['xpack.apm.searchAggregatedTransactions'] !== | ||
SearchAggregatedTransactionSetting.never; | ||
|
||
const index = searchAggregatedTransactions | ||
? indices['apm_oss.metricsIndices'] | ||
: indices['apm_oss.transactionIndices']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to extract this to a function so it can be re-used between the alerts?
And add a comment to make the behaviour around auto
clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems overkill for two usages? I'll add a comment anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
apart from the comments lgtm
@elasticmachine merge upstream |
@elasticmachine merge upstream |
…ana into alert-transaction-metrics
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / general / Firefox XPack UI Functional Tests.x-pack/test/functional/apps/status_page/status_page·ts.Status page Status Page allows user to navigate without authenticationStandard Out
Stack Trace
Metrics [docs]
History
To update your PR or re-run it, just comment with: |
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
Closes #78122.
Queries transaction metrics in relevant rule types, except when
searchAggregatedTransactions
is set tonever
.auto
works the same asalways
, because there is no check for the presence of transaction metrics. This is to prevent a blocking search in the rule execution, which is performance-sensitive.