johnsusek / praeco

Elasticsearch alerting made simple.
GNU General Public License v3.0
549 stars 88 forks source link

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

Closed SamuelBizon closed 1 year ago

SamuelBizon commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

https://elastalert2.readthedocs.io/en/latest/ruletypes.html#rule-types

nsano-rururu commented 1 year ago

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 commented 1 year ago

https://elastalert2.readthedocs.io/en/latest/recipes/faq.html

nsano-rururu commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

I think the related source code is the following three. I think the first file is the screen display file. https://github.com/johnsusek/praeco/blob/master/src/components/config/ConfigCondition.vue https://github.com/johnsusek/praeco/blob/master/src/store/config/index.js https://github.com/johnsusek/praeco/blob/master/src/store/config/match.js

nsano-rururu commented 1 year ago

showForTheLast

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

Display conditions for showForTheLast

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

nsano-rururu commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@SamuelBizon

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

SamuelBizon commented 1 year ago

Awesome. Thank you so much.

nsano-rururu commented 1 year ago

https://github.com/johnsusek/praeco/commit/d17601bde95c4c7f4e119ff627bf66d60a9acc18

nsano-rururu commented 10 months ago

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