Skip to content

[Response Ops][Alerting] Remove echoed field value from bulk error responses when indexing alerts#172020

Merged
ymao1 merged 2 commits intoelastic:mainfrom
ymao1:rule-registry/less-verbose-error
Nov 28, 2023
Merged

[Response Ops][Alerting] Remove echoed field value from bulk error responses when indexing alerts#172020
ymao1 merged 2 commits intoelastic:mainfrom
ymao1:rule-registry/less-verbose-error

Conversation

@ymao1
Copy link
Contributor

@ymao1 ymao1 commented Nov 27, 2023

Summary

When alerts are bulk indexed in the rule registry and the alerts client, indexing errors may be returned where the entire field value that failed to be indexed is echoed in the reason. This can cause unnecessarily verbose logging so we want to sanitize the field value.

@ymao1 ymao1 self-assigned this Nov 27, 2023
@ymao1 ymao1 added Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// v8.12.0 labels Nov 27, 2023
@kibana-ci
Copy link

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
alerting 780 782 +2
Unknown metric groups

API count

id before after diff
alerting 811 813 +2

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @ymao1

@ymao1 ymao1 marked this pull request as ready for review November 28, 2023 12:37
@ymao1 ymao1 requested review from a team as code owners November 28, 2023 12:37
@elasticmachine
Copy link
Contributor

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

Copy link
Contributor

@ersin-erdal ersin-erdal left a comment

Choose a reason for hiding this comment

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

LGTM

@ymao1 ymao1 merged commit d9ebfd9 into elastic:main Nov 28, 2023
@ymao1 ymao1 deleted the rule-registry/less-verbose-error branch November 28, 2023 16:25
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 28, 2023
…sponses when indexing alerts (elastic#172020)

## Summary

When alerts are bulk indexed in the rule registry and the alerts client,
indexing errors may be returned where the entire field value that failed
to be indexed is echoed in the reason. This can cause unnecessarily
verbose logging so we want to sanitize the field value.

(cherry picked from commit d9ebfd9)
@kibanamachine
Copy link
Contributor

💔 Some backports could not be created

Status Branch Result
7.17 Backport failed because of merge conflicts
8.11

Note: Successful backport PRs will be merged automatically after passing CI.

Manual backport

To create the backport manually run:

node scripts/backport --pr 172020

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Nov 28, 2023
…rror responses when indexing alerts (#172020) (#172085)

# Backport

This will backport the following commits from `main` to `8.11`:
- [[Response Ops][Alerting] Remove echoed field value from bulk error
responses when indexing alerts
(#172020)](#172020)

<!--- Backport version: 8.9.7 -->

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

<!--BACKPORT [{"author":{"name":"Ying
Mao","email":"ying.mao@elastic.co"},"sourceCommit":{"committedDate":"2023-11-28T16:25:03Z","message":"[Response
Ops][Alerting] Remove echoed field value from bulk error responses when
indexing alerts (#172020)\n\n## Summary\r\n\r\nWhen alerts are bulk
indexed in the rule registry and the alerts client,\r\nindexing errors
may be returned where the entire field value that failed\r\nto be
indexed is echoed in the reason. This can cause unnecessarily\r\nverbose
logging so we want to sanitize the field
value.","sha":"d9ebfd9af1365bba54d5e1ac92f5e53f5fbebea8","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Alerting","release_note:skip","Team:ResponseOps","backport:prev-minor","backport:prev-MAJOR","v8.12.0"],"number":172020,"url":"https://github.com/elastic/kibana/pull/172020","mergeCommit":{"message":"[Response
Ops][Alerting] Remove echoed field value from bulk error responses when
indexing alerts (#172020)\n\n## Summary\r\n\r\nWhen alerts are bulk
indexed in the rule registry and the alerts client,\r\nindexing errors
may be returned where the entire field value that failed\r\nto be
indexed is echoed in the reason. This can cause unnecessarily\r\nverbose
logging so we want to sanitize the field
value.","sha":"d9ebfd9af1365bba54d5e1ac92f5e53f5fbebea8"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/172020","number":172020,"mergeCommit":{"message":"[Response
Ops][Alerting] Remove echoed field value from bulk error responses when
indexing alerts (#172020)\n\n## Summary\r\n\r\nWhen alerts are bulk
indexed in the rule registry and the alerts client,\r\nindexing errors
may be returned where the entire field value that failed\r\nto be
indexed is echoed in the reason. This can cause unnecessarily\r\nverbose
logging so we want to sanitize the field
value.","sha":"d9ebfd9af1365bba54d5e1ac92f5e53f5fbebea8"}}]}]
BACKPORT-->

Co-authored-by: Ying Mao <ying.mao@elastic.co>
@ymao1
Copy link
Contributor Author

ymao1 commented Nov 28, 2023

Backport to 7.17 is unnecessary because alerts client did not exist in 7.17 and the error log inside the rule registry error handling was added in 8.0 in this PR: #120439

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// v8.11.2 v8.12.0

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

5 participants