Strange "Uncaught exception thrown" when including Zipkin starter #966
Comments
nickcodefresh
changed the title
|
You're not getting Zipkin related issues, you're getting Sleuth related issues. You'd have to disable Sleuth. What's going on is that we're wrapping an output stream in a tracing representation. When The issue you mentioned has nothing to do with this problem. Don't you get these exceptions thrown when you don't have Zipkin starter on the classpath? Can you share a sample? |
|
So I discovered this is related to change in Spring Cloud whereby the default Ribbon timeout has changed from 5s to 1s. Our service hits a laggy legacy API that can be slow, so numerous calls would timeout. When I added to application.yml, the timeouts went, as did these exceptions. |
|
maybe we shouldn't log the exception as the usual thing is to do span.error() instead? possibly less confusion. wdyt? |
`ExceptionLoggingFilter` logs "Uncaught exception thrown" to error level when there is a synchronous exception not otherwise swallowed. This is a cure worse than the disease. This issue disables by default and the 3.x should end the years of problems it caused. Fixes #1347 This was done IIUC to help @jfuerth who had log messages with trace IDs, except the uncaught one, in #714 (Sept 2017). They were formerly told to use `ExceptionHandler` and noted that didn't work in practice as it subverted their status code logic. Concretely, 4203566 "solved" the issue by having sleuth log all uncaught exceptions with the message "Uncaught exception thrown". This code was initially embedded in sleuth's and later pulled out to `ExceptionLoggingFilter` in 2.0. A few months later @brenuart noticed this "solution" had problems. For example, it caused the same exception to be logged twice. Bertrand also mentioned some faults in the implementation including edge cases like `ClientAbortException` not addressed by this. (In fact, there are even more problems: the implementation assumes async isn't in use!) Bertands concerns about this having surprising logging effects were dismissed as a non-issue. Bertrand raised #859 about glitches this caused, the status_code related ones being if statemented around, and it went quiet a while. A month later @oburgosm opened #902 (currently 5 thumbs up) asking to remove the "Uncaught exception thrown" or at least set it to debug. The response was if they don't like it, control their own tracing filter and the issue was closed. A month later, @nickcodefresh opened #966 confused about "Uncaught exception thrown" messages raised by sleuth, incidentally the `ClientAbortException` mentioned by @brenuart before. Because the logging came from Sleuth, it appeared they were zipkin errors, and was explained they weren't. A year later, @T3rm1 opened #1347 to remove the filter, or at least disable it by default. The response was first to set the Logger to OFF. A few days later the issue was closed as the submitter was told they were the first person to complain about this. 5 months later, another user rallied for support. In April 2020, #902 was re-touched by @kosciej who asked to please reconsider, mentioning it was not typical even in Spring to do this, for example 'org.springframework.web.filter' package. The impact of needing to specifically change apps to OFF the sleuth logger seemed harsh. The suggestion was instead of setting log config, to set a property instead `spring.sleuth.web.exception-logging-filter-enabled`, to `false` as that could also accomplish the goal of stopping "Uncaught exception thrown". A month later (yesterday) @MrHurniak commented again on #1347 with the usual complaint that it is redundant and confusing.
If I include this dependency in my Spring Boot 1.5.9.RELEASE / Spring Cloud Edgware.SR3 project:
I get lots of Zipkin-related errors when calling my controllers, despite not having Zipkin enabled:
I found this issue but I don't know if it's related (but it's supposed to be fixed in the version I'm running anyway).
The text was updated successfully, but these errors were encountered: