-
Notifications
You must be signed in to change notification settings - Fork 26.5k
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
RequestEvent optimization #12655
RequestEvent optimization #12655
Conversation
Codecov Report
@@ Coverage Diff @@
## 3.2 #12655 +/- ##
============================================
+ Coverage 69.36% 69.39% +0.03%
Complexity 2 2
============================================
Files 1647 1646 -1
Lines 68198 68198
Branches 9966 9967 +1
============================================
+ Hits 47304 47327 +23
+ Misses 16325 16307 -18
+ Partials 4569 4564 -5 see 34 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Please resolve conflicts |
# Conflicts: # dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/filter/support/MetricsClusterFilter.java # dubbo-metrics/dubbo-metrics-default/src/main/java/org/apache/dubbo/metrics/event/DefaultSubDispatcher.java # dubbo-metrics/dubbo-metrics-default/src/main/java/org/apache/dubbo/metrics/event/RequestBeforeEvent.java # dubbo-metrics/dubbo-metrics-default/src/main/java/org/apache/dubbo/metrics/event/RequestEvent.java # dubbo-metrics/dubbo-metrics-default/src/test/java/org/apache/dubbo/metrics/collector/AggregateMetricsCollectorTest.java # dubbo-metrics/dubbo-metrics-default/src/test/java/org/apache/dubbo/metrics/collector/DefaultCollectorTest.java
Well, it's resolved |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to rename requestBeforeEvent
to requestErrorEvent
I don't think so because this is applied to MetricsClusterFilter to monitor exceptions that occur before executing requests |
The focus of this event is that the request was aborted before execution, |
However there are only one event in the case |
…nto metircs_requestEvent_optimize
Kudos, SonarCloud Quality Gate passed! |
Oh, sure! I've made the update |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@songxiaosheng PTAL
What is the purpose of the change
issue
Brief changelog
Verifying this change
Checklist