feat: dynamic flow control p3: add FlowControllerEventStats#1332
feat: dynamic flow control p3: add FlowControllerEventStats#1332igorbernstein2 merged 14 commits intogoogleapis:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1332 +/- ##
============================================
+ Coverage 81.31% 81.45% +0.14%
- Complexity 1338 1347 +9
============================================
Files 210 211 +1
Lines 5690 5728 +38
Branches 521 526 +5
============================================
+ Hits 4627 4666 +39
+ Misses 852 850 -2
- Partials 211 212 +1
Continue to review full report at Codecov.
|
igorbernstein2
left a comment
There was a problem hiding this comment.
lgtm with a nit.
@vam-google wanna take a pass?
vam-google
left a comment
There was a problem hiding this comment.
LGTM with a bunch of minor comments.
| return new AutoValue_FlowControlEventStats_FlowControlEvent(timestampMs, null, exception); | ||
| } | ||
|
|
||
| public abstract long getTimestampMs(); |
There was a problem hiding this comment.
Subjective, but it looks like this class does not need to be AutoValue. It is way less trivial than most of the AutoValue classes, which are usually just a set of setters/getters (not the case here). Please consider just storing the throttledtimeInMs, exception and timestampMs values manually in this class.
There was a problem hiding this comment.
AutoValue doesnt actually hurt anything here. I generally prefer AutoValue here to communicate the fact that this is a simple value type
| return new AutoValue_FlowControlEventStats_FlowControlEvent(timestampMs, null, exception); | ||
| } | ||
|
|
||
| public abstract long getTimestampMs(); |
|
Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot. |
76fdf5d to
74eae2b
Compare
Implementation of go/veneer-dynamic-flow-control part 3, adding a class to record flow control events