alibaba / Sentinel

A powerful flow control component enabling reliability, resilience and monitoring for microservices. (面向云原生微服务的高可用流控防护组件)
https://sentinelguard.io/
Apache License 2.0
22.47k stars 8.04k forks source link

sentinel-dubbo-adapter 1.8.2 consumer端 SentinelRpcException 非业务异常也被统计 #2464

Open tianzh98 opened 3 years ago

tianzh98 commented 3 years ago

Issue Description

Type: bug report or feature request

Describe what happened (or what feature you want)

现象: 使用1.8.2版本的sentinel-dubbo-adapter模块,非业务异常(Sentinel因为熔断或者限流的异常)被消费者SentinelDubboConsumerFilter记录成为了一次业务异常,逻辑错误。 原因: 在sentinel-dubbo-adapter模块利用了Dubbo的Filter机制,包裹了一层。先看SentinelDubboProviderFilter的逻辑,我们设想提供者端触发了限流,则会抛出BlockException,但是返回给消费者的异常并不是BlockException,而是包裹成为了SentinelRpcException。 image image

而在SentinelDubboConsumerFilter端,result.hasException() 则会为true,触发Tracer.traceEntry() 逻辑,这里面的shouldTrace()方法只判断了排除BlockException,而SentinelRpcException未被排除,即会被记录为一次业务异常。会造成consumer端熔断判断的失误。 image

Describe what you expected to happen

在Tracer.traceEntry()逻辑中,也要去掉SentinelRpcException,遇到这个异常 则不记录,和BlockException同等对待。 解决方案: 1.可自定义一个init接口,Sentinel启动初始化时调用Tracer.setExceptionsToIgnore() 把SentinelRpcException加进去 2.因为DubboFilter类在dubbo启动refer或者export时会被加载,可在filter的静态块调用Tracer.setExceptionsToIgnore()把SentinelRpcException加进去

How to reproduce it (as minimally and precisely as possible)

  1. 设置2个dubbo服务,A,B。A调用B。
  2. 给A设置异常数熔断规则,给B设置限流规则。保证业务无异常。
  3. 让B触发限流
  4. A会被熔断

Tell us your environment

Win10 ,Dubbo2.6.5,Sentinel1.8.2,Nacos2.0.3

Anything else we need to know?

why wrap this exception? image

sczyh30 commented 3 years ago

Hi, this should not be wrapped with SentinelRpcException anymore. Would you like to contribute a PR to fix it?

tianzh98 commented 3 years ago

Hi, this should not be wrapped with SentinelRpcException anymore. Would you like to contribute a PR to fix it?

Yes, I will fix it later and submit a PR.

tianzh98 commented 3 years ago

Hi @sczyh30 ,I think we may still need to wrap the BlockException ,because BlockException doesn't implement the Java.io.Serializable,and it isn't a RuntimeException. It will cause a Dubbo serialization exception or wrapped with the UndeclaredThrowableException if we use the AOP. Can we modify the method com.alibaba.csp.sentinel.Tracer#shouldTrace , excluding the SentinelRpcException or set the ignoreClasses? Or any other suggestions? I'm trying to find the best solution to fix it .

sczyh30 commented 3 years ago

Hi @sczyh30 ,I think we may still need to wrap the BlockException ,because BlockException doesn't implement the Java.io.Serializable,and it isn't a RuntimeException. It will cause a Dubbo serialization exception or wrapped with the UndeclaredThrowableException if we use the AOP. Can we modify the method com.alibaba.csp.sentinel.Tracer#shouldTrace , excluding the SentinelRpcException or set the ignoreClasses? Or any other suggestions? I'm trying to find the best solution to fix it .

The BlockException has been converted to a RuntimeException by toRuntimeException().

liufeiyu1002 commented 2 years ago

我认为他应该被统计进去, 因为你是在服务端被限流了,客户端已经发出了请求 在客户端的规则下并没有被 block,如果发生了异常 都应该算做是 服务端 不可用(不管服务端是因为什么异常) ,如果多个消费端 请求打满了服务端 抛出 blockException,客户端 收到SentinelRpcException异常信息 应该进行熔断保护, 而不是 把它当做 blockException 忽略掉,让流量持续的请求到服务端