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

Log error to console while executing test #409

Merged
merged 1 commit into from
Nov 27, 2023

Conversation

cgchinmay
Copy link
Collaborator

@cgchinmay cgchinmay commented Nov 8, 2023

Description

During execute-test command, often there are errors reported in logs but user understands them only for test completion. With this change as soon as an error for a given operation is encountered, it will be printed on console

Issues Resolved

#349

Testing

  • New functionality includes testing

[Describe how this change was tested]
Before change, there were errors in logs, but its displayed only for test run as below

|                                       100th percentile latency |            desc-sort-timestamp-after-force-merge-1-seg |     407.499 |     ms |
|                                  100th percentile service time |            desc-sort-timestamp-after-force-merge-1-seg |     407.499 |     ms |
|                                                     error rate |            desc-sort-timestamp-after-force-merge-1-seg |         100 |      % |
|                                       100th percentile latency |             asc-sort-timestamp-after-force-merge-1-seg |     407.876 |     ms |
|                                  100th percentile service time |             asc-sort-timestamp-after-force-merge-1-seg |     407.876 |     ms |
|                                                     error rate |             asc-sort-timestamp-after-force-merge-1-seg |         100 |      % |
|                                       100th percentile latency | desc-sort-with-after-timestamp-after-force-merge-1-seg |     409.831 |     ms |
|                                  100th percentile service time | desc-sort-with-after-timestamp-after-force-merge-1-seg |     409.831 |     ms |
|                                                     error rate | desc-sort-with-after-timestamp-after-force-merge-1-seg |         100 |      % |
|                                       100th percentile latency |  asc-sort-with-after-timestamp-after-force-merge-1-seg |     404.191 |     ms |
|                                  100th percentile service time |  asc-sort-with-after-timestamp-after-force-merge-1-seg |     404.191 |     ms |
|                                                     error rate |  asc-sort-with-after-timestamp-after-force-merge-1-seg |         100 |      % |

[WARNING] Error rate is 100.0 for operation 'delete-index'. Please check the logs.
[WARNING] No throughput metrics available for [delete-index]. Likely cause: Error rate is 100.0%. Please check the logs.
[WARNING] Error rate is 100.0 for operation 'create-index'. Please check the logs.
[WARNING] No throughput metrics available for [create-index]. Likely cause: Error rate is 100.0%. Please check the logs.

after change, running same query results in below output

python3 benchmark.py execute-test --target-hosts=https://search-demo-whkwj7g7a2cpehen52sjzrmmri.us-east-1.es.amazonaws.com --pipeline=benchmark-only --workload=http_logs --client-options=timeout:120,amazon_aws_log_in:environment --test-mode --kill-running-processes

[ERROR] permission denied for index-create. check logs for details
Running create-index                                                           [100% done]
Running check-cluster-health                                                   [100% done]
Running index-append                                                           [  0% done][ERROR] permission denied for index-append. check logs for details
[ERROR] permission denied for index-append. check logs for details
[ERROR] permission denied for index-append. check logs for details
[ERROR] permission denied for index-append. check logs for details
[ERROR] permission denied for index-append. check logs for details

Detailed error is sent to logs

2023-11-27 04:30:37,946 ActorAddr-(T|:60534)/PID:20179 osbenchmark.worker_coordinator.worker_coordinator ERROR {"error":{"root_cause":[{"type":"security_exception","reason":"no permissions for [indices:data/write/bulk] and User [name=arn:aws:iam::558706479279:role/opensearch-admin, backend_roles=[arn:aws:iam::558706479279:role/opensearch-admin], requestedTenant=null]"}],"type":"security_exception","reason":"no permissions for [indices:data/write/bulk] and User [name=arn:aws:iam::558706479279:role/opensearch-admin, backend_roles=[arn:aws:iam::558706479279:role/opensearch-admin], requestedTenant=null]"},"status":403}

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.

Copy link
Collaborator

@IanHoang IanHoang left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for this. Users will now be able to view errors faster without having to wait till the end of the test for the summary report or dig through the logs.

@cgchinmay What do you think of the following: In terms of readability, I'm wondering if there's a neater way we can incorporate a class that registers common exceptions, such as the "403 create-index" and replaces the error message with a concise message that's less verbose. For all other uncommon errors, we can just have it print out the error that OSB receives.

@cgchinmay
Copy link
Collaborator Author

Nice! Thanks for this. Users will now be able to view errors faster without having to wait till the end of the test for the summary report or dig through the logs.

@cgchinmay What do you think of the following: In terms of readability, I'm wondering if there's a neater way we can incorporate a class that registers common exceptions, such as the "403 create-index" and replaces the error message with a concise message that's less verbose. For all other uncommon errors, we can just have it print out the error that OSB receives.

thats good suggestion. let me take a look into it and send an update

@cgchinmay cgchinmay force-pushed the log_error_console branch 2 times, most recently from 0fd91cc to 79f2824 Compare November 27, 2023 04:06
Signed-off-by: Chinmay Gadgil <[email protected]>

added erros file for common opensearch errors during execute-test

Signed-off-by: Chinmay Gadgil <[email protected]>

make errors less verbose

Signed-off-by: Chinmay Gadgil <[email protected]>

added detailed error in logs

Signed-off-by: Chinmay Gadgil <[email protected]>
Copy link
Collaborator

@IanHoang IanHoang left a comment

Choose a reason for hiding this comment

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

Nice! These changes lay a good foundation for future error handling

@IanHoang IanHoang merged commit b445c6b into opensearch-project:main Nov 27, 2023
8 checks passed
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.

2 participants