Skip to content

[Synthetics] Settings add config to enable default rules#190800

Merged
shahzad31 merged 19 commits intoelastic:mainfrom
shahzad31:default-rules-config
Aug 22, 2024
Merged

[Synthetics] Settings add config to enable default rules#190800
shahzad31 merged 19 commits intoelastic:mainfrom
shahzad31:default-rules-config

Conversation

@shahzad31
Copy link
Contributor

Summary

Settings add config to enable default rules !!

separated out of #186585 !!

image

@obltmachine
Copy link

🤖 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 marked this pull request as ready for review August 20, 2024 14:05
@shahzad31 shahzad31 requested a review from a team as a code owner August 20, 2024 14:05
@shahzad31 shahzad31 added the release_note:feature Makes this part of the condensed release notes label Aug 20, 2024
@botelastic botelastic bot added ci:project-deploy-observability Create an Observability project Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics. labels Aug 20, 2024
@elasticmachine
Copy link
Contributor

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

import { AlertInstanceState } from '@kbn/alerting-plugin/server';
import { AlertInstanceContext } from '@kbn/alerting-plugin/server';
import { uptimeRuleFieldMap } from '../../../../common/rules/uptime_rule_field_map';
import { syntheticsRuleFieldMap } from '@kbn/synthetics-plugin/common/rules/synthetics_rule_field_map';
Copy link
Contributor

Choose a reason for hiding this comment

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

I want to thoroughly avoid changes to Uptime. I feel like any kind of unnecessary change raises the risk matrix for having to deal with Uptime SDHs, and we certainly don't have time for that. I don't think this is a necessary change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should be reverted, will do it.

@dominiqueclarke dominiqueclarke self-requested a review August 20, 2024 17:06
Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

The disabling doesn't appear to work as expected. I still received an alert and the rule appears to still be enabled from management. I would expect that this setting actually disables the default rule from the alerting side

With that in mind, maybe we should create new settings for this purpose, but instead use the rule state as the single source of truth

On this page we can fetch the default rule as see if it's enabled, then the toggle will be one. When the user clicks the toggle off and saves, it'll just disable the rule from the alerting plugin.
Screenshot 2024-08-20 at 1 39 14 PM
Screenshot 2024-08-20 at 1 40 19 PM
Screenshot 2024-08-20 at 1 40 36 PM

statusRule,
tlsRule,
};
const deleteStatusRulePromise =
Copy link
Contributor

Choose a reason for hiding this comment

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

We should move this into the DefaultAlertService.

@shahzad31 shahzad31 requested a review from a team as a code owner August 22, 2024 10:30
Copy link
Contributor

@jeramysoucy jeramysoucy left a comment

Choose a reason for hiding this comment

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

AppEx Security changes LGTM. No ESO changes.

@kibana-ci
Copy link

kibana-ci commented Aug 22, 2024

💛 Build succeeded, but was flaky

  • Buildkite Build
  • Commit: dc2bf20
  • Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-190800-dc2bf20734b2

Failed CI Steps

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
synthetics 935.9KB 937.6KB +1.6KB

History

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

Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

LGTM

@shahzad31 shahzad31 merged commit 20ccb21 into elastic:main Aug 22, 2024
@shahzad31 shahzad31 deleted the default-rules-config branch August 22, 2024 15:13
@kibanamachine kibanamachine added v8.16.0 backport:skip This PR does not require backporting labels Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting ci:project-deploy-observability Create an Observability project release_note:feature Makes this part of the condensed release notes Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics. v8.16.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants