-
Notifications
You must be signed in to change notification settings - Fork 32
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
Monitoring class for process data collection #5747
Monitoring class for process data collection #5747
Conversation
Added Bytes (B) as unit of disk usage tracking. Removed optional type of parameter in method _get_partition_for_path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great Job! I've tested the code and it seems to work correctly. I've left a few comments, mostly about styles.
deps/wazuh_testing/wazuh_testing/process_resource_monitoring/README.md
Outdated
Show resolved
Hide resolved
deps/wazuh_testing/wazuh_testing/process_resource_monitoring/README.md
Outdated
Show resolved
Hide resolved
deps/wazuh_testing/wazuh_testing/process_resource_monitoring/README.md
Outdated
Show resolved
Hide resolved
...esting/wazuh_testing/process_resource_monitoring/src/process_resource_monitoring/__init__.py
Outdated
Show resolved
Hide resolved
...esting/wazuh_testing/process_resource_monitoring/src/process_resource_monitoring/__init__.py
Show resolved
Hide resolved
...uh_testing/process_resource_monitoring/src/process_resource_monitoring/disk_usage_tracker.py
Outdated
Show resolved
Hide resolved
...testing/wazuh_testing/process_resource_monitoring/src/process_resource_monitoring/monitor.py
Show resolved
Hide resolved
...testing/wazuh_testing/process_resource_monitoring/src/process_resource_monitoring/monitor.py
Outdated
Show resolved
Hide resolved
deps/wazuh_testing/wazuh_testing/process_resource_monitoring/src/wazuh_metrics.py
Outdated
Show resolved
Hide resolved
deps/wazuh_testing/wazuh_testing/process_resource_monitoring/src/wazuh_metrics.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great Job!
Some minor changes are requested.
Remember that some unitary tests are mandatory for new development
...uh_testing/process_resource_monitoring/src/process_resource_monitoring/disk_usage_tracker.py
Outdated
Show resolved
Hide resolved
...uh_testing/process_resource_monitoring/src/process_resource_monitoring/disk_usage_tracker.py
Outdated
Show resolved
Hide resolved
...uh_testing/process_resource_monitoring/src/process_resource_monitoring/disk_usage_tracker.py
Show resolved
Hide resolved
...uh_testing/process_resource_monitoring/src/process_resource_monitoring/disk_usage_tracker.py
Show resolved
Hide resolved
...uh_testing/process_resource_monitoring/src/process_resource_monitoring/disk_usage_tracker.py
Outdated
Show resolved
Hide resolved
deps/wazuh_testing/wazuh_testing/process_resource_monitoring/README.md
Outdated
Show resolved
Hide resolved
deps/wazuh_testing/wazuh_testing/process_resource_monitoring/README.md
Outdated
Show resolved
Hide resolved
deps/wazuh_testing/wazuh_testing/process_resource_monitoring/src/wazuh_metrics.py
Outdated
Show resolved
Hide resolved
deps/wazuh_testing/wazuh_testing/process_resource_monitoring/src/wazuh_metrics.py
Outdated
Show resolved
Hide resolved
deps/wazuh_testing/wazuh_testing/process_resource_monitoring/README.md
Outdated
Show resolved
Hide resolved
Removed specific references of Wazuh installation to make it more generic. Changed format of numbers in CSV to have 3 decimal values. Modified the percentage of the disk to be in [0.0, 100.0] instead of [0.0, 1.0]
During the final review, several issues were detected regarding the current approach Not handling of exceptionSubprocess is killed during the monitoringDuring the monitoring of a multiprocess program, it appears that failures
The tool should be capable of handling this The presence of zombie process makes the tool failIf a zombie process is present in the system, the whole monitoring fails:
LoggingTool logging does not differ, contrary to the CSV data, from different iterations of the tool. This should be included in the same directory as the CSV data Reports
➜ Desktop tree -l /tmp/process_metrics
/tmp/process_metrics
├── 27-09-2024
│ ├── 1727423503
│ │ ├── wazuh_apid_29459.csv
│ │ ├── wazuh_apid_39133.csv
│ │ ├── wazuh_apid_39134.csv
│ │ ├── wazuh_apid_39135.csv
│ │ ├── wazuh_apid_39136.csv
│ │ ├── wazuh_apid_39137.csv
│ │ ├── wazuh_apid_39242.csv
│ │ ├── wazuh_apid_39243.csv
│ │ ├── wazuh_apid_child_1.csv
│ │ ├── wazuh_apid_child_2.csv
│ │ ├── wazuh_apid_child_3.csv
│ │ ├── wazuh_apid_child_4.csv
│ │ ├── wazuh_apid_child_5.csv
│ │ └── wazuh_apid.csv
│ └── 1727425589
│ ├── wazuh_apid_29459.csv
│ ├── wazuh_apid_42457.csv
│ ├── wazuh_apid_42458.csv
│ ├── wazuh_apid_42459.csv
│ ├── wazuh_apid_42460.csv
│ ├── wazuh_apid_42461.csv
│ ├── wazuh_apid_42488.csv
│ ├── wazuh_apid_42489.csv
│ ├── wazuh_apid_child_1.csv
│ ├── wazuh_apid_child_2.csv
│ ├── wazuh_apid_child_3.csv
│ ├── wazuh_apid_child_4.csv
│ ├── wazuh_apid_child_5.csv
│ └── wazuh_apid.csv
└── wazuh-resource-metrics.log |
Description
This PR includes the development done to implement
Monitoring class for process data collection
, as well as the documentation related to its use.Related to wazuh/wazuh#24683