Skip to content

Original exception never gets logged in Retry Topic flow #2212

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

Closed
tomazfernandes opened this issue Apr 5, 2022 · 2 comments · Fixed by #2213
Closed

Original exception never gets logged in Retry Topic flow #2212

tomazfernandes opened this issue Apr 5, 2022 · 2 comments · Fixed by #2213

Comments

@tomazfernandes
Copy link
Contributor

This SO user pointed out that, when the record goes through the Retry Topic flow, the original exception never gets logged.

We had already discussed this a bit earlier this year.

@garyrussell proposed:

But I see your point; since we always recover these failures, they are never logged. Maybe the DLPR needs an option to log the exception after successfully recovering a record (but that doesn't need to be part of this fix).

I suggested as an alternative:

We could log the error message just before we either send the record to the DLT or give up processing - that way nothing gets logged if any of the retrials work, and gets logged only once if none.

The main difference seems to be whether we want to log the exception at every retry attempt or only before it goes to the DLT.

We might look into adding both and making these configurable - users might be able to set a flag in DLPR to log after every attempt, and DLPRF to log before going to the DLT.

We could also have this as an enum in DLPRF, with something like OriginalExceptionLoggingStrategy.NEVER, EVERY_ATTEMPT, AFTER_RETRIES_EXHAUSTED - DLPRF would then set the appropriate flag in DLPR.

WDYT?

Originally posted by @tomazfernandes in #2211

@tomazfernandes
Copy link
Contributor Author

Looks like Reference this discussion in new issue is not the same as converting discussion into issue as you do 😄

Guess it's alright though - let me know if otherwise. I should open a PR for this shortly if that's ok.

Thanks.

@garyrussell
Copy link
Contributor

It doesn't really make a difference; this is fine.

@garyrussell garyrussell added this to the 3.0.0-M4 milestone Apr 5, 2022
tomazfernandes added a commit to tomazfernandes/spring-kafka that referenced this issue Apr 5, 2022
Resolves spring-projects#2212

As in Retry Topic flow recovery is successful, the exception thrown by the listener was never being logged.

Now it logs at DEBUG level for intermediate retries and ERROR level when retries are exhausted.

Also some polishing in TimestampedException as it was cluttering the stacktrace with duplicated information from cause.getMessage(). Now it logs the time when the exception occurred.
tomazfernandes added a commit to tomazfernandes/spring-kafka that referenced this issue Apr 5, 2022
Resolves spring-projects#2212

As in Retry Topic flow recovery is successful, the exception thrown by the listener was never being logged.

Now it logs at DEBUG level for intermediate retries and ERROR level when retries are exhausted.

Also some polishing in TimestampedException as it was cluttering the stacktrace with duplicated information from cause.getMessage(). Now it logs the time when the exception occurred.
tomazfernandes added a commit to tomazfernandes/spring-kafka that referenced this issue Apr 6, 2022
Resolves spring-projects#2212

As in Retry Topic flow recovery is successful, the exception thrown by the listener was never being logged.

Now it logs at DEBUG level for intermediate retries and ERROR level when retries are exhausted.

Also some polishing in TimestampedException as it was cluttering the stacktrace with duplicated information from cause.getMessage(). Now it logs the time when the exception occurred.
tomazfernandes added a commit to tomazfernandes/spring-kafka that referenced this issue Apr 6, 2022
Resolves spring-projects#2212

As in Retry Topic flow recovery is successful, the exception thrown by the listener was never being logged.

Now it logs at DEBUG level for intermediate retries and ERROR level when retries are exhausted.

Also some polishing in TimestampedException as it was cluttering the stacktrace with duplicated information from cause.getMessage(). Now it logs the time when the exception occurred.
garyrussell pushed a commit that referenced this issue Apr 11, 2022
* GH-2212: Log listener ex. in retry topic flow

Resolves #2212

As in Retry Topic flow recovery is successful, the exception thrown by the listener was never being logged.

Now it logs at DEBUG level for intermediate retries and ERROR level when retries are exhausted.

Also some polishing in TimestampedException as it was cluttering the stacktrace with duplicated information from cause.getMessage(). Now it logs the time when the exception occurred.

* Address TimestampedException review suggestions

Add ListenerExceptionLoggingStrategy

Add unit tests

* Address review comments
garyrussell pushed a commit that referenced this issue Apr 11, 2022
* GH-2212: Log listener ex. in retry topic flow

Resolves #2212

As in Retry Topic flow recovery is successful, the exception thrown by the listener was never being logged.

Now it logs at DEBUG level for intermediate retries and ERROR level when retries are exhausted.

Also some polishing in TimestampedException as it was cluttering the stacktrace with duplicated information from cause.getMessage(). Now it logs the time when the exception occurred.

* Address TimestampedException review suggestions

Add ListenerExceptionLoggingStrategy

Add unit tests

* Address review comments
garyrussell pushed a commit that referenced this issue Apr 11, 2022
* GH-2212: Log listener ex. in retry topic flow

Resolves #2212

As in Retry Topic flow recovery is successful, the exception thrown by the listener was never being logged.

Now it logs at DEBUG level for intermediate retries and ERROR level when retries are exhausted.

Also some polishing in TimestampedException as it was cluttering the stacktrace with duplicated information from cause.getMessage(). Now it logs the time when the exception occurred.

* Address TimestampedException review suggestions

Add ListenerExceptionLoggingStrategy

Add unit tests

* Address review comments
garyrussell pushed a commit that referenced this issue Apr 11, 2022
* GH-2212: Log listener ex. in retry topic flow

Resolves #2212

As in Retry Topic flow recovery is successful, the exception thrown by the listener was never being logged.

Now it logs at DEBUG level for intermediate retries and ERROR level when retries are exhausted.

Also some polishing in TimestampedException as it was cluttering the stacktrace with duplicated information from cause.getMessage(). Now it logs the time when the exception occurred.

* Address TimestampedException review suggestions

Add ListenerExceptionLoggingStrategy

Add unit tests

* Address review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants