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

[Actionable Observability] hide rules from sidebar and move under alerts #128437

Merged
merged 5 commits into from
Mar 24, 2022
Merged

[Actionable Observability] hide rules from sidebar and move under alerts #128437

merged 5 commits into from
Mar 24, 2022

Conversation

mgiota
Copy link
Contributor

@mgiota mgiota commented Mar 23, 2022 •
edited
Loading

Fixes #128435

Acceptance criteria

  • Remove rules menu option from Observability sidebar
  • Update rules route path to be under alerts /alerts/rules
  • Update Manage rules link to /app/observability/alerts/rules
  • Update Breadcrumb to be Observability > Alerts > Rules

@mgiota mgiota force-pushed the 128435_update_rules_link branch from e0345ce to 6037b6a Compare March 23, 2022 21:56
@mgiota mgiota marked this pull request as ready for review March 23, 2022 22:09
@mgiota mgiota requested review from a team as code owners March 23, 2022 22:09
@mgiota mgiota self-assigned this Mar 23, 2022
@mgiota mgiota added Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" release_note:skip Skip the PR/issue when compiling release notes v8.2.0 labels Mar 23, 2022
katrin-freihofner
katrin-freihofner reviewed Mar 24, 2022
View reviewed changes
Copy link
Contributor

@katrin-freihofner katrin-freihofner left a comment

Choose a reason for hiding this comment

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

Looks good from the UX side.

This was referenced Mar 24, 2022
Closed
Closed
@mgiota
Copy link
Contributor Author

mgiota commented Mar 24, 2022

@elasticmachine merge upstream

shahzad31
shahzad31 approved these changes Mar 24, 2022
View reviewed changes
Copy link
Contributor

@shahzad31 shahzad31 left a comment

Choose a reason for hiding this comment

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

Uptime changes LGTM !!

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
observability 415.9KB 416.1KB +167.0B

Page load bundle

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

id before after diff
observability 90.2KB 90.2KB +15.0B

History

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

cc @mgiota

@mgiota mgiota enabled auto-merge (squash) March 24, 2022 10:21
@mgiota mgiota requested a review from fkanout March 24, 2022 10:30
gbamparop
gbamparop approved these changes Mar 24, 2022
View reviewed changes
Copy link
Contributor

@gbamparop gbamparop left a comment

Choose a reason for hiding this comment

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

APM changes LGTM!

fkanout
fkanout approved these changes Mar 24, 2022
View reviewed changes
Copy link
Contributor

@fkanout fkanout 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 tested it and it matches the ACs. I left one suggestion regarding the feature flag

x-pack/plugins/observability/public/plugin.ts
},
getCasesDeepLinks({
basePath: casesPath,
extend: {
[CasesDeepLinkId.cases]: {
order: 8003,
order: 8002,
Copy link
Contributor

@fkanout fkanout Mar 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@mgiota, What is order? it seems like a CSS z-index

x-pack/plugins/observability/public/pages/alerts/containers/alerts_page/alerts_page.tsx
@@ -142,7 +142,7 @@ function AlertsPage() {
}, []);

const manageRulesHref = config.unsafe.rules.enabled
Copy link
Contributor

Choose a reason for hiding this comment

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

@mgiota, I'm thinking if we need to update the feature flag to align it with this update. e.g. xpack.observability.unsafe.alerts.rules.enabled ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good thought! Let me merge this PR to hide the menu option from sidebar for now and we could do it in another PR. I am hoping that we will soon get rid of this flag completely, so I wouldn't worry much about it now.

@mgiota mgiota disabled auto-merge March 24, 2022 15:47
@mgiota mgiota enabled auto-merge (squash) March 24, 2022 15:48
@Kerry350 Kerry350 self-requested a review March 24, 2022 16:58
Kerry350
Kerry350 approved these changes Mar 24, 2022
View reviewed changes
Copy link
Contributor

@Kerry350 Kerry350 left a comment

Choose a reason for hiding this comment

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

Changes to the infra tests LGTM 👍

@mgiota mgiota merged commit 8d1b8c0 into elastic:main Mar 24, 2022
@mgiota mgiota added the backport:skip This commit does not require backporting label Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fkanout fkanout fkanout approved these changes

@gbamparop gbamparop gbamparop approved these changes

@shahzad31 shahzad31 shahzad31 approved these changes

@Kerry350 Kerry350 Kerry350 approved these changes

@katrin-freihofner katrin-freihofner Awaiting requested review from katrin-freihofner

Assignees

@mgiota mgiota

Labels
backport:skip This commit does not require backporting 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.2.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Move rules menu option under alerts
8 participants
@mgiota @kibana-ci @Kerry350 @shahzad31 @gbamparop @fkanout @katrin-freihofner @kibanamachine