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

Time Window Feature #301

Merged
merged 3 commits into from
Mar 16, 2021
Merged

Time Window Feature #301

merged 3 commits into from
Mar 16, 2021

Conversation

abirunity
Copy link

@abirunity abirunity commented Jan 4, 2021

Hi @johnsusek
My name is Abir and I am working with Praeco in my company (I am working with @osherdp if you remember him).
We wanted to send alerts only during specific time range in the day, so we developed a feature for Praeco.
The feature is called Time Window and the user can use it to send alerts only during specific time range.

This feature includes additions to Praeco gui and one more enhancement in Elastalert-server's elastalert_modules.
I opened pull request for johnsusek/elastalert-server: johnsusek/elastalert-server#6

Credits:
@abirsigron - my presonal account
@IamShobe for helping me with the time window gui additions.

@nsano-rururu
Copy link
Collaborator

Thank you very much for this pull request!

@nsano-rururu
Copy link
Collaborator

@abirunity

Due to the maintenance hibernation of the elastalert formula, we have recently made a bug fix or feature addition pull request for the active fork jertel/elastalert, so we haven't reviewed it yet. I'm sorry. When the work is settled, be sure to review it and merge it if there is no problem.

@nsano-rururu
Copy link
Collaborator

@abirunity

Why are you erasing "config = {... config, ... getters.aggregation};"?

@abirsigron
Copy link

@abirunity

Why are you erasing "config = {... config, ... getters.aggregation};"?

@nsano-rururu
I didn't erease it, just added more necessary configurations for the time window feature:

config = { ...config, ...getters.aggregation, ...getters.timeWindow, match_enhancements: [...existingMatchEnhancements, ...timeWindowMatchEnhancements] };

@nsano-rururu
Copy link
Collaborator

@abirunity

thank you for your answer. I overlooked it.

@nsano-rururu
Copy link
Collaborator

Unit tests that give an error cannot be merged. Do you know the cause?

[CORP\sano@a-ngft53r34ong praeco]$ git fetch origin pull/301/head:timewindow
remote: Enumerating objects: 25, done.
remote: Counting objects: 100% (25/25), done.
remote: Compressing objects: 100% (10/10), done.
remote: Total 25 (delta 15), reused 25 (delta 15), pack-reused 0
Unpacking objects: 100% (25/25), done.
From https://github.com/johnsusek/praeco
 * [new ref]         refs/pull/301/head -> timewindow
 * [new tag]         1.5.2              -> 1.5.2
 * [new tag]         1.5.3              -> 1.5.3
 * [new tag]         1.5.4              -> 1.5.4
[CORP\sano@a-ngft53r34ong praeco]$ git checkout timewindow
Switched to branch 'timewindow'
[CORP\sano@a-ngft53r34ong praeco]$ nvm use "$(cat .nvmrc)"
Now using node v14.15.0 (npm v7.2.0)
[CORP\sano@a-ngft53r34ong praeco]$ npm run test:unit

> [email protected] test:unit
> vue-cli-service test:unit

Download the Vue Devtools extension for a better development experience:
https://github.com/vuejs/vue-devtools
You are running Vue in development mode.
Make sure to turn on production mode when deploying for production.
See more tips at https://vuejs.org/guide/deployment.html
 WEBPACK  Compiling...

  [=========================] 98% (after emitting)

 ERROR  Failed to compile with 1 errors

 error  in ./src/store/config/index.js

Module parse failed: Unexpected token (18048:129)
File was processed with these loaders:
 * ./node_modules/cache-loader/dist/cjs.js
 * ./node_modules/babel-loader/lib/index.js
 * ./node_modules/eslint-loader/index.js
You may need an additional loader to handle the result of these loaders.
| 
| 
>         const existingMatchEnhancements = (cov_2lb7p0f7g0().s[554]++, (cov_2lb7p0f7g0().b[185][0]++, config.match_enhancements) ?? (cov_2lb7p0f7g0().b[185][1]++, []));
|         const timeWindowMatchEnhancements = (cov_2lb7p0f7g0().s[555]++, (cov_2lb7p0f7g0().b[186][0]++, getters.timeWindow.match_enhancements) ?? (cov_2lb7p0f7g0().b[186][1]++, []));
|         cov_2lb7p0f7g0().s[556]++;

 @ ./src/store/index.js 26:37-56
 @ ./tests/unit/specs/ConfigAlert.spec.js
 @ ./node_modules/mochapack/lib/entry.js

  [=========================] 100% (completed)

 WEBPACK  Failed to compile with 1 error(s)

Error in ./src/store/config/index.js

  Module parse failed: Unexpected token (18048:129)
  File was processed with these loaders:
   * ./node_modules/cache-loader/dist/cjs.js
   * ./node_modules/babel-loader/lib/index.js
   * ./node_modules/eslint-loader/index.js
  You may need an additional loader to handle the result of these loaders.
  | 
  | 
  >         const existingMatchEnhancements = (cov_2lb7p0f7g0().s[554]++, (cov_2lb7p0f7g0().b[185][0]++, config.match_enhancements) ?? (cov_2lb7p0f7g0().b[185][1]++, []));
  |         const timeWindowMatchEnhancements = (cov_2lb7p0f7g0().s[555]++, (cov_2lb7p0f7g0().b[186][0]++, getters.timeWindow.match_enhancements) ?? (cov_2lb7p0f7g0().b[186][1]++, []));
  |         cov_2lb7p0f7g0().s[556]++;

 ERROR  mochapack exited with code 1.
npm ERR! code 1
npm ERR! path /home/sano/dkwork3/elastalert_praeco/praeco
npm ERR! command failed
npm ERR! command sh -c vue-cli-service test:unit

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/sano/.npm/_logs/2021-02-09T13_14_06_227Z-debug.log

@IamShobe
Copy link

IamShobe commented Feb 9, 2021

hey looks like nullish-coalescing-operator is yet to be supported: vuejs/vue#10386
either add @babel/plugin-proposal-nullish-coalescing-operator to the babelrc config file
or - @abirunity should write the code and replace every instance of ?? he added with maybe || (I know its not the same but its should work properly there).

@nsano-rururu
Copy link
Collaborator

I feel like I need to modify the results of the test code.

npm install --save-dev @babel/plugin-proposal-nullish-coalescing-operator

add '@babel/plugin-proposal-nullish-coalescing-operator'

vi babel.config.js

module.exports = {
  presets: ['@vue/cli-plugin-babel/preset'],
  plugins: [
    'babel-plugin-istanbul',
    '@babel/plugin-proposal-nullish-coalescing-operator'
  ],
};
[CORP\sano@a-ngft53r34ong praeco]$ npm install

up to date, audited 2072 packages in 3s

26 packages are looking for funding
  run `npm fund` for details

6 low severity vulnerabilities

Some issues need review, and may require choosing
a different dependency.

Run `npm audit` for details.
[CORP\sano@a-ngft53r34ong praeco]$ npm run test:unit

> [email protected] test:unit
> vue-cli-service test:unit

