Skip to content
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

Fix the charts and group by section on the Log Threshold alert detail page #155327

Conversation

simianhacker
Copy link
Member

@simianhacker simianhacker commented Apr 19, 2023

Summary

This PR fixes #155083 with the following changes:

  • Create a new field to store the action context for an alert under ALERT_CONTEXT (kibana.alert.context) for Log Threshold Rule.
  • Change the alert detail page to reference the groupByKeys under ALERT_CONTEXT for the group by section
  • Change the history chart to only display 12h buckets

I plan to do a follow up PR to add the ALERT_CONTEXT to the other Observability Rules which we will also need for our alert details pages.

How to test

  1. Index data using: https://github.com/elastic/high-cardinality-cluster/tree/main/high_cardinality_indexer by running the following command:
DATASET="fake_stack" EVENTS_PER_CYCLE=1 INDEX_INTERVAL=60000 ELASTICSEARCH_HOSTS=http://localhost:9200 node src/run.js
  1. Create a DataView for named "Admin Console" with the index pattern of high-cardinality-data-fake_stack.admin-console-* and the timestamp field set to @timestamp
  2. Go to the Log Stream in Observability and change the index pattern to "Admin Console"
  3. Create a rule that looks like:

image

  1. Create a rule WITHOUT a group by that will trigger and check the alert detail page
  2. Create a rule with a ratio WITHOUT a group by that will trigger and check the alert detail page
  3. Create a rule with a ratio WITH a group by that will trigger and check the alert detail page

@simianhacker simianhacker added release_note:skip Skip the PR/issue when compiling release notes Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" v8.8.0 labels Apr 19, 2023
@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

Copy link
Contributor

@kdelemme kdelemme left a comment

Choose a reason for hiding this comment

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

LGTM!
Tested locally and work as expected!

packages/kbn-rule-data-utils/src/default_alerts_as_data.ts Outdated Show resolved Hide resolved
@@ -232,7 +240,9 @@ const AlertDetailsAppSection = ({
rule &&
rule.params.criteria.length === 1 && (
<EuiFlexItem>
<LogsHistoryChart rule={rule} />
<LogsHistoryChart
rule={{ ...rule, params: { ...rule.params, timeSize: 12, timeUnit: 'h' } }}
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

@simianhacker simianhacker marked this pull request as ready for review April 20, 2023 13:00
@simianhacker simianhacker requested review from a team as code owners April 20, 2023 13:00
@elasticmachine
Copy link
Contributor

Pinging @elastic/actionable-observability (Team: Actionable Observability)

Copy link
Contributor

@kdelemme kdelemme left a comment

Choose a reason for hiding this comment

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

👍🏻

@@ -82,6 +83,11 @@ export const alertFieldMap = {
array: false,
required: false,
},
[ALERT_CONTEXT]: {
Copy link
Contributor

Choose a reason for hiding this comment

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

this adds kibana.alert.context to the framework alert mappings that applies to all alerts as data docs, not just the log threshold mappings. was that the intent?

Copy link
Contributor

Choose a reason for hiding this comment

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

If you just want it for the log threshold rule type, I would maybe just update the legacyExperimentalFieldMap in packages/kbn-alerts-as-data-utils/src/field_maps/legacy_experimental_field_map.ts. That is also used by the other observability rule types but doesn't affect every rule type.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes... we plan to update ALL of the Observability Rules to store their context in the AAD documents. There have been few things we've needed access to for the Alert Details page. Every time we need something from the context, we have to debate "Where should we store that in AAD?" This seems like something every alert document should have, the context the alert was triggered with.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll move it to the legacy_experimental_field_map.ts

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 I think it makes sense to start there and then if we see a broader use for it we can promote it to the framework mappings. Since it's being indexed as an object is there a chance that multiple rule types could use the same context field and a different type and cause mapping clashes?

Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

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

response ops changes lgtm

Copy link
Member

@spong spong left a comment

Choose a reason for hiding this comment

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

Security Detection changes LGTM 👍

…x-charts-and-group-by-attributes-on-detail-page
@simianhacker
Copy link
Member Author

@elasticmachine merge upstream

Copy link
Contributor

@crespocarlos crespocarlos left a comment

Choose a reason for hiding this comment

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

Infra Monitoring UI changes LGTM.

@simianhacker simianhacker enabled auto-merge (squash) May 11, 2023 15:16
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

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
@kbn/rule-data-utils 102 103 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
apm 3.5MB 3.5MB +5.0B
infra 2.0MB 2.0MB +149.0B
observability 972.1KB 972.1KB +44.0B
securitySolution 9.2MB 9.2MB +261.0B
triggersActionsUi 1.4MB 1.4MB +5.0B
total +464.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
apm 32.9KB 33.0KB +54.0B
infra 101.7KB 101.8KB +54.0B
observability 57.0KB 57.0KB +54.0B
triggersActionsUi 86.9KB 87.0KB +54.0B
total +216.0B
Unknown metric groups

API count

id before after diff
@kbn/rule-data-utils 105 106 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 400 404 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 480 484 +4
total +6

History

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request May 11, 2023
… page (elastic#155327)

## Summary

This PR fixes elastic#155083 with the following changes:

- Create a new field to store the action context for an alert under
`ALERT_CONTEXT` (`kibana.alert.context`) for Log Threshold Rule.
- Change the alert detail page to reference the `groupByKeys` under
`ALERT_CONTEXT` for the group by section
- Change the history chart to only display `12h` buckets

I plan to do a follow up PR to add the ALERT_CONTEXT to the other
Observability Rules which we will also need for our alert details pages.

### How to test

1. Index data using:
https://github.com/elastic/high-cardinality-cluster/tree/main/high_cardinality_indexer
by running the following command:
```
DATASET="fake_stack" EVENTS_PER_CYCLE=1 INDEX_INTERVAL=60000 ELASTICSEARCH_HOSTS=http://localhost:9200 node src/run.js
```
2. Create a DataView for named "Admin Console" with the index pattern of
`high-cardinality-data-fake_stack.admin-console-*` and the timestamp
field set to `@timestamp`
3. Go to the Log Stream in Observability and change the index pattern to
"Admin Console"
4. Create a rule that looks like:

<img width="600" alt="image"
src="https://user-images.githubusercontent.com/41702/232578891-e65a3f1a-457c-459a-8d7f-cadc85e7067c.png">

5. Create a rule WITHOUT a group by that will trigger and check the
alert detail page
6. Create a rule with a ratio WITHOUT a group by that will trigger and
check the alert detail page
7. Create a rule with a ratio WITH a group by that will trigger and
check the alert detail page

---------

Co-authored-by: Kevin Delemme <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
(cherry picked from commit 78671f1)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.8

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 May 11, 2023
…detail page (#155327) (#157410)

# Backport

This will backport the following commits from `main` to `8.8`:
- [Fix the charts and group by section on the Log Threshold alert detail
page (#155327)](#155327)

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

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

<!--BACKPORT [{"author":{"name":"Chris
Cowan","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-05-11T15:54:35Z","message":"Fix
the charts and group by section on the Log Threshold alert detail page
(#155327)\n\n## Summary\r\n\r\nThis PR fixes #155083 with the following
changes:\r\n\r\n- Create a new field to store the action context for an
alert under\r\n`ALERT_CONTEXT` (`kibana.alert.context`) for Log
Threshold Rule.\r\n- Change the alert detail page to reference the
`groupByKeys` under\r\n`ALERT_CONTEXT` for the group by section\r\n-
Change the history chart to only display `12h` buckets\r\n\r\nI plan to
do a follow up PR to add the ALERT_CONTEXT to the other\r\nObservability
Rules which we will also need for our alert details pages.\r\n\r\n###
How to test\r\n\r\n1. Index data
using:\r\nhttps://github.com/elastic/high-cardinality-cluster/tree/main/high_cardinality_indexer\r\nby
running the following command:\r\n```\r\nDATASET=\"fake_stack\"
EVENTS_PER_CYCLE=1 INDEX_INTERVAL=60000
ELASTICSEARCH_HOSTS=http://localhost:9200 node src/run.js\r\n```\r\n2.
Create a DataView for named \"Admin Console\" with the index pattern
of\r\n`high-cardinality-data-fake_stack.admin-console-*` and the
timestamp\r\nfield set to `@timestamp`\r\n3. Go to the Log Stream in
Observability and change the index pattern to\r\n\"Admin Console\"\r\n4.
Create a rule that looks like:\r\n\r\n<img width=\"600\"
alt=\"image\"\r\nsrc=\"https://user-images.githubusercontent.com/41702/232578891-e65a3f1a-457c-459a-8d7f-cadc85e7067c.png\">\r\n\r\n5.
Create a rule WITHOUT a group by that will trigger and check
the\r\nalert detail page\r\n6. Create a rule with a ratio WITHOUT a
group by that will trigger and\r\ncheck the alert detail page\r\n7.
Create a rule with a ratio WITH a group by that will trigger
and\r\ncheck the alert detail
page\r\n\r\n---------\r\n\r\nCo-authored-by: Kevin Delemme
<[email protected]>\r\nCo-authored-by: Kibana Machine
<[email protected]>","sha":"78671f113cc68f5e1696bbe4aed2320978c97e11","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:
Actionable
Observability","v8.8.0","v8.9.0"],"number":155327,"url":"https://github.com/elastic/kibana/pull/155327","mergeCommit":{"message":"Fix
the charts and group by section on the Log Threshold alert detail page
(#155327)\n\n## Summary\r\n\r\nThis PR fixes #155083 with the following
changes:\r\n\r\n- Create a new field to store the action context for an
alert under\r\n`ALERT_CONTEXT` (`kibana.alert.context`) for Log
Threshold Rule.\r\n- Change the alert detail page to reference the
`groupByKeys` under\r\n`ALERT_CONTEXT` for the group by section\r\n-
Change the history chart to only display `12h` buckets\r\n\r\nI plan to
do a follow up PR to add the ALERT_CONTEXT to the other\r\nObservability
Rules which we will also need for our alert details pages.\r\n\r\n###
How to test\r\n\r\n1. Index data
using:\r\nhttps://github.com/elastic/high-cardinality-cluster/tree/main/high_cardinality_indexer\r\nby
running the following command:\r\n```\r\nDATASET=\"fake_stack\"
EVENTS_PER_CYCLE=1 INDEX_INTERVAL=60000
ELASTICSEARCH_HOSTS=http://localhost:9200 node src/run.js\r\n```\r\n2.
Create a DataView for named \"Admin Console\" with the index pattern
of\r\n`high-cardinality-data-fake_stack.admin-console-*` and the
timestamp\r\nfield set to `@timestamp`\r\n3. Go to the Log Stream in
Observability and change the index pattern to\r\n\"Admin Console\"\r\n4.
Create a rule that looks like:\r\n\r\n<img width=\"600\"
alt=\"image\"\r\nsrc=\"https://user-images.githubusercontent.com/41702/232578891-e65a3f1a-457c-459a-8d7f-cadc85e7067c.png\">\r\n\r\n5.
Create a rule WITHOUT a group by that will trigger and check
the\r\nalert detail page\r\n6. Create a rule with a ratio WITHOUT a
group by that will trigger and\r\ncheck the alert detail page\r\n7.
Create a rule with a ratio WITH a group by that will trigger
and\r\ncheck the alert detail
page\r\n\r\n---------\r\n\r\nCo-authored-by: Kevin Delemme
<[email protected]>\r\nCo-authored-by: Kibana Machine
<[email protected]>","sha":"78671f113cc68f5e1696bbe4aed2320978c97e11"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/155327","number":155327,"mergeCommit":{"message":"Fix
the charts and group by section on the Log Threshold alert detail page
(#155327)\n\n## Summary\r\n\r\nThis PR fixes #155083 with the following
changes:\r\n\r\n- Create a new field to store the action context for an
alert under\r\n`ALERT_CONTEXT` (`kibana.alert.context`) for Log
Threshold Rule.\r\n- Change the alert detail page to reference the
`groupByKeys` under\r\n`ALERT_CONTEXT` for the group by section\r\n-
Change the history chart to only display `12h` buckets\r\n\r\nI plan to
do a follow up PR to add the ALERT_CONTEXT to the other\r\nObservability
Rules which we will also need for our alert details pages.\r\n\r\n###
How to test\r\n\r\n1. Index data
using:\r\nhttps://github.com/elastic/high-cardinality-cluster/tree/main/high_cardinality_indexer\r\nby
running the following command:\r\n```\r\nDATASET=\"fake_stack\"
EVENTS_PER_CYCLE=1 INDEX_INTERVAL=60000
ELASTICSEARCH_HOSTS=http://localhost:9200 node src/run.js\r\n```\r\n2.
Create a DataView for named \"Admin Console\" with the index pattern
of\r\n`high-cardinality-data-fake_stack.admin-console-*` and the
timestamp\r\nfield set to `@timestamp`\r\n3. Go to the Log Stream in
Observability and change the index pattern to\r\n\"Admin Console\"\r\n4.
Create a rule that looks like:\r\n\r\n<img width=\"600\"
alt=\"image\"\r\nsrc=\"https://user-images.githubusercontent.com/41702/232578891-e65a3f1a-457c-459a-8d7f-cadc85e7067c.png\">\r\n\r\n5.
Create a rule WITHOUT a group by that will trigger and check
the\r\nalert detail page\r\n6. Create a rule with a ratio WITHOUT a
group by that will trigger and\r\ncheck the alert detail page\r\n7.
Create a rule with a ratio WITH a group by that will trigger
and\r\ncheck the alert detail
page\r\n\r\n---------\r\n\r\nCo-authored-by: Kevin Delemme
<[email protected]>\r\nCo-authored-by: Kibana Machine
<[email protected]>","sha":"78671f113cc68f5e1696bbe4aed2320978c97e11"}}]}]
BACKPORT-->

Co-authored-by: Chris Cowan <[email protected]>
@simianhacker simianhacker deleted the issue-155083-fix-charts-and-group-by-attributes-on-detail-page branch April 17, 2024 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" v8.8.0 v8.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Charts are not displaying properly on Log Threshold Rule's alert detail page
9 participants