[ResponseOps][alerting] add rule info to logging in alertsClient#190857
Merged
pmuellr merged 1 commit intoelastic:mainfrom Aug 22, 2024
Merged
[ResponseOps][alerting] add rule info to logging in alertsClient#190857pmuellr merged 1 commit intoelastic:mainfrom
pmuellr merged 1 commit intoelastic:mainfrom
Conversation
While investigating some issues with the alertsClient, I realized that we weren't writing out any rule information for the logged messages. This made debugging quite difficult, as I wanted to see the rule, so had to search through the alerts indices for the specified alert to get it's rule id, rule type, etc. This PR adds that kind of rule info to the logged messages in alertsClient, as well as the typical sort of tags we write out (rule id, rule type, module).
pmuellr
added
Feature:Alerting
release_note:skip
Contributor
|
Pinging @elastic/response-ops (Team:ResponseOps) |
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]
To update your PR or re-run it, just comment with: |
ymao1
reviewed
Aug 21, 2024
| this._isUsingDataStreams = this.options.dataStreamAdapter.isUsingDataStreams(); | ||
| this.ruleInfoMessage = `for ${this.ruleType.id}:${this.options.rule.id} '${this.options.rule.name}'`; | ||
| this.logTags = { tags: [this.ruleType.id, this.options.rule.id, 'alerts-client'] }; | ||
| } |
Contributor
There was a problem hiding this comment.
With this PR: #187785, the logger that's passed into the alerts client should already include the rule ID and rule type ID in the tags, so I think you just need to add the alerts-client tag.
Contributor
Author
There was a problem hiding this comment.
I tried this out, and it didn't add the tags. Looks like we're getting a normal logger here
which is then replaced in run() here:
Looking through the code, seems like it should have picked up ours, not the original one, not sure why it didn't.
I tested this by changing a condition around one of the comparisons in the code that would generate a log, like here:
I changed !isValidAlertIndexName() to isValidAlertIndexName().
To see the tags when running, I had to switch to JSON mode:
logging:
appenders:
console_appender:
type: console
layout:
type: json
# type: pattern - what I normally use
# highlight: trueI also ran this in a debugger and saw that it wasn't our logger but a base one, when the alertsClient logger logged.
I feel like opening another issue to investigate this, rather than trying to figure out why, then fix it, and find 10K tests to fix :-). But maybe it's simpler?
Contributor
There was a problem hiding this comment.
Weird. But I'm fine with figuring it out later :)
Contributor
There was a problem hiding this comment.
Looks like it's here:
Instead of passing in this.options.logger into the AlertsClient, we could pass in opts.logger to get the TaskRunner logger. But again, it's not a big deal and I don't think it's something we need to update rn.
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Aug 22, 2024
…stic#190857) ## Summary While investigating some issues with the alertsClient, I realized that we weren't writing out any rule information for the logged messages. This made debugging quite difficult, as I wanted to see the rule, so had to search through the alerts indices for the specified alert to get it's rule id, rule type, etc. As an example, see elastic#190376 This PR adds that kind of rule info to the logged messages in alertsClient, as well as the typical sort of tags we write out (rule id, rule type, module). (cherry picked from commit 07717a4)
Contributor
💚 All backports created successfully
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
Aug 22, 2024
…nt (#190857) (#191094) # Backport This will backport the following commits from `main` to `8.15`: - [[ResponseOps][alerting] add rule info to logging in alertsClient (#190857)](#190857) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Patrick Mueller","email":"patrick.mueller@elastic.co"},"sourceCommit":{"committedDate":"2024-08-22T14:29:22Z","message":"[ResponseOps][alerting] add rule info to logging in alertsClient (#190857)\n\n## Summary\r\n\r\nWhile investigating some issues with the alertsClient, I realized that\r\nwe weren't writing out any rule information for the logged messages.\r\nThis made debugging quite difficult, as I wanted to see the rule, so had\r\nto search through the alerts indices for the specified alert to get it's\r\nrule id, rule type, etc.\r\n\r\nAs an example, see https://github.com/elastic/kibana/issues/190376\r\n\r\nThis PR adds that kind of rule info to the logged messages in\r\nalertsClient, as well as the typical sort of tags we write out (rule id,\r\nrule type, module).","sha":"07717a43ab369847d87c8e15071759502a89c48b","branchLabelMapping":{"^v8.16.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Alerting","release_note:skip","Team:ResponseOps","v8.16.0","v8.15.1"],"title":"[ResponseOps][alerting] add rule info to logging in alertsClient","number":190857,"url":"https://github.com/elastic/kibana/pull/190857","mergeCommit":{"message":"[ResponseOps][alerting] add rule info to logging in alertsClient (#190857)\n\n## Summary\r\n\r\nWhile investigating some issues with the alertsClient, I realized that\r\nwe weren't writing out any rule information for the logged messages.\r\nThis made debugging quite difficult, as I wanted to see the rule, so had\r\nto search through the alerts indices for the specified alert to get it's\r\nrule id, rule type, etc.\r\n\r\nAs an example, see https://github.com/elastic/kibana/issues/190376\r\n\r\nThis PR adds that kind of rule info to the logged messages in\r\nalertsClient, as well as the typical sort of tags we write out (rule id,\r\nrule type, module).","sha":"07717a43ab369847d87c8e15071759502a89c48b"}},"sourceBranch":"main","suggestedTargetBranches":["8.15"],"targetPullRequestStates":[{"branch":"main","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/190857","number":190857,"mergeCommit":{"message":"[ResponseOps][alerting] add rule info to logging in alertsClient (#190857)\n\n## Summary\r\n\r\nWhile investigating some issues with the alertsClient, I realized that\r\nwe weren't writing out any rule information for the logged messages.\r\nThis made debugging quite difficult, as I wanted to see the rule, so had\r\nto search through the alerts indices for the specified alert to get it's\r\nrule id, rule type, etc.\r\n\r\nAs an example, see https://github.com/elastic/kibana/issues/190376\r\n\r\nThis PR adds that kind of rule info to the logged messages in\r\nalertsClient, as well as the typical sort of tags we write out (rule id,\r\nrule type, module).","sha":"07717a43ab369847d87c8e15071759502a89c48b"}},{"branch":"8.15","label":"v8.15.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Patrick Mueller <patrick.mueller@elastic.co>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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
While investigating some issues with the alertsClient, I realized that we weren't writing out any rule information for the logged messages. This made debugging quite difficult, as I wanted to see the rule, so had to search through the alerts indices for the specified alert to get it's rule id, rule type, etc.
As an example, see #190376
This PR adds that kind of rule info to the logged messages in alertsClient, as well as the typical sort of tags we write out (rule id, rule type, module).
Checklist
Delete any items that are not applicable to this PR.