File watch sensor to utilize trigger with parameters #3361

lakshmi-kannan · 2017-04-18T14:57:12Z

Cherry-picked #1291

Then, I made the following changes:

I enabled the file_watch_sensor by default.
I edited the sensor YAML file to reflect pack name explicitly
I added a sample rule which would register a trigger with parameter (file_path). Sensor automatically picks up the file name from rule and starts to listen to file and emit lines from file.

To test, do the following:

Spin st2 up and place the linux pack from this branch to /opt/stackstorm/packs
Register contents of linux pack - st2ctl reload --register-all
Restart st2 so file_watch_sensor is run - st2ctl restart
Verify file_watch_sensor is running and enabled

(virtualenv)vagrant@st2dev /m/s/s/st2 ❯❯❯ ps auxww | grep file_watch | grep -v grep                                      tt_with_params ✭ ◼
vagrant  15142  0.1  2.0 111104 42520 pts/5    S+   07:19   0:04 /mnt/src/storm/st2/virtualenv/bin/python /mnt/src/storm/st2/st2reactor/st2reactor/container/sensor_wrapper.py --pack=linux --file-path=/opt/stackstorm/packs/linux/sensors/file_watch_sensor.py --class-name=FileWatchSensor --trigger-type-refs=linux.file_watch.file_path,linux.file_watch.line --parent-args=["--config-file", "/mnt/src/storm/st2/tools/../conf/st2.dev.conf"]
(virtualenv)vagrant@st2dev /m/s/s/st2 ❯❯❯

(virtualenv)vagrant@st2dev /m/s/s/st2 ❯❯❯ st2 sensor list --pack=linux                                                   tt_with_params ✭ ◼
+-----------------------+-------+-------------------------------------+---------+
| ref                   | pack  | description                         | enabled |
+-----------------------+-------+-------------------------------------+---------+
| linux.FileWatchSensor | linux | Sensor which monitors files for new | True    |
|                       |       | lines                               |         |
+-----------------------+-------+-------------------------------------+---------+
(virtualenv)vagrant@st2dev /m/s/s/st2 ❯❯❯

Now copy sample rule that will register the rule

(virtualenv)vagrant@st2dev /m/s/s/st2 ❯❯❯ st2 rule create /opt/stackstorm/packs/examples/rules/sample_rule_file_watch.yaml
+-------------+-----------------------------------------------------------+
| Property    | Value                                                     |
+-------------+-----------------------------------------------------------+
| id          | 58f620c6d9d7ed3af4554f4d                                  |
| name        | sample_rule_file_watch                                    |
| pack        | examples                                                  |
| description | Sample rule customer trigger type.                        |
| action      | {                                                         |
|             |     "ref": "core.local",                                  |
|             |     "parameters": {                                       |
|             |         "cmd": "echo "{{trigger}}""                       |
|             |     }                                                     |
|             | }                                                         |
| criteria    |                                                           |
| enabled     | False                                                     |
| ref         | examples.sample_rule_file_watch                           |
| tags        |                                                           |
| trigger     | {                                                         |
|             |     "type": "linux.file_watch.file_path",                 |
|             |     "ref": "linux.1c65e441-fd18-4217-bc53-8ac616a31074",  |
|             |     "parameters": {                                       |
|             |         "file_path": "/var/log/dmesg"                     |
|             |     }                                                     |
|             | }                                                         |
| type        | {                                                         |
|             |     "ref": "standard",                                    |
|             |     "parameters": {}                                      |
|             | }                                                         |
| uid         | rule:examples:sample_rule_file_watch                      |
+-------------+-----------------------------------------------------------+
(virtualenv)vagrant@st2dev /m/s/s/st2 ❯❯❯

Kami · 2017-04-20T09:09:21Z

contrib/linux/sensors/file_watch_sensor.py


    def add_trigger(self, trigger):
-        pass
+        if trigger['type'] not in ['linux.file_watch.file_path']:


As discussed on Slack - I prototyped it this way for easiness.

If we decide to go with this approach I would propose to move this filtering to the base sensor class so those trigger handler methods only get called with triggers which belong to that sensor (so no additional filtering needed in sensor class trigger handler methods).

Another, even better future optimization (better than this suggested later filtering in the base class) would be to use a more advanced message routing so each sensor class would only receive triggers it should receive. Problem with that approach is that it's not fully backward compatible and would need to research how expensive it is to use queue per sensor (probably not too expensive).

@Kami I think the trigger check is not strictly required. We already filter in TriggerWatcher.

(virtualenv)vagrant@st2dev /m/s/s/st2 ❯❯❯ ps auxww | grep file_watch | grep -v grep tt_with_params ✭ ◼ vagrant 18758 1.1 2.1 115520 45008 pts/4 S+ 14:25 0:02 /mnt/src/storm/st2/virtualenv/bin/python /mnt/src/storm/st2/st2reactor/st2reactor/container/sensor_wrapper.py --pack=linux --file-path=/opt/stackstorm/packs/linux/sensors/file_watch_sensor.py --class-name=FileWatchSensor --trigger-type-refs=linux.file_watch.file_path,linux.file_watch.line --parent-args=["--config-file", "/mnt/src/storm/st2/tools/../conf/st2.dev.conf"] (virtualenv)vagrant@st2dev /m/s/s/st2 ❯❯❯

Note we pass --trigger-type-refs to the sensor wrapper. This is then passed to TriggerWatcher
https://github.com/StackStorm/st2/blob/master/st2reactor/st2reactor/container/sensor_wrapper.py#L354

https://github.com/StackStorm/st2/blob/master/st2reactor/st2reactor/container/sensor_wrapper.py#L192

