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

Add operation identifier to retry logs. #1112

Merged
merged 4 commits into from
Apr 28, 2023
Merged

Add operation identifier to retry logs. #1112

merged 4 commits into from
Apr 28, 2023

Conversation

vbabanin
Copy link
Member

This PR adds logging of the OperationContext object to the CommandOperationHelper.logRetryExecute() method. The OperationContext object provides context information about the operation being executed, such as the operation id.

Logging the OperationContext object can be useful for troubleshooting, as it allows us to track the execution of specific operations.

JAVA-4939

@jyemin jyemin removed the request for review from katcharov April 28, 2023 11:44
@stIncMale
Copy link
Member

@vbabanin Is the PR a draft because you had been waiting for the tests to pass?

? format("Retrying the operation due to the error \"%s\"; attempt #%d", exception, oneBasedAttempt)
: format("Retrying the operation '%s' due to the error \"%s\"; attempt #%d",
commandDescription, exception, oneBasedAttempt));
? format("Retrying the operation due to the error \"%s\"; attempt #%d; operation ID %s", exception, oneBasedAttempt, operationId)
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make the message a bit more similar in structure/style to what's in https://github.com/mongodb/specifications/blob/master/source/command-logging-and-monitoring/command-logging-and-monitoring.rst#command-started-message. We typically try to use complete sentences, for instance, rather than weird punctuation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, i have made the necessary changes to align with the recommended style.

@vbabanin
Copy link
Member Author

@vbabanin Is the PR a draft because you had been waiting for the tests to pass?
Yes, the PR is a draft because I was waiting for the results of some manual testing that I was finishing up

@vbabanin vbabanin requested a review from jyemin April 28, 2023 19:42
@vbabanin vbabanin marked this pull request as ready for review April 28, 2023 19:42
Copy link
Contributor

@jyemin jyemin left a comment

Choose a reason for hiding this comment

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

LGTM!

@vbabanin vbabanin merged commit 0efc78c into mongodb:master Apr 28, 2023
41 checks passed
@vbabanin vbabanin deleted the JAVA-4939 branch April 28, 2023 23:25
ispringer pushed a commit to evergage/mongo-java-driver that referenced this pull request May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants