-
Notifications
You must be signed in to change notification settings - Fork 182
Add Req/Res count/time to candidate stats #763
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #763 +/- ##
==========================================
+ Coverage 78.66% 78.77% +0.10%
==========================================
Files 41 41
Lines 5367 5437 +70
==========================================
+ Hits 4222 4283 +61
- Misses 915 923 +8
- Partials 230 231 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
a63eb7f to
c645c14
Compare
These details will provide information for connectivity issue.
c645c14 to
d1e43a7
Compare
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.
nice!!!
|
|
||
| // UpdateResponseSent increments the number of responses sent. | ||
| func (p *CandidatePair) UpdateResponseSent() { | ||
| atomic.AddUint64(&p.responsesSent, 1) |
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.
Is there any value to adding first/last response sent time?
With this PR, we have
- first/last time of request sent
- first/last time of request received
- first/last time of response received
wondering if there is more to learn by recording first/last time of response sent?
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.
I had it but deleted at last since the agent always send response when request is received, so the timpstamp is useless (the responseSent has same issue too but it is in the specification https://www.w3.org/TR/webrtc-stats/#dom-rtcicecandidatepairstats-responsessent so I keep it).
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.
got it, thank you @cnderrauber
These details will provide information for understanding
connectivity issue.