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

Change logging in ConsulFailoverInterceptor to WARN level #305

Closed
sleberknight opened this issue Dec 17, 2023 · 2 comments · Fixed by #309
Closed

Change logging in ConsulFailoverInterceptor to WARN level #305

sleberknight opened this issue Dec 17, 2023 · 2 comments · Fixed by #309
Assignees
Labels
enhancement A request for change or improvement to an existing feature
Milestone

Comments

@sleberknight
Copy link
Member

To help debugging, change the DEBUG level log message in ConsulFailoverInterceptor#intercept to WARN level, but without the exception and stack trace. Those can be kept at DEBUG level in a separate log message.

@sleberknight sleberknight added the enhancement A request for change or improvement to an existing feature label Dec 17, 2023
@sleberknight sleberknight added this to the 1.2.0 milestone Dec 17, 2023
@sleberknight sleberknight self-assigned this Dec 17, 2023
@sleberknight sleberknight added the in progress A task that is actively being worked on label Dec 17, 2023
@sleberknight
Copy link
Member Author

Instead of having separate log messages at DEBUG and WARN levels, it will be better to do something like:

if (LOG.isDebugEnabled()) {
    LOG.debug(...);
} else {
    LOG.warn(...);
}

@sleberknight
Copy link
Member Author

Also, the warning level log message should include the exception type, in addition to the destination URL.

sleberknight added a commit that referenced this issue Dec 17, 2023
* If debug level is enabled, log the exception (and stack trace) at
  debug level. Otherwise, log the exception at warning level including
  the exception type, but not the stack trace.
* Internal refactoring to remove the duplicate calls to get() the
  value from the Optional<Request>. When inside the loop, we know
  that the Optional contains a value, so it's annoying to see
  repetitive calls to nextRequest.get().

Closes #305
sleberknight added a commit that referenced this issue Dec 17, 2023
* If debug level is enabled, log the exception (and stack trace) at
  debug level. Otherwise, log the exception at warning level including
  the exception type, but not the stack trace. Include a message
  that the stack trace can be seen by enabling debug level
  for the Logger.
* Internal refactoring to remove the duplicate calls to get() the
  value from the Optional<Request>. When inside the loop, we know
  that the Optional contains a value, so it's annoying to see
  repetitive calls to nextRequest.get().
* Rename the nextRequest Optional to maybeNextRequest
  to make it more obvious that it isn't a real Request.
* Replace JetBrains NotNull annotation with Checker's NonNull.
  The reason is that the JetBrains annotations come in transitively
  whereas Checker is a direct dependency. While this is not
  huge deal, we also have (more or less) standardized on
  using the Checker annotations for nullability.
* Change the Optional#get call to orElseThrow.
  The reason is that, since the while loop's Optional#isPresent test
  was true, the Optional#get call must succeed inside the loop body.
  Otherwise, something else (i.e., a coding error) is wrong.

Closes #305
@sleberknight sleberknight removed the in progress A task that is actively being worked on label Dec 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A request for change or improvement to an existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant