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

Can not select a Timeframe in any other option than "count". #588

Closed
SamuelBizon opened this issue Nov 8, 2023 · 16 comments
Closed

Can not select a Timeframe in any other option than "count". #588

SamuelBizon opened this issue Nov 8, 2023 · 16 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@SamuelBizon
Copy link
Contributor

Image versions:
praecoapp/praeco:1.8.16
praecoapp/elastalert-server:20231017

When choosing "count" option:
image
you can choose the timeframe "for the last" x minutes.

However, if you choose anything other than "count", for example "average", you can not choose the timeframe:
image

Also, notice that "is above" is showing double.

@SamuelBizon SamuelBizon added the bug Something isn't working label Nov 8, 2023
@SamuelBizon
Copy link
Contributor Author

Also in the first rule, in the yaml file it makes a field:
timeframe:
minutes: 25
But in the second one it doesnt make any timeframe field.

@nsano-rururu
Copy link
Collaborator

@nsano-rururu
Copy link
Collaborator

If yaml is not generated according to the specifications, we will consider fixing it, but Praeco is only a function to generate yaml to be used with elastalert2 with a GUI, and yaml operation is not supported.
As for the behavior of the generated yaml, you should probably ask in the elastalert2 discussion.
The rule type name displayed on the screen and the name of elastalert2 are different, so I would like to make them consistent in the future.
I didn't create it from scratch, but inherited it from somewhere in the middle, so I currently don't understand all the implementation specifications.
The double display is a bug, but since I don't do it for work, I can't promise when it will be fixed.
https://github.com/jertel/elastalert2/discussions

@nsano-rururu
Copy link
Collaborator

@nsano-rururu
Copy link
Collaborator

nsano-rururu commented Nov 8, 2023

If you read the documentation, you should understand that the timeframe setting is not a setting used by all rule types. Why not start by researching it yourself?
https://elastalert2.readthedocs.io/en/latest/ruletypes.html#rule-types

@SamuelBizon
Copy link
Contributor Author

I understood that the buffer_time serves similar function in Metric Aggregation ("average" selected) as timeframe in Frequency ("count" selected). Wouldnt it make sense to also have the "for the last x" for this ruletype as well but change the buffer_time instead ?
image
image
Sorry I might have missunderstood.

One more bug I have found: When creating a rule and choosing certain amount of minutes in the "for the x" option while "count" selected and then changing "count" to "average", the "for the x" option still stays there:
image
This caused me a little bit of confusion and I thought it is supposed to be there, you just cant edit it.

@nsano-rururu
Copy link
Collaborator

@nsano-rururu
Copy link
Collaborator

The following condition 941 is incorrect. It seems that it will be displayed if you modify the following so that it is true.
https://github.com/johnsusek/praeco/blob/master/src/components/config/ConfigCondition.vue#L941

@nsano-rururu
Copy link
Collaborator

The correct move is for buffer_time to be configurable in metric_aggregation.
The elastalert2 sample yaml also looks like this.
https://github.com/jertel/elastalert2/blob/master/examples/rules/example_single_metric_agg.yaml

@nsano-rururu nsano-rururu self-assigned this Nov 13, 2023
@nsano-rururu nsano-rururu added this to the 1.8.17 milestone Nov 13, 2023
@nsano-rururu
Copy link
Collaborator

https://github.com/johnsusek/praeco/blob/master/src/components/config/ConfigCondition.vue#L577

current

v-show="metricAggType === 'count' || metricAggType === 'field changes' || metricAggType === 'cardinality'"

after

v-show="metricAggType === 'count' || metricAggType === 'field changes' || metricAggType === 'cardinality' || metricAggType === 'avg' || metricAggType === 'sum' || metricAggType === 'min' || metricAggType === 'max'"

https://github.com/johnsusek/praeco/blob/master/src/components/config/ConfigCondition.vue#L586

current

<span v-if="metricAggType === 'count' || metricAggType === 'cardinality'">FOR </span>

after

<span v-if="metricAggType === 'count' || metricAggType === 'cardinality' || metricAggType === 'avg' || metricAggType === 'sum' || metricAggType === 'min' || metricAggType === 'max'">FOR </span>

https://github.com/johnsusek/praeco/blob/master/src/components/config/ConfigCondition.vue#L941

current

return this.metricAggType !== 'count' && this.metricAggType !== 'field changes' && this.metricAggType !== 'cardinality';

after

return this.metricAggType !== 'count' && this.metricAggType !== 'field changes' && this.metricAggType !== 'cardinality' && this.metricAggType !== 'avg' && this.metricAggType !== 'sum' && this.metricAggType !== 'min' && this.metricAggType !== 'max';

@nsano-rururu
Copy link
Collaborator

https://github.com/johnsusek/praeco/blob/master/src/components/config/ConfigCondition.vue#L448
current

        <span v-if="spikeOrThreshold === 'is' || metricAggType !== 'count'" class="pop-trigger">
          <span>IS</span>
          <span v-if="numEvents || maxThreshold">
            ABOVE {{ metricAggType === 'count' ? numEvents : maxThreshold }}
          </span>
          <span v-if="(numEvents && threshold) || (maxThreshold && minThreshold)">
            &amp;
          </span>
          <span v-if="threshold || minThreshold">
            BELOW {{ metricAggType === 'count' ? threshold : minThreshold }}
          </span>
        </span>

        <span v-if="spikeOrThreshold === 'is' || metricAggType !== 'count'" class="pop-trigger">
          <span>IS</span>
          <span v-if="numEvents || maxThreshold">
            ABOVE {{ metricAggType === 'count' ? numEvents : maxThreshold }}
          </span>
          <span v-if="(numEvents && threshold) || (maxThreshold && minThreshold)">
            &amp;
          </span>
          <span v-if="threshold || minThreshold">
            BELOW {{ metricAggType === 'count' ? threshold : minThreshold }}
          </span>
        </span>

after

        <span v-if="spikeOrThreshold === 'is' || metricAggType !== 'count'" class="pop-trigger">
          <span>IS</span>
          <span v-if="numEvents || maxThreshold">
            ABOVE {{ metricAggType === 'count' ? numEvents : maxThreshold }}
          </span>
          <span v-if="(numEvents && threshold) || (maxThreshold && minThreshold)">
            &amp;
          </span>
          <span v-if="threshold || minThreshold">
            BELOW {{ metricAggType === 'count' ? threshold : minThreshold }}
          </span>
        </span>

@nsano-rururu
Copy link
Collaborator

@SamuelBizon

The fixes have been identified and will be included in the next update, 1.8.17.

@SamuelBizon
Copy link
Contributor Author

Awesome. Thank you so much.

@nsano-rururu
Copy link
Collaborator

d17601b

@nsano-rururu
Copy link
Collaborator

1.8.17 released. Contains corrections.
Docker image has also been released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants