spring-cloud / spring-cloud-openfeign

Support for using OpenFeign in Spring Cloud apps
Apache License 2.0
1.17k stars 755 forks source link

[BUG] MicrometerObservationCapability not reporting timeouts #958

Open sigand opened 6 months ago

sigand commented 6 months ago

Describe the bug Feign cloud version: 4.1.0

When using the MicrometerObservationCapability, anyjava.net.SocketTimeoutException is not caught and observations is never closed (MicrometerObservationCapability#enrich only catches FeignExceptions). Probably affects other errors from the underlying web layer too. The result is that these requests are never added to the metrics. There is either an ordering error, or MicrometerObservationCapability should catch more types of errors

Sample No specific config added. Plain FeignClients

@FeignClient(name = "NAME", url = "${url.prop}")
interface Client {
// calls here
}

@Configuration
class FeignConfig {
  @Bean
  def observation(observationRegistry: ObservationRegistry): MicrometerObservationCapability = new MicrometerObservationCapability(observationRegistry, new DefaultFeignObservationConvention())
}

application.yml

spring:
  cloud:
    openfeign:
      client:
        config:
          default:
            connect-timeout: 3000
            read-timeout: 3000

Stacktrace

Caused by: java.net.SocketTimeoutException: Read timed out
    at java.base/sun.nio.ch.NioSocketImpl.timedRead(NioSocketImpl.java:288)
    at java.base/sun.nio.ch.NioSocketImpl.implRead(NioSocketImpl.java:314)
    at java.base/sun.nio.ch.NioSocketImpl.read(NioSocketImpl.java:355)
    at java.base/sun.nio.ch.NioSocketImpl$1.read(NioSocketImpl.java:808)
    at java.base/java.net.Socket$SocketInputStream.read(Socket.java:966)
    at java.base/java.io.BufferedInputStream.fill(BufferedInputStream.java:244)
    at java.base/java.io.BufferedInputStream.read1(BufferedInputStream.java:284)
    at java.base/java.io.BufferedInputStream.read(BufferedInputStream.java:343)
    at java.base/sun.net.www.http.HttpClient.parseHTTPHeader(HttpClient.java:826)
    at java.base/sun.net.www.http.HttpClient.parseHTTP(HttpClient.java:761)
    at java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1688)
    at java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1589)
    at java.base/java.net.HttpURLConnection.getResponseCode(HttpURLConnection.java:529)
    at feign.Client$Default.convertResponse(Client.java:111)
    at feign.Client$Default.execute(Client.java:107)
    at feign.micrometer.MicrometerObservationCapability.lambda$enrich$1(MicrometerObservationCapability.java:53)
    at feign.SynchronousMethodHandler.executeAndDecode(SynchronousMethodHandler.java:100)
marcingrzejszczak commented 5 months ago

I think this should be moved to Feign. Spring Cloud OpenFeign simply configures the capability.