Download the Vue Devtools extension for a better development experience:
https://github.com/vuejs/vue-devtools
You are running Vue in development mode.
Make sure to turn on production mode when deploying for production.
See more tips at https://vuejs.org/guide/deployment.html
 WEBPACK  Compiling...

  [=========================] 98% (after emitting)

 DONE  Compiled successfully in 23687ms

  [=========================] 100% (completed)

 WEBPACK  Compiled successfully in 23687ms

 MOCHA  Testing...



  ConfigAlert
    ✓ renders the realert amount (1985ms)
    ✓ renders the realert unit (47ms)
    ✓ renders the right destination (181ms)
    ✓ renders the right subject (235ms)
    ✓ renders the right body (43ms)
    ✓ renders the right slack channel (62ms)
    ✓ renders the right slack user
    ✓ renders the right slack message color

  ConfigCondition log
    ✓ renders the numEvents (580ms)
    ✓ renders the aboveOrBelow
    ✓ renders the WHEN count
    ✓ renders the OVER all documents
    ✓ renders the UNFILTERED
    ✓ renders the GROUPED OVER field
    ✓ renders the timeframe interval
    ✓ renders the timeframe amount
    ✓ renders the WITH OPTIONS useCountQuery
    ✓ renders the WITH OPTIONS docType

  YAML parsing
    1) renders the correct yaml

  Errors log
    ✓ renders the rule
    ✓ renders the error

  Queries log
    ✓ renders the query log correctly (413ms)

  RuleView
    ✓ renders the rule

  Silences log
    ✓ renders the silence log correctly (242ms)

  alertText
    ✓ returns config-formatted alert text
    ✓ returns html-formatted alert text

  luceneSyntaxBuilder
    ✓ returns correct query string given empty querybuilder tree
    ✓ returns correct query string given filled querybuilder tree
    ✓ returns correct query string given completely filled querybuilder tree


  28 passing (5s)
  1 failing

  1) YAML parsing
       renders the correct yaml:

      AssertionError: expected '__praeco_full_path: test123\n__praeco_query_builder: \'{"query":{"logicalOperator":"all","children":[]}}\'\nalert:\n  - slack\nalert_subject: this is a test subject\nalert_subject_args: []\nalert_text: this is a test body\nalert_text_args: []\nalert_text_type: alert_text_only\ndoc_type: syslog\nfilter:\n  - query:\n      query_string:\n        query: \'@timestamp:*\'\nimport: BaseRule.config\nindex: hannibal-*\nis_enabled: false\nmatch_enhancements: []\nname: test123\nnum_events: 10000\nrealert:\n  minutes: 5\nslack_channel_override: \'#elastalert-debugging\'\nslack_msg_color: danger\nslack_title_link: undefined/rules/test123\nslack_username_override: Praeco\nterms_size: 50\ntimeframe:\n  minutes: 5\ntimestamp_field: \'@timestamp\'\ntimestamp_type: iso\ntype: frequency\nuse_count_query: true\nuse_strftime_index: false\n' to equal '__praeco_full_path: test123\n__praeco_query_builder: \'{"query":{"logicalOperator":"all","children":[]}}\'\nalert:\n  - slack\nalert_subject: this is a test subject\nalert_subject_args: []\nalert_text: this is a test body\nalert_text_args: []\nalert_text_type: alert_text_only\ndoc_type: syslog\nfilter:\n  - query:\n      query_string:\n        query: \'@timestamp:*\'\nimport: BaseRule.config\nindex: hannibal-*\nis_enabled: false\nname: test123\nnum_events: 10000\nrealert:\n  minutes: 5\nslack_channel_override: \'#elastalert-debugging\'\nslack_msg_color: danger\nslack_title_link: undefined/rules/test123\nslack_username_override: Praeco\nterms_size: 50\ntimeframe:\n  minutes: 5\ntimestamp_field: \'@timestamp\'\ntimestamp_type: iso\ntype: frequency\nuse_count_query: true\nuse_strftime_index: false\n'
      + expected - actual

               query: '@timestamp:*'
       import: BaseRule.config
       index: hannibal-*
       is_enabled: false
      -match_enhancements: []
       name: test123
       num_events: 10000
       realert:
         minutes: 5
      
      at Context.<anonymous> (dist/js/webpack:/tests/unit/specs/ConfigYaml.spec.js:49:1)



 MOCHA  Tests completed with 1 failure(s)

@nsano-rururu
Copy link
Collaborator

praeco/tests/unit/specs/ConfigYaml.spec.js

add「match_enhancements: []」

import { expect } from 'chai';
import store from '@/store';
import { mockAxios } from '../setup';
import { ruleYaml } from '../mockData/ruleData.js';

mockAxios.onGet('/api/rules/test123').reply(200, { yaml: ruleYaml });

describe('YAML parsing', () => {
  it('renders the correct yaml', async () => {
    await store.dispatch('config/load', { type: 'rules', path: 'test123' });

    let yaml = store.getters['config/yaml']();

    let expected = `__praeco_full_path: test123
__praeco_query_builder: '{"query":{"logicalOperator":"all","children":[]}}'
alert:
  - slack
alert_subject: this is a test subject
alert_subject_args: []
alert_text: this is a test body
alert_text_args: []
alert_text_type: alert_text_only
doc_type: syslog
filter:
  - query:
      query_string:
        query: '@timestamp:*'
import: BaseRule.config
index: hannibal-*
is_enabled: false
match_enhancements: []
name: test123
num_events: 10000
realert:
  minutes: 5
slack_channel_override: '#elastalert-debugging'
slack_msg_color: danger
slack_title_link: undefined/rules/test123
slack_username_override: Praeco
terms_size: 50
timeframe:
  minutes: 5
timestamp_field: '@timestamp'
timestamp_type: iso
type: frequency
use_count_query: true
use_strftime_index: false
`;

    return expect(yaml).to.equal(expected);
  });
});
[CORP\sano@a-ngft53r34ong praeco]$ npm run test:unit

> [email protected] test:unit
> vue-cli-service test:unit

