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.

Sign up for GitHub

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

Jump to bottom

[Synthetics] Fix overview page vizs for large number of monitors !! #199512

Merged
merged 13 commits into from
Nov 21, 2024
Merged

[Synthetics] Fix overview page vizs for large number of monitors !! #199512

merged 13 commits into from
Nov 21, 2024

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Nov 8, 2024 •
edited by kibanamachine
Loading

Summary

Fixes #187264 !!

Apply filters directly instead of passing each monitor id !!

Testing

No special testing is needed, other than make sure, alerts/errors vizs continue to work as expected !!

image

@shahzad31 shahzad31 requested a review from a team as a code owner November 8, 2024 16:32
@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:obs-ux-management Observability Management User Experience Team labels Nov 8, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 8, 2024

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

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

@shahzad31 shahzad31 added release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Nov 8, 2024
@dominiqueclarke dominiqueclarke self-requested a review November 14, 2024 14:14
justinkambic
justinkambic approved these changes Nov 20, 2024
View reviewed changes
Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

LGTM, I did ask for some unit tests in one part

...nthetics/public/apps/synthetics/components/monitors_page/overview/overview/overview_grid.tsx
}}
</FixedSizeList>
)}
{({ onItemsRendered, ref }) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

What lead to you making this change? Seems fine, just curious why this changed as part of this fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was throwing a console error in kibana

...tion/synthetics/public/apps/synthetics/components/monitors_page/hooks/use_monitor_filters.ts
const { status: overviewStatus } = useSelector(selectOverviewStatus);
const allIds = overviewStatus?.allIds ?? [];

return [
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a lot of code branching in here, ideally we'd have some unit tests to make sure we avoid inadvertent modifications to the hook's output.

@elasticmachine
Copy link
Contributor

⏳ Build in-progress

  • Buildkite Build
  • Commit: 40631f4
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-199512-40631f49b531

History

@shahzad31 shahzad31 added backport:version Backport to applied version labels v8.17.0 v8.16.0 and removed backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Nov 21, 2024
@shahzad31 shahzad31 merged commit 944e6fa into elastic:main Nov 21, 2024
36 checks passed
@shahzad31 shahzad31 deleted the fix-vizs branch November 21, 2024 07:27
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.16, 8.17

https://github.com/elastic/kibana/actions/runs/11948560329

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 21, 2024
@shahzad31
[Synthetics] Fix overview page vizs for large number of monitors !! (e…
2c420ff 
…lastic#199512)

## Summary

Fixes elastic#187264 !!

Apply filters directly instead of passing each monitor id !!

### Testing

No special testing is needed, other than make sure, alerts/errors vizs
continue to work as expected !!

<img width="1726" alt="image"
src="https://github.com/user-attachments/assets/9c1889a5-4822-442b-97af-c2a4084c4503">

---------

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit 944e6fa)
@kibanamachine kibanamachine mentioned this pull request Nov 21, 2024
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 21, 2024
@shahzad31
[Synthetics] Fix overview page vizs for large number of monitors !! (e…
df67cda 
…lastic#199512)

## Summary

Fixes elastic#187264 !!

Apply filters directly instead of passing each monitor id !!

### Testing

No special testing is needed, other than make sure, alerts/errors vizs
continue to work as expected !!

<img width="1726" alt="image"
src="https://github.com/user-attachments/assets/9c1889a5-4822-442b-97af-c2a4084c4503">

---------

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit 944e6fa)
@kibanamachine kibanamachine mentioned this pull request Nov 21, 2024
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.16
✅ 8.17

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 Nov 21, 2024
@kibanamachine @shahzad31
[8.16] [Synthetics] Fix overview page vizs for large number of monito…
d0a3d5d 
…rs !! (#199512) (#201080)

# Backport

This will backport the following commits from `main` to `8.16`:
- [[Synthetics] Fix overview page vizs for large number of monitors !!
(#199512)](#199512)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT
[{"author":{"name":"Shahzad","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-21T07:27:41Z","message":"[Synthetics]
Fix overview page vizs for large number of monitors !! (#199512)\n\n##
Summary\r\n\r\nFixes #187264
!!\r\n\r\nApply filters directly instead of passing each monitor id
!!\r\n\r\n### Testing\r\n\r\nNo special testing is needed, other than
make sure, alerts/errors vizs\r\ncontinue to work as expected
!!\r\n\r\n<img width=\"1726\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/9c1889a5-4822-442b-97af-c2a4084c4503\">\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"944e6fa0376702342bb37c3c9893e1574689b211","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","ci:project-deploy-observability","Team:obs-ux-management","v8.16.0","backport:version","v8.17.0"],"title":"[Synthetics]
Fix overview page vizs for large number of monitors
!!","number":199512,"url":"https://github.com/elastic/kibana/pull/199512","mergeCommit":{"message":"[Synthetics]
Fix overview page vizs for large number of monitors !! (#199512)\n\n##
Summary\r\n\r\nFixes #187264
!!\r\n\r\nApply filters directly instead of passing each monitor id
!!\r\n\r\n### Testing\r\n\r\nNo special testing is needed, other than
make sure, alerts/errors vizs\r\ncontinue to work as expected
!!\r\n\r\n<img width=\"1726\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/9c1889a5-4822-442b-97af-c2a4084c4503\">\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"944e6fa0376702342bb37c3c9893e1574689b211"}},"sourceBranch":"main","suggestedTargetBranches":["8.16","8.17"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/199512","number":199512,"mergeCommit":{"message":"[Synthetics]
Fix overview page vizs for large number of monitors !! (#199512)\n\n##
Summary\r\n\r\nFixes #187264
!!\r\n\r\nApply filters directly instead of passing each monitor id
!!\r\n\r\n### Testing\r\n\r\nNo special testing is needed, other than
make sure, alerts/errors vizs\r\ncontinue to work as expected
!!\r\n\r\n<img width=\"1726\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/9c1889a5-4822-442b-97af-c2a4084c4503\">\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"944e6fa0376702342bb37c3c9893e1574689b211"}},{"branch":"8.16","label":"v8.16.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.17","label":"v8.17.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Shahzad <[email protected]>
mbondyra pushed a commit to mbondyra/kibana that referenced this pull request Nov 21, 2024
@shahzad31 @kibanamachine @mbondyra
[Synthetics] Fix overview page vizs for large number of monitors !! (e…
a1f96e1 
…lastic#199512)

## Summary

Fixes elastic#187264 !!

Apply filters directly instead of passing each monitor id !!

### Testing

No special testing is needed, other than make sure, alerts/errors vizs
continue to work as expected !!

<img width="1726" alt="image"
src="https://github.com/user-attachments/assets/9c1889a5-4822-442b-97af-c2a4084c4503">

---------

Co-authored-by: kibanamachine <[email protected]>
@mistic mistic removed the v8.16.1 label Nov 21, 2024
kibanamachine added a commit that referenced this pull request Nov 21, 2024
@kibanamachine @shahzad31 @Ikuni17
[8.17] [Synthetics] Fix overview page vizs for large number of monito…
6fb8dc6 
…rs !! (#199512) (#201081)

# Backport

This will backport the following commits from `main` to `8.17`:
- [[Synthetics] Fix overview page vizs for large number of monitors !!
(#199512)](#199512)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT
[{"author":{"name":"Shahzad","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-21T07:27:41Z","message":"[Synthetics]
Fix overview page vizs for large number of monitors !! (#199512)\n\n##
Summary\r\n\r\nFixes #187264
!!\r\n\r\nApply filters directly instead of passing each monitor id
!!\r\n\r\n### Testing\r\n\r\nNo special testing is needed, other than
make sure, alerts/errors vizs\r\ncontinue to work as expected
!!\r\n\r\n<img width=\"1726\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/9c1889a5-4822-442b-97af-c2a4084c4503\">\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"944e6fa0376702342bb37c3c9893e1574689b211","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","ci:project-deploy-observability","Team:obs-ux-management","v8.16.0","backport:version","v8.17.0"],"title":"[Synthetics]
Fix overview page vizs for large number of monitors
!!","number":199512,"url":"https://github.com/elastic/kibana/pull/199512","mergeCommit":{"message":"[Synthetics]
Fix overview page vizs for large number of monitors !! (#199512)\n\n##
Summary\r\n\r\nFixes #187264
!!\r\n\r\nApply filters directly instead of passing each monitor id
!!\r\n\r\n### Testing\r\n\r\nNo special testing is needed, other than
make sure, alerts/errors vizs\r\ncontinue to work as expected
!!\r\n\r\n<img width=\"1726\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/9c1889a5-4822-442b-97af-c2a4084c4503\">\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"944e6fa0376702342bb37c3c9893e1574689b211"}},"sourceBranch":"main","suggestedTargetBranches":["8.16","8.17"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/199512","number":199512,"mergeCommit":{"message":"[Synthetics]
Fix overview page vizs for large number of monitors !! (#199512)\n\n##
Summary\r\n\r\nFixes #187264
!!\r\n\r\nApply filters directly instead of passing each monitor id
!!\r\n\r\n### Testing\r\n\r\nNo special testing is needed, other than
make sure, alerts/errors vizs\r\ncontinue to work as expected
!!\r\n\r\n<img width=\"1726\"
alt=\"image\"\r\nsrc=\"https://github.com/user-attachments/assets/9c1889a5-4822-442b-97af-c2a4084c4503\">\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"944e6fa0376702342bb37c3c9893e1574689b211"}},{"branch":"8.16","label":"v8.16.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.17","label":"v8.17.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Shahzad <[email protected]>
Co-authored-by: Brad White <[email protected]>
@kibanamachine kibanamachine mentioned this pull request Nov 26, 2024
Merged
74 tasks
paulinashakirova pushed a commit to paulinashakirova/kibana that referenced this pull request Nov 26, 2024
@shahzad31 @kibanamachine @paulinashakirova
[Synthetics] Fix overview page vizs for large number of monitors !! (e…
5b6a570 
…lastic#199512)

## Summary

Fixes elastic#187264 !!

Apply filters directly instead of passing each monitor id !!

### Testing

No special testing is needed, other than make sure, alerts/errors vizs
continue to work as expected !!

<img width="1726" alt="image"
src="https://github.com/user-attachments/assets/9c1889a5-4822-442b-97af-c2a4084c4503">

---------

Co-authored-by: kibanamachine <[email protected]>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Dec 12, 2024
@shahzad31 @kibanamachine @CAWilson94
[Synthetics] Fix overview page vizs for large number of monitors !! (e…
d562c38 
…lastic#199512)

## Summary

Fixes elastic#187264 !!

Apply filters directly instead of passing each monitor id !!

### Testing

No special testing is needed, other than make sure, alerts/errors vizs
continue to work as expected !!

<img width="1726" alt="image"
src="https://github.com/user-attachments/assets/9c1889a5-4822-442b-97af-c2a4084c4503">

---------

Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justinkambic justinkambic justinkambic approved these changes

@dominiqueclarke dominiqueclarke Awaiting requested review from dominiqueclarke

Assignees
No one assigned
Labels
backport:version Backport to applied version labels ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-management Observability Management User Experience Team v8.16.2 v8.17.0 v8.18.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Synthetics] Large numbers of monitors break graphs on the Overview page
6 participants
@shahzad31 @elasticmachine @kibanamachine @dominiqueclarke @justinkambic @mistic