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

[Custom threshold] Add prefill functionality for the custom threshold rule #170295

Closed
maryam-saeidi opened this issue Nov 1, 2023 · 6 comments · Fixed by #172783
Closed

[Custom threshold] Add prefill functionality for the custom threshold rule #170295

maryam-saeidi opened this issue Nov 1, 2023 · 6 comments · Fixed by #172783
Assignees
@maryam-saeidi
Labels
Feature: Custom threshold Observability custom threshold rule type Team:obs-ux-management Observability Management User Experience Team

Comments

@maryam-saeidi
Copy link
Member

📝 Summary

Currently, we have prefill functionality in metric threshold to use it in metric explorer, we want to add a similar functionality in the custom threshold rule to use this rule in infra and logs apps. (Here is how it is implemented for the metric threshold rule)

✅ Acceptance Criteria

  • Add prefill functionality for the custom threshold rule
@maryam-saeidi maryam-saeidi added Feature:Alerting Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" v8.12.0 labels Nov 1, 2023
@elasticmachine
Copy link
Contributor

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

@maryam-saeidi maryam-saeidi mentioned this issue Nov 1, 2023
Merged
@paulb-elastic paulb-elastic added the Team:obs-ux-management Observability Management User Experience Team label Nov 13, 2023
@elasticmachine
Copy link
Contributor

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

maryam-saeidi added a commit that referenced this issue Nov 14, 2023
@maryam-saeidi @kibanamachine @fkanout
Revisit custom threshold types in public folder (#170306)
6f2ad26 
Part of #159340
Closes #169364

## Summary

This PR:
1. Removes preFill logic
In this PR, I removed the logic about prefilling custom threshold rule
params as it was originally for other rule types (not custom equation)
and to be used in the Metric threshold rule and the code related to this
logic was super confusing, and I wasn't even sure if it works as
expected since we haven't used this logic anywhere. I created a
[ticket](#170295) to bring back
this feature properly later, specifically for the custom equation, and
integrate it in one of the apps, such as Infra. We also need to be able
to preFill data view information (both adHoc and persisted data view)
2. Renames types and file names 
      - From `metricThreshold` to `customThreshold`
      - From `metricExplorer` to `expression`
3. Removes unused types
4. Remove logic related to aggregations other than the custom equation
at the top level

Also, the fields that end with `pct` now have the `%` after the related
value: (The reason message was fixed in another PR)

<img
src="https://github.com/elastic/kibana/assets/12370520/83694d3b-2ee2-4e95-afe9-5a959c76c3c7"
width=400 />


## 🧪 How to test
- Nothing has changed related to functionality, so please make sure the
custom threshold rule is working as before for
    - Creating a new rule with multiple conditions
    - Adding groups
    - Editing a rule and checking the charts are shown as before
    - Test both adHoc and persisted data view

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Faisal Kanout <faisal.kanout@elastic.co>
@paulb-elastic paulb-elastic removed the Team: Actionable Observability - DEPRECATED For Observability Alerting and SLOs use "Team:obs-ux-management", for AIops "Team:obs-knowledge" label Nov 14, 2023
jillguyonnet pushed a commit to jillguyonnet/kibana that referenced this issue Nov 16, 2023
@maryam-saeidi @kibanamachine
@fkanout @jillguyonnet
Revisit custom threshold types in public folder (elastic#170306)
16f386a 
Part of elastic#159340
Closes elastic#169364

## Summary

This PR:
1. Removes preFill logic
In this PR, I removed the logic about prefilling custom threshold rule
params as it was originally for other rule types (not custom equation)
and to be used in the Metric threshold rule and the code related to this
logic was super confusing, and I wasn't even sure if it works as
expected since we haven't used this logic anywhere. I created a
[ticket](elastic#170295) to bring back
this feature properly later, specifically for the custom equation, and
integrate it in one of the apps, such as Infra. We also need to be able
to preFill data view information (both adHoc and persisted data view)
2. Renames types and file names 
      - From `metricThreshold` to `customThreshold`
      - From `metricExplorer` to `expression`
3. Removes unused types
4. Remove logic related to aggregations other than the custom equation
at the top level

Also, the fields that end with `pct` now have the `%` after the related
value: (The reason message was fixed in another PR)

<img
src="https://github.com/elastic/kibana/assets/12370520/83694d3b-2ee2-4e95-afe9-5a959c76c3c7"
width=400 />


## 🧪 How to test
- Nothing has changed related to functionality, so please make sure the
custom threshold rule is working as before for
    - Creating a new rule with multiple conditions
    - Adding groups
    - Editing a rule and checking the charts are shown as before
    - Test both adHoc and persisted data view

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Faisal Kanout <faisal.kanout@elastic.co>
@maryam-saeidi maryam-saeidi added Feature: Custom threshold Observability custom threshold rule type and removed Feature:Alerting labels Nov 29, 2023
@maryam-saeidi maryam-saeidi self-assigned this Dec 7, 2023
@maryam-saeidi maryam-saeidi mentioned this issue Dec 7, 2023
Merged
@fkanout fkanout mentioned this issue Dec 9, 2023
Closed
4 tasks
@maryam-saeidi maryam-saeidi mentioned this issue Dec 12, 2023
Closed
@gbamparop
Copy link
Contributor

@maryam-saeidi which parts are going to be configurable? Do you need any input from the team?

@maryam-saeidi
Copy link
Member Author

maryam-saeidi commented Dec 12, 2023 •
edited
Loading

@gbamparop At the moment, I added support for the following parts:

  1. Data view
  2. Optional filter
  3. Metrics
  4. Group by

I think it is a good set of options to start with (similar to what we have in the Metric threshold). When you start working on passing parameters, our team can collaborate in case something is missing or there is an issue with this feature.

@gbamparop
Copy link
Contributor

Thanks, the FOR THE LAST option defaults to 1 minute whereas the logs threshold rule to 5 minutes and the threshold defaults to 100 instead of 75. Are there any concerns about these? cc @ruflin

image image

maryam-saeidi added a commit that referenced this issue Dec 14, 2023
@maryam-saeidi
[Custom threshold] Add prefill functionality to the custom threshold …
54616ec 
…rule (#172783)

Closes #170295

## Summary

This PR adds prefill functionality to the custom threshold rule. Since
we don't have any usage for it right now (it will be used in logs and
infra apps), the only way to check it is by looking at tests.
@maryam-saeidi
Copy link
Member Author

@gbamparop I merged the related PR, if something is missing, I will create a separate issue/PR to cover that.

@fkanout fkanout mentioned this issue Jan 15, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@maryam-saeidi maryam-saeidi

Labels
Feature: Custom threshold Observability custom threshold rule type Team:obs-ux-management Observability Management User Experience Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Custom threshold] Add prefill functionality to the custom threshold rule maryam-saeidi/kibana
4 participants
@gbamparop @maryam-saeidi @elasticmachine @paulb-elastic