Download the Vue Devtools extension for a better development experience:
https://github.com/vuejs/vue-devtools
You are running Vue in development mode.
Make sure to turn on production mode when deploying for production.
See more tips at https://vuejs.org/guide/deployment.html
 WEBPACK  Compiling...

  [=========================] 98% (after emitting)

 DONE  Compiled successfully in 10599ms

  [=========================] 100% (completed)

 WEBPACK  Compiled successfully in 10599ms

 MOCHA  Testing...



  ConfigAlert
    ✓ renders the realert amount (1942ms)
    ✓ renders the realert unit
    ✓ renders the right destination (53ms)
    ✓ renders the right subject
    ✓ renders the right body
    ✓ renders the right slack channel (41ms)
    ✓ renders the right slack user
    ✓ renders the right slack message color

  ConfigCondition log
    ✓ renders the numEvents (602ms)
    ✓ renders the aboveOrBelow
    ✓ renders the WHEN count
    ✓ renders the OVER all documents
    ✓ renders the UNFILTERED
    ✓ renders the GROUPED OVER field
    ✓ renders the timeframe interval
    ✓ renders the timeframe amount
    ✓ renders the WITH OPTIONS useCountQuery
    ✓ renders the WITH OPTIONS docType

  YAML parsing
    ✓ renders the correct yaml (92ms)

  Errors log
    ✓ renders the rule
    ✓ renders the error

  Queries log
    ✓ renders the query log correctly (430ms)

  RuleView
    ✓ renders the rule

  Silences log
    ✓ renders the silence log correctly (260ms)

  alertText
    ✓ returns config-formatted alert text
    ✓ returns html-formatted alert text

  luceneSyntaxBuilder
    ✓ returns correct query string given empty querybuilder tree
    ✓ returns correct query string given filled querybuilder tree
    ✓ returns correct query string given completely filled querybuilder tree


  29 passing (4s)

 MOCHA  Tests completed successfully

@nsano-rururu
Copy link
Collaborator

nsano-rururu commented Feb 9, 2021

Isn't it better to add an example of what values to enter for Start time and End time to the explanation in the input field? .. I'm wondering what format to use.
キャプチャ1

If it's HH: mm format, I think it's like Time Picker's Fixed time range. The reason why I think so is that there will always be people who dare to put in strange prices and come up with various things.
https://element.eleme.io/#/en-US/component/time-picker

@abirsigron
Copy link

The format is HH:mm
Time Picker's Fixed time range is a good idea.

@nsano-rururu
Copy link
Collaborator

@abirsigron

Are you using this pull request function in actual operation? If so, is there any problem with other functions?

@nsano-rururu
Copy link
Collaborator

You can edit the start time and end time values when you are not in edit mode. It's a bug.

@nsano-rururu
Copy link
Collaborator

Is there anything else? I want to fix it before merging.

@nsano-rururu
Copy link
Collaborator

@IamShobe @abirsigron

There are concerns about quality. Are you licking?

  • With the Use Time Window enabled, just select "Drop If" and click the "Save" button. It says that the save was successful, but the Use Time Window settings are not saved.

I think it is correct that Start time and End time are required check errors. Did you dare to implement it like this?

  • Select "Drop If" with the Use Time Window enabled, enter only the Start time, and click the "Save" button.

End time is required I think it is correct that the check error occurs. Did you dare to implement it like this?

  • Select "Drop If" with Use Time Window enabled, enter only End time, and click the "Save" button.

I think it is correct that Start time is a required check error. Did you dare to implement it like this?

  • Enter the value (13:00 for Start time, 9:00 for End time) with the Start time and End time reversed with the Use Time Window enabled, and click the "Save" button. The save is successful.

I feel that it is the correct movement to get a magnitude comparison error between Start time and End Time.

@abirsigron
Copy link

Hey @nsano-rururu
I am currently on sick leave.
When I will get back to work I will fix the bugs and let you know.

@abirsigron
Copy link

abirsigron commented Feb 11, 2021

@abirsigron

Are you using this pull request function in actual operation? If so, is there any problem with other functions?

We are using this function in actual operation and the feature is working as expected.
@nsano-rururu

@nsano-rururu
Copy link
Collaborator

@abirsigron

The format is HH:mm
Time Picker's Fixed time range is a good idea.

In this case, I will investigate how to implement it.
It is a one-point confirmation, but it seems that you can select it in 15-minute units depending on the mounting method, but is it better to specify it in 1-minute units?
I wonder which one is better. I'm planning to check if it can be implemented ..

ElementUI el-time-picker- only display hours, minutes, minutes and add range check
https://www.programmersought.com/article/82395029292/

Vue.js ElementUIのTimePickerに任意の値を入力する
https://qiita.com/at-sea/items/4889ffdac9f70be9c279

Vue.js ElementUIのTimePickerに任意の値を入力する(バリデータの追加)
https://qiita.com/at-sea/items/96fec874ab0566512256

@nsano-rururu
Copy link
Collaborator

@abirsigron

I also want to review the source code structure, so I would like to support it. We plan to develop it in another branch. I'd like to ask only for a code review of the finished product, but what about?

@nsano-rururu
Copy link
Collaborator

I will not proceed until I get a reply

@johnsusek
Copy link
Owner

Please make sure to use el-time-select before merging, if you want to proceed, thanks!

@nsano-rururu
Copy link
Collaborator

nsano-rururu commented Mar 7, 2021

@abirsigron @IamShobe

Since there is a conflict file, I resolved it and put the one up to the modified state of the test code in another branch. I'd like to fix it in the following branch and merge it into master if there is no problem, but how about it?
https://github.com/johnsusek/praeco/tree/time_window_feature

@abirunity
Copy link
Author

abirunity commented Mar 7, 2021

Hey @nsano-rururu @johnsusek
Just got back to my job after the sick leave.
Do you want me to fix the conlicts or just review your new branch?

@nsano-rururu
Copy link
Collaborator

nsano-rururu commented Mar 7, 2021

@abirunity

The conflict is fixed in the following branch, so no action is required.
https://github.com/johnsusek/praeco/tree/time_window_feature

Do you want me to fix the conlicts or just review your new branch?

We haven't supported el-time-select or fixed any bugs, so we will contact you if we can.

We are planning to support el-time-select and make it impossible to edit when it is not in edit mode because the movement is strange even though it has a mandatory mark.

@nsano-rururu
Copy link
Collaborator

@abirunity

el-time-select support, bug fixes, and source code configuration review. Please review.
https://github.com/johnsusek/praeco/tree/time_window_feature

1
2
3
4
5
6
7
8

@nsano-rururu
Copy link
Collaborator

We will not proceed until we receive a reply

@abirsigron
Copy link

Sorry for the delay again, I had very busy week.
I reviewed the code, next time can you send PR link instead?
You did very nice job and this feature is much better than I expected it to be when I started it.
Do you think time steps of 15 minutes is good enough?

@nsano-rururu
Copy link
Collaborator

I'm not sure if the time steps are every 15 minutes. For some reason I don't operate a system using Praeco and ElastAlert, either at work or in private.

@nsano-rururu
Copy link
Collaborator

I'm not particular about 15 minutes, so it can be 1 minute or 30 minutes. 1 minute to meet all your needs

@abirsigron
Copy link

1 minute will provide a very long list of options.
Maybe we should devide it to 2 different fields for hours and minutes?
If you don't think so we can leave it as it is.

@nsano-rururu
Copy link
Collaborator

1 minute will provide a very long list of options.
Maybe we should devide it to 2 different fields for hours and minutes?
If you don't think so we can leave it as it is.

Then, do you leave it as it is?
In addition, according to the following reference site, it seems that you can also allow input of arbitrary values in the current state.

Vue.js ElementUIのTimePickerに任意の値を入力する
https://qiita.com/at-sea/items/4889ffdac9f70be9c279

@abirsigron
Copy link

I think we should leave it as it is.
What are the next steps in oder to merge it to the master?

@nsano-rururu
Copy link
Collaborator

This is the work I do, but since there is a difference from the current master, I will take in the contents of master and then merge it into master. I just did another release yesterday, so I'm thinking of releasing it along with what I'm currently doing in another case.

@nsano-rururu nsano-rururu merged commit d89bb37 into johnsusek:master Mar 16, 2021
@nsano-rururu
Copy link
Collaborator

@abirsigron

Merged into Praeco & elastalert-server has released a new Docker image.

https://hub.docker.com/r/praecoapp/praeco
https://hub.docker.com/r/praecoapp/elastalert-server

@abirsigron
Copy link

abirsigron commented Mar 17, 2021

Thanks 👍
It was nice working with you.
@nsano-rururu

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants