[Alerting] Fix the charts on Log Threshold Rule Alert Detail page #160321

simianhacker · 2023-06-22T19:51:19Z

Summary

This PR fixes #160320 by changing the chart from the CriterionPreview chart, borrowed from the Log Threshold Rule, to an embedded Lens visualization that represents the correct document count in one chart. I also took the liberty of changing the ratio chart to use the same technique for consistency sake.

Count with multiple conditions

Before

After

Count with multiple conditions and a group by

Before

After

Ratio with multiple conditions

Before

After

Ratio with multiple conditions and a group by

Before

After

apmmachine · 2023-06-22T19:51:35Z

🤖 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!)

…ld-charts

fkanout

I tested it locally, and it works with count and ratio as excepted.

I left a comment regarding the annotation color to use the them color.
I would suggest moving the chart and its definitions to the @kbn/observability-alert-details package to reuse it for the Threshold Rule alert details.
- We maintain only one chart
- There is an overlap between the rules' use cases.

...ld/components/alert_details_app_section/components/threhsold_chart/create_lens_definition.ts

...k/plugins/infra/public/alerting/log_threshold/components/alert_details_app_section/index.tsx

…ld-charts

benakansara

I noticed a difference in log count between this chart and Log spikes analysis chart. Is this expected?

For this chart, criteria is passed as KQL and group by is passed as filter. In the Log spikes, we are passing everything as filter. Depending on the criteria, there can be different filter types - term, match, match_phrase, range which are used in rule executor. Do they provide same results as KQL generated for this chart?

Also, timestamp and label in the tooltip look different. Should we make it consistent between both charts?

walterra · 2023-06-27T10:29:04Z

@benakansara Explain Log Rate Spikes uses a dynamic agg interval for the chart similar to the chart in Discover, it's not based on the alerts lookback interval. So in the example screenshot it looks like the agg interval for the upper chart is 5 minutes, for Explain Log Rate Spikes it's 1 minute. That should explain the differences for the counts.

benakansara · 2023-06-27T10:35:04Z

@walterra I did sum of the last 5 minutes of data from Logs Spike chart, and thought it should match with data in first chart. But they did not match. Is the doc count shown in the Logs spike chart the exact number of logs or is it average number of logs in 1 minute interval (for above screenshot)?

walterra · 2023-06-27T10:41:49Z

@benakansara Oh sorry I missed you summed up the individual buckets. it should match in this case since it's only a few docs that don't trigger random sampling. I suggest to create a separate issue for this to unblock this PR, it's something we can look into separately and it might have already been the case with the previous chart too.

benakansara · 2023-06-27T12:50:24Z

@walterra sure, I will create an issue for investigation. I am not sure which chart has issue, and whether it is really an issue or not.

Created: #160618

benakansara · 2023-06-27T12:55:05Z

Also, timestamp and label in the tooltip look different. Should we make it consistent between both charts?

@simianhacker what do you think about this?

simianhacker · 2023-06-27T15:10:04Z

Also, timestamp and label in the tooltip look different. Should we make it consistent between both charts?

@benakansara We can change the label to read "document count" instead of "Logs" but the timestamp is configured by Lens.

…ld-charts

simianhacker · 2023-06-27T15:14:52Z

I updated the labels to match.

benakansara · 2023-06-27T15:30:51Z

We have different timestamp in tooltip in each of the charts on the page. We can look into it later to see if there is a way to streamline this.

benakansara · 2023-06-27T15:31:15Z

@simianhacker Do you have any insight on mismatching doc count and difference in query building between first chart and spike analysis chart? Could this be an issue or not really?

I noticed a difference in log count between this chart and Log spikes analysis chart. Is this expected?

For this chart, criteria is passed as KQL and group by is passed as filter. In the Log spikes, we are passing everything as filter. Depending on the criteria, there can be different filter types - term, match, match_phrase, range which are used in rule executor. Do they provide same results as KQL generated for this chart?

Also, timestamp and label in the tooltip look different. Should we make it consistent between both charts?

benakansara

LGTM. Tested locally, and works as expected.

I left a question which may not be related to this PR.

kibana-ci · 2023-06-27T16:18:21Z

💚 Build Succeeded

Buildkite Build
Commit: 8f83bc9

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`infra`	1471	1473	+2

Async chunks

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

id	before	after	diff
`infra`	2.0MB	2.0MB	-4.7KB

Page load bundle

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

id	before	after	diff
`infra`	119.8KB	119.8KB	+23.0B

Unknown metric groups

async chunk count

id	before	after	diff
`infra`	28	29	+1

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	14	16	+2
`infra`	53	52	-1
`securitySolution`	413	417	+4
total			+5

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	15	17	+2
`infra`	63	62	-1
`securitySolution`	492	496	+4
total			+5

History

💚 Build #138257 succeeded 0ed1296
💔 Build #138238 failed aea3f02
💚 Build #138179 succeeded 628f8c7
💔 Build #138163 failed 4ad3ff9
💔 Build #138150 failed 6b7972e
💚 Build #137719 succeeded 6cc959d

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

…60321) ## Summary This PR fixes #160320 by changing the chart from the `CriterionPreview` chart, borrowed from the Log Threshold Rule, to an embedded Lens visualization that represents the correct document count in one chart. I also took the liberty of changing the ratio chart to use the same technique for consistency sake. ## Count with multiple conditions ### Before <img width="736" alt="image" src="https://github.com/elastic/kibana/assets/41702/6c6a27ea-f8e4-491f-8a12-261d0ed13dcb"> ### After <img width="736" alt="image" src="https://github.com/elastic/kibana/assets/41702/9b18ebe9-e911-4e40-8911-bee55cd7d245"> ## Count with multiple conditions and a group by ### Before <img width="736" alt="image" src="https://github.com/elastic/kibana/assets/41702/7b9462da-55b2-4f54-ba09-3c55b372ae2c"> ### After <img width="736" alt="image" src="https://github.com/elastic/kibana/assets/41702/b268caed-242f-430a-ade0-14bf491ec899"> ## Ratio with multiple conditions ### Before <img width="736" alt="image" src="https://github.com/elastic/kibana/assets/41702/55b8dfa2-7789-433b-bffd-e412bdb08b3f"> ### After <img width="736" alt="image" src="https://github.com/elastic/kibana/assets/41702/a029bf8a-3ba1-4e16-87bd-097ebc526a4e"> ## Ratio with multiple conditions and a group by ### Before <img width="736" alt="image" src="https://github.com/elastic/kibana/assets/41702/61ddf1e9-c5ad-4546-a539-15a51ee563c0"> ### After <img width="736" alt="image" src="https://github.com/elastic/kibana/assets/41702/15b0aaa3-4ef9-47f6-baba-24869feae77e"> (cherry picked from commit a8322d2)

kibanamachine · 2023-06-27T16:24:01Z

💚 All backports created successfully

Status	Branch	Result
✅	8.9

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

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

…ge (#160321) (#160661) # Backport This will backport the following commits from `main` to `8.9`: - [[Alerting] Fix the charts on Log Threshold Rule Alert Detail page (#160321)](#160321)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Chris Cowan <[email protected]>

simianhacker · 2023-06-27T18:50:39Z

@benakansara I think the only problem is they are NOT using the same interval. The chart at the top is using the interval defined by the look back window of the rule. The spike analysis is using the "auto" bucketing. When I originally put the chart on the page, it used the same bucketing as the spike analysis, the charts matched exactly. BUT I felt like the minimum bucket size for the chart at the top of the page should reflect the look back window of the rule.

@grabowskit @maciejforcone Do you all have opinions on which interval we should use? We can easily use the same interval as the spike analysis OR should we push to have the spike analysis match the look back window of the alert? I can go either way.

[Alerting] Fix the charts on Log Threhsold Rule Alert Detail page

1a35d7b

Merge branch 'main' of github.com:elastic/kibana into fix-log-thresho…

53107ac

…ld-charts

simianhacker added release_note:fix auto-backport Deprecated - use backport:version if exact versions are needed v8.9.0 v8.10.0 labels Jun 22, 2023

Fixing translations

30ed1a6

simianhacker marked this pull request as ready for review June 23, 2023 14:01

simianhacker requested a review from a team as a code owner June 23, 2023 14:01

Merge branch 'main' of github.com:elastic/kibana into fix-log-thresho…

6cc959d

…ld-charts

fkanout reviewed Jun 23, 2023

View reviewed changes

benakansara reviewed Jun 26, 2023

View reviewed changes

...k/plugins/infra/public/alerting/log_threshold/components/alert_details_app_section/index.tsx Outdated Show resolved Hide resolved

simianhacker added 2 commits June 26, 2023 09:01

Replacing with service

6b7972e

Changing red to euiTheme.colors.danger

4ca0462

simianhacker requested review from fkanout, maryam-saeidi and benakansara June 26, 2023 15:27

Merge branch 'main' of github.com:elastic/kibana into fix-log-thresho…

e946ce5

…ld-charts

simianhacker enabled auto-merge (squash) June 26, 2023 15:28

simianhacker added 5 commits June 26, 2023 09:28

Merge branch 'main' of github.com:elastic/kibana into fix-log-thresho…

4ad3ff9

…ld-charts

remvoing extra services

72b5a0d

Merge branch 'main' of github.com:elastic/kibana into fix-log-thresho…

628f8c7

…ld-charts

Merge branch 'main' of github.com:elastic/kibana into fix-log-thresho…

aea3f02

…ld-charts

Merge branch 'main' of github.com:elastic/kibana into fix-log-thresho…

0ed1296

…ld-charts

benakansara reviewed Jun 27, 2023

View reviewed changes

benakansara mentioned this pull request Jun 27, 2023

Log count not matching between alert duration chart and Log spikes analysis chart #160618

Open

simianhacker added 2 commits June 27, 2023 09:12

Fixing tooltip label for Lens visualizations

eeff595

Merge branch 'main' of github.com:elastic/kibana into fix-log-thresho…

8f83bc9

…ld-charts

simianhacker requested a review from benakansara June 27, 2023 15:15

benakansara approved these changes Jun 27, 2023

View reviewed changes

simianhacker merged commit a8322d2 into elastic:main Jun 27, 2023

kibanamachine mentioned this pull request Jun 27, 2023

[8.9] [Alerting] Fix the charts on Log Threshold Rule Alert Detail page (#160321) #160661

Merged

simianhacker deleted the fix-log-threshold-charts branch April 17, 2024 15:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Alerting] Fix the charts on Log Threshold Rule Alert Detail page #160321

[Alerting] Fix the charts on Log Threshold Rule Alert Detail page #160321

simianhacker commented Jun 22, 2023

apmmachine commented Jun 22, 2023

fkanout left a comment

benakansara left a comment •

edited

Loading

walterra commented Jun 27, 2023

benakansara commented Jun 27, 2023

walterra commented Jun 27, 2023

benakansara commented Jun 27, 2023 •

edited

Loading

benakansara commented Jun 27, 2023

simianhacker commented Jun 27, 2023

simianhacker commented Jun 27, 2023

benakansara commented Jun 27, 2023

benakansara commented Jun 27, 2023

benakansara left a comment

kibana-ci commented Jun 27, 2023

async chunk count

ESLint disabled line counts

Total ESLint disabled count

kibanamachine commented Jun 27, 2023

simianhacker commented Jun 27, 2023

[Alerting] Fix the charts on Log Threshold Rule Alert Detail page #160321

[Alerting] Fix the charts on Log Threshold Rule Alert Detail page #160321

Conversation

simianhacker commented Jun 22, 2023

Summary

Count with multiple conditions

Before

After

Count with multiple conditions and a group by

Before

After

Ratio with multiple conditions

Before

After

Ratio with multiple conditions and a group by

Before

After

apmmachine commented Jun 22, 2023

🤖 GitHub comments

fkanout left a comment

Choose a reason for hiding this comment

benakansara left a comment • edited Loading

Choose a reason for hiding this comment

walterra commented Jun 27, 2023

benakansara commented Jun 27, 2023

walterra commented Jun 27, 2023

benakansara commented Jun 27, 2023 • edited Loading

benakansara commented Jun 27, 2023

simianhacker commented Jun 27, 2023

simianhacker commented Jun 27, 2023

benakansara commented Jun 27, 2023

benakansara commented Jun 27, 2023

benakansara left a comment

Choose a reason for hiding this comment

kibana-ci commented Jun 27, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

async chunk count

ESLint disabled line counts

Total ESLint disabled count

History

kibanamachine commented Jun 27, 2023

💚 All backports created successfully

Questions ?

simianhacker commented Jun 27, 2023

benakansara left a comment •

edited

Loading

benakansara commented Jun 27, 2023 •

edited

Loading