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.

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

Update flapping code once the flapping lookback value is configurable #145929

Closed
doakalexi opened this issue Nov 21, 2022 · 4 comments · Fixed by #149448
Closed

Update flapping code once the flapping lookback value is configurable #145929

doakalexi opened this issue Nov 21, 2022 · 4 comments · Fixed by #149448
Assignees
Labels
blocked Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)

Comments

@doakalexi
Copy link
Contributor

doakalexi commented Nov 21, 2022

The flapping lookback value is hardcoded to 20 in this PR, which is making the functional tests run for a long time.

We should update them to be faster once the lookback value is configurable and make the necessary changes to read from this new location.

@doakalexi doakalexi added the Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) label Nov 21, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@mikecote
Copy link
Contributor

mikecote commented Jan 3, 2023

Adding blocked label until #143529 is resolved.

@mikecote mikecote changed the title Update flapping tests once the flapping lookback value is configurable Update flapping code once the flapping lookback value is configurable Jan 17, 2023
@doakalexi doakalexi self-assigned this Jan 19, 2023
@pmuellr
Copy link
Member

pmuellr commented Jan 24, 2023

We now have the flapping configuration available, I think the missing link is we need to thread this info through the rule executor to use instead of the hard-coded values. Which is a bit more work than this issue describes, which sounds like customizing function test configurations.

Should we re-target this issue to do that - pass the flapping setting info through the rule executor? Or open a new issue?

@doakalexi
Copy link
Contributor Author

Yes, we should retarget this issue. I have already starting on using the configured values instead of hard coded

doakalexi added a commit that referenced this issue Feb 2, 2023
…ck value is configurable (#149448)

Resolves #145929

## Summary

Updates previous flapping tests to use the new flapping settings
configs.
Updates flapping logic to use flapping configs instead of hardcoded
values. Calls the flapping api on every rule execution, and then passes
in the flapping settings to the rule executors so they can be used by
the rule registry.


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios


### To verify

I think it's helpful to hide the whitespace when reviewing this pr.

- The flapping logic should remain the same, and all previous tests
should pass. I only updated them to pass in the flapping settings.
- Create rules, and set flapping settings in the ui and see the flapping
behavior change for your rules.
- Verify that the
`x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log.ts`
run with the new flapping configs and output results we would expect

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
ogupte pushed a commit to ogupte/kibana that referenced this issue Feb 3, 2023
…ck value is configurable (elastic#149448)

Resolves elastic#145929

## Summary

Updates previous flapping tests to use the new flapping settings
configs.
Updates flapping logic to use flapping configs instead of hardcoded
values. Calls the flapping api on every rule execution, and then passes
in the flapping settings to the rule executors so they can be used by
the rule registry.


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios


### To verify

I think it's helpful to hide the whitespace when reviewing this pr.

- The flapping logic should remain the same, and all previous tests
should pass. I only updated them to pass in the flapping settings.
- Create rules, and set flapping settings in the ui and see the flapping
behavior change for your rules.
- Verify that the
`x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log.ts`
run with the new flapping configs and output results we would expect

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
kqualters-elastic pushed a commit to kqualters-elastic/kibana that referenced this issue Feb 6, 2023
…ck value is configurable (elastic#149448)

Resolves elastic#145929

## Summary

Updates previous flapping tests to use the new flapping settings
configs.
Updates flapping logic to use flapping configs instead of hardcoded
values. Calls the flapping api on every rule execution, and then passes
in the flapping settings to the rule executors so they can be used by
the rule registry.


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios


### To verify

I think it's helpful to hide the whitespace when reviewing this pr.

- The flapping logic should remain the same, and all previous tests
should pass. I only updated them to pass in the flapping settings.
- Create rules, and set flapping settings in the ui and see the flapping
behavior change for your rules.
- Verify that the
`x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log.ts`
run with the new flapping configs and output results we would expect

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
darnautov pushed a commit to darnautov/kibana that referenced this issue Feb 7, 2023
…ck value is configurable (elastic#149448)

Resolves elastic#145929

## Summary

Updates previous flapping tests to use the new flapping settings
configs.
Updates flapping logic to use flapping configs instead of hardcoded
values. Calls the flapping api on every rule execution, and then passes
in the flapping settings to the rule executors so they can be used by
the rule registry.


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios


### To verify

I think it's helpful to hide the whitespace when reviewing this pr.

- The flapping logic should remain the same, and all previous tests
should pass. I only updated them to pass in the flapping settings.
- Create rules, and set flapping settings in the ui and see the flapping
behavior change for your rules.
- Verify that the
`x-pack/test/alerting_api_integration/spaces_only/tests/alerting/event_log.ts`
run with the new flapping configs and output results we would expect

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)
Projects
No open projects
4 participants