-
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
Enhance StatisticMonitor with API support #4970
Enhance StatisticMonitor with API support #4970
Conversation
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.
Good jobs. Some changes are required
urllib3.disable_warnings() | ||
|
||
API_URL="https://localhost:55000" | ||
DAEMONS_ENDPOINT="/manager/daemons/stats?daemons_list=wazuh-analysisd,wazuh-remoted,wazuh-db" |
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.
Hardcoded daemons endpoint
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.
Fixed in 8128e8a
import wazuh_testing.tools as tls | ||
urllib3.disable_warnings() | ||
|
||
API_URL="https://localhost:55000" |
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.
Hardcoded api url and port
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.
Fixed in 8128e8a
|
||
|
||
|
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.
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.
Fixed in 8128e8a
@@ -26,13 +26,16 @@ def get_script_arguments(): | |||
formatter_class=argparse.RawTextHelpFormatter) | |||
parser.add_argument('-t', '--target', dest='target_list', required=True, type=str, nargs='+', action='store', | |||
help='Type the statistics target to collect separated by whitespace. ' | |||
'Targets: agent, logcollector, remote and analysis.') | |||
'Targets: agent, logcollector, remote, analysis_events, analysisd_state and wazuhdb') |
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.
This could affect other QA process. Did it only affect test_cluster https://github.com/wazuh/wazuh-jenkins/pull/6276/files?
@@ -40,44 +52,56 @@ class StatisticMonitor: | |||
dst_dir (str): directory to store the CSVs. Defaults to temp directory. | |||
csv_file (str): path to the CSV file. | |||
target (str): target file to monitor. | |||
parse_json (bool): Determine if the file is a JSON file. Default False. |
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.
parse_json (bool): Determine if the file is a JSON file. Default False. |
data = daemons_response.json()['data']['affected_items'] | ||
self._write_csv(data, self.target, self.csv_file) | ||
|
||
|
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.
if response.status_code != 200: | ||
logging.info("Retrying get API data, status code {}".format(response.status_code)) | ||
return self._parse_api_data() | ||
|
||
daemons_response = requests.get(API_URL + DAEMONS_ENDPOINT, verify=False, headers={'Authorization': 'Bearer ' + response.json()['data']['token']}) | ||
if daemons_response.status_code != 200: | ||
logging.info("Retrying get API data, status code {}".format(response.status_code)) | ||
return self._parse_api_data() |
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.
Infinite loop. We should include some minor logic in order to prevent credential and others errors. In addition, logging should be error instead of info
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.
fixed in 135c652
timestamp = datetime.fromtimestamp(time()).strftime('%Y-%m-%d %H:%M:%S') | ||
|
||
if self.use_api: | ||
|
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.
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.
fixed in 135c652
if target == "analysis_events": | ||
data = data[0] | ||
elif target == "remote": | ||
data = data[1] | ||
elif target == "wazuhdb": | ||
data = data[2] |
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.
Hardcoded elements. We should change the monitored process through the API
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.
fixed in 135c652
parser.add_argument('-u', '--use_state_file', action='store_true', default=False, | ||
help="Determine if the state files should be used to collect the for analysisd and remoted." | ||
"Use with remoted and analysis to get data from state files. Default False.") | ||
parser.add_argument('-i', '--ip', dest='ip', action='store', default='localhost', | ||
help=f"IP for the API. Default localhost.") | ||
parser.add_argument('-p', '--port', dest='port', action='store', default='55000', | ||
help=f"port for the API. Default localhost.") |
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.
parser.add_argument('-u', '--use_state_file', action='store_true', default=False, | |
help="Determine if the state files should be used to collect the for analysisd and remoted." | |
"Use with remoted and analysis to get data from state files. Default False.") | |
parser.add_argument('-i', '--ip', dest='ip', action='store', default='localhost', | |
help=f"IP for the API. Default localhost.") | |
parser.add_argument('-p', '--port', dest='port', action='store', default='55000', | |
help=f"port for the API. Default localhost.") | |
parser.add_argument('-u', '--use_state_file', action='store_true', default=False, | |
help="Use state files for analysis and remote operations." | |
"When used with 'remote' and 'analysis', data will be collected from state files; otherwise, the API will be used. Default False.") | |
parser.add_argument('-i', '--ip', dest='ip', action='store', default='localhost', | |
help=f"Specify the IP address for the API. Default is 'localhost'.") | |
parser.add_argument('-p', '--port', dest='port', action='store', default='55000', | |
help=f"Specify the port for the API. Default is '55000'.") |
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.
Fixed in a9192a5
else: | ||
raise ValueError(f'The target {self.target} is not a valid one.') | ||
|
||
state_file = splitext(basename(self.statistics_file))[0] | ||
self.csv_file = join(self.dst_dir, f'{state_file}_stats.csv') | ||
if self.use_state_file == True: |
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.
if self.use_state_file == True: | |
if self.use_state_file: |
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.
Fixed in a1d0448
if target == "analysis": | ||
csv_header = headers.analysisd_header if self.use_state_file == True else headers.analysisd_events_header | ||
elif target == "logcollector": | ||
csv_header = headers.logcollector_header | ||
elif target == "remote": | ||
csv_header = headers.remoted_header if self.use_state_file == True else headers.remoted_api_header | ||
elif target == "wazuhdb": | ||
csv_header = headers.wazuhdb_header | ||
else: | ||
csv_header = headers.agentd_header |
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.
if target == "analysis": | |
csv_header = headers.analysisd_header if self.use_state_file == True else headers.analysisd_events_header | |
elif target == "logcollector": | |
csv_header = headers.logcollector_header | |
elif target == "remote": | |
csv_header = headers.remoted_header if self.use_state_file == True else headers.remoted_api_header | |
elif target == "wazuhdb": | |
csv_header = headers.wazuhdb_header | |
else: | |
csv_header = headers.agentd_header | |
if target == "analysis": | |
csv_header = headers.analysisd_header if self.use_state_file else headers.analysisd_events_header | |
elif target == "logcollector": | |
csv_header = headers.logcollector_header | |
elif target == "remote": | |
csv_header = headers.remoted_header if self.use_state_file else headers.remoted_api_header | |
elif target == "wazuhdb": | |
csv_header = headers.wazuhdb_header | |
else: | |
csv_header = headers.agentd_header |
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.
Fixed in b48cd5b
Description
This PR enhances the StatisticMonitor class to enable using the API for analysisd events and remoted stats.
use_api
added called with-a
and--use_api
. It is used in withremoted
to get remote data from the API.analysis_events
are recovered by default from the API.Testing performed