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

[Benchmark] Add mechanism to retreive the logs from test-execution.json #4686

Merged
merged 3 commits into from
May 9, 2024

Conversation

Divyaasm
Copy link
Collaborator

@Divyaasm Divyaasm commented May 6, 2024

Description

Get the logs from the test_executions folder and log for user's reference. Eventually the converted test_execution.csv file is archived to the respective jenkins build.

Issues Resolved

#4634

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Divya Madala <[email protected]>
with open(results) as file:
data = json.load(file)
formatted_data = pd.json_normalize(data["results"]["op_metrics"])
formatted_data.to_csv(os.path.join(os.getcwd(), "test_execution.csv"), index=False)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename the csv file to test_execution_<suffix>.json

@@ -53,4 +56,7 @@ def run_tests(self) -> None:
with WorkingDirectory(current_workspace):
with BenchmarkCreateCluster.create(self.args, self.test_manifest, config, current_workspace) as test_cluster:
benchmark_test_suite = BenchmarkTestSuite(test_cluster.endpoint_with_port, self.security, self.args, test_cluster.fetch_password())
retry_call(benchmark_test_suite.execute, tries=3, delay=60, backoff=2)
try:
benchmark_test_suite.execute()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why remove retry_call?

Copy link

codecov bot commented May 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.29%. Comparing base (46f773e) to head (b64621a).
Report is 58 commits behind head on main.

❗ Current head b64621a differs from pull request most recent head 41ae02a. Consider uploading reports for the commit 41ae02a to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4686      +/-   ##
==========================================
- Coverage   92.40%   92.29%   -0.11%     
==========================================
  Files         193      193              
  Lines        6317     6411      +94     
==========================================
+ Hits         5837     5917      +80     
- Misses        480      494      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Divya Madala <[email protected]>
try:
retry_call(benchmark_test_suite.execute, tries=3, delay=60, backoff=2)
finally:
subprocess.check_call(f"docker rm docker-container-{self.args.stack_suffix}", cwd=os.getcwd(), shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move the docker rm command to BenchmarkTestSuite class to a method like remove_container or whatever you like and then use that?

log_info = f"Executing {self.command.replace(self.endpoint, len(self.endpoint) * '*').replace(self.args.username, len(self.args.username) * '*')}"
logging.info(log_info.replace(self.password, len(self.password) * '*') if self.password else log_info)
subprocess.check_call(f"{self.command}", cwd=os.getcwd(), shell=True)
with TemporaryDirectory() as work_dir:
Copy link
Collaborator

@rishabh6788 rishabh6788 May 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move this file processing logic a new method and call that method only in the case endpoint is provided. This way the existing nightly runs will not be affected and we can enable it whenever we are ready.

Signed-off-by: Divya Madala <[email protected]>
@rishabh6788 rishabh6788 merged commit 95f99ea into opensearch-project:main May 9, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BENCHMARK] Add mechanism to retreive the logs from test-execution.json after running benchmark tests
2 participants