https://github.com/StackStorm/st2/blob/master/st2common/st2common/services/triggerwatcher.py#L93

So I thought you originally did it for safety measure.

Good call - this was probably added after my original trigger with parameters change was implemented so it looks like my comment is obsolete :)

In any case, would be good to double check and verify that is indeed the case.

I verified and we are filtering right in TriggerWatcher and sensors don't see triggers that they don't register.

Cool, in this case we should get rid of checks in the sensor code :)

dzimine · 2017-04-25T18:59:40Z

This looks right.
I don't see any draw-backs with this approach, if there are, pls list. Regardless, let's move forward.

To complete this PR, let's modify the line-watching sensor to take path as a trigger parameter, not a global parameter, too.

If we decide to go with this approach I would propose to move this filtering to the base sensor class so those trigger handler methods only get called with triggers which belong to that sensor (so no additional filtering needed in sensor class trigger handler methods).
👍

lakshmi-kannan · 2017-04-25T21:18:16Z

To complete this PR, let's modify the line-watching sensor to take path as a trigger parameter, not a global parameter, too.

Dude, that's what this PR does. I am confused.

I don't see any draw-backs with this approach, if there are, pls list

I do see a drawback which is the sensor code is now responsible for some kind of scheduling handling the trigger types. In this particular case, this logic is hidden luckily https://github.com/StackStorm/st2/pull/3361/files#diff-414b51e67b8a124ee29083fc2bbce3a8R19. But it also is a poor example for people. We might want to add an example where we loop over the trigger types (may come from different rules) and do something.

dzimine

+1

dzimine · 2017-04-26T01:20:28Z

Dude, that's what this PR does. I am confused.

I think you're adding file_watcher which takes parameters, but the line_watcher still relies on the global, no?

Kami · 2017-04-26T08:19:08Z

Let's please also update the changelog.

Kami · 2017-04-26T08:21:22Z

@dzimine

I think you're adding file_watcher which takes parameters, but the line_watcher still relies on the global, no?

Not exactly sure what you mean with line watcher? There is no such thing as line watcher (or am I missing something) - file watcher emits a trigger each time a change (new line) is detected on monitored file.

dzimine

@lakshmi-kannan there seem to be a mess between trigger references.

I don't quite get how you handle both .line and .file_pathtriggers.

Also, I am having troubles to make it work but likely because I'm in a rush and did something wrong.

dzimine · 2017-05-06T22:43:04Z

contrib/linux/sensors/file_watch_sensor.py

+        self._logger.info('Removed file "%s"' % (file_path))

    def _handle_line(self, file_path, line):
        trigger = self._trigger_ref


Will it be always emitting 'linux.file_watch.line, see line 12?

dzimine · 2017-05-06T22:54:34Z

contrib/examples/rules/sample_rule_file_watch.yaml

+trigger:
+  parameters:
+    file_path: /var/log/dmesg
+  type: linux.file_watch.file_path


The emitted trigger is linux.file_watch.line (I checked the logs, too) and here it is linux.file_watch.file_path. Funniest is that it works.

dzimine · 2017-05-07T07:31:17Z

contrib/linux/sensors/file_watch_sensor.yaml

    -
      name: "file_watch.line"
+      pack: "linux"
      description: "Trigger which indicates a new line has been detected"


need to add parameters_schema here, too

Pack config was removed for the linux pack in #3361 and #3475. Remove the README note as it no longer applies.

Kami reviewed Apr 20, 2017

View reviewed changes

lakshmi-kannan changed the title ~~WIP: File watch sensor to utilize trigger with parameters~~ File watch sensor to utilize trigger with parameters Apr 26, 2017

dzimine approved these changes Apr 26, 2017

View reviewed changes

Kami approved these changes Apr 26, 2017

View reviewed changes

Kami and others added 5 commits April 27, 2017 11:03

Update FileWatchSensor to utilize trigger parameters.

9ea1067

Cleanup sensor YAML and enable sensor by default

b53f8b8

Add a rule to watch a file (trigger type with parameters)

6837265

Rewrite trigger type check in file_watch_sensor.py

40daf1a

Update CHANGELOG

e9c7f52

lakshmi-kannan force-pushed the tt_with_params branch from 293afee to e9c7f52 Compare April 27, 2017 18:31

Lakshmi Kannan added 2 commits April 27, 2017 11:37

Update remove_trigger method

1b5a72a

Remove duplicate line in remove_trigger

f342598

lakshmi-kannan merged commit c48c6c3 into StackStorm:master Apr 27, 2017

lakshmi-kannan deleted the tt_with_params branch April 27, 2017 20:10

dzimine reviewed May 6, 2017

View reviewed changes

dzimine reviewed May 7, 2017

View reviewed changes

cognifloyd mentioned this pull request Jan 30, 2024

Refactor FileWatchSensor to remove logshipper #5096

Draft

cognifloyd added a commit that referenced this pull request Jan 30, 2024

linux pack: Drop out-of-date README notes about pack config

ed32348

Pack config was removed for the linux pack in #3361 and #3475. Remove the README note as it no longer applies.

Uh oh!

File watch sensor to utilize trigger with parameters #3361

File watch sensor to utilize trigger with parameters #3361

Uh oh!

Conversation

lakshmi-kannan commented Apr 18, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dzimine commented Apr 25, 2017

Uh oh!

lakshmi-kannan commented Apr 25, 2017

Uh oh!

dzimine left a comment

Choose a reason for hiding this comment

Uh oh!

dzimine commented Apr 26, 2017

Uh oh!

Kami commented Apr 26, 2017

Uh oh!

Kami commented Apr 26, 2017

Uh oh!

dzimine left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants