Skip to content
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

Add Req/Res count/time to candidate stats #763

Merged
merged 1 commit into from
Feb 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -980,18 +980,23 @@
return nil
}

func (a *Agent) sendBindingRequest(m *stun.Message, local, remote Candidate) {
func (a *Agent) sendBindingRequest(msg *stun.Message, local, remote Candidate) {
a.log.Tracef("Ping STUN from %s to %s", local, remote)

a.invalidatePendingBindingRequests(time.Now())
a.pendingBindingRequests = append(a.pendingBindingRequests, bindingRequest{
timestamp: time.Now(),
transactionID: m.TransactionID,
transactionID: msg.TransactionID,
destination: remote.addr(),
isUseCandidate: m.Contains(stun.AttrUseCandidate),
isUseCandidate: msg.Contains(stun.AttrUseCandidate),
})

a.sendSTUN(m, local, remote)
if pair := a.findPair(local, remote); pair != nil {
pair.UpdateRequestSent()
} else {
a.log.Warnf("Failed to find pair for add binding request from %s to %s", local, remote)
}

Check warning on line 998 in agent.go

View check run for this annotation

Codecov / codecov/patch

agent.go#L997-L998

Added lines #L997 - L998 were not covered by tests
a.sendSTUN(msg, local, remote)
}

func (a *Agent) sendBindingSuccess(m *stun.Message, local, remote Candidate) {
Expand All @@ -1014,6 +1019,11 @@
); err != nil {
a.log.Warnf("Failed to handle inbound ICE from: %s to: %s error: %s", local, remote, err)
} else {
if pair := a.findPair(local, remote); pair != nil {
pair.UpdateResponseSent()
} else {
a.log.Warnf("Failed to find pair for add binding response from %s to %s", local, remote)
}
a.sendSTUN(out, local, remote)
}
}
Expand Down
16 changes: 10 additions & 6 deletions agent_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,22 @@ func (a *Agent) GetCandidatePairsStats() []CandidatePairStats {
// BytesReceived uint64
// LastPacketSentTimestamp time.Time
// LastPacketReceivedTimestamp time.Time
// FirstRequestTimestamp time.Time
// LastRequestTimestamp time.Time
// LastResponseTimestamp time.Time
FirstRequestTimestamp: cp.FirstRequestSentAt(),
LastRequestTimestamp: cp.LastRequestSentAt(),
FirstResponseTimestamp: cp.FirstReponseReceivedAt(),
LastResponseTimestamp: cp.LastResponseReceivedAt(),
FirstRequestReceivedTimestamp: cp.FirstRequestReceivedAt(),
LastRequestReceivedTimestamp: cp.LastRequestReceivedAt(),

TotalRoundTripTime: cp.TotalRoundTripTime(),
CurrentRoundTripTime: cp.CurrentRoundTripTime(),
// AvailableOutgoingBitrate float64
// AvailableIncomingBitrate float64
// CircuitBreakerTriggerCount uint32
// RequestsReceived uint64
// RequestsSent uint64
RequestsReceived: cp.RequestsReceived(),
RequestsSent: cp.RequestsSent(),
ResponsesReceived: cp.ResponsesReceived(),
// ResponsesSent uint64
ResponsesSent: cp.ResponsesSent(),
// RetransmissionsReceived uint64
// RetransmissionsSent uint64
// ConsentRequestsSent uint64
Expand Down
17 changes: 14 additions & 3 deletions agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,7 @@ func TestInvalidGather(t *testing.T) {
})
}

func TestCandidatePairsStats(t *testing.T) { //nolint:cyclop
func TestCandidatePairsStats(t *testing.T) { //nolint:cyclop,gocyclo
defer test.CheckRoutines(t)()

// Avoid deadlocks?
Expand Down Expand Up @@ -715,14 +715,18 @@ func TestCandidatePairsStats(t *testing.T) { //nolint:cyclop
p := agent.findPair(hostLocal, remote)

if p == nil {
agent.addPair(hostLocal, remote)
p = agent.addPair(hostLocal, remote)
}
p.UpdateRequestReceived()
p.UpdateRequestSent()
p.UpdateResponseSent()
p.UpdateRoundTripTime(time.Second)
}

p := agent.findPair(hostLocal, prflxRemote)
p.state = CandidatePairStateFailed

for i := 0; i < 10; i++ {
for i := 1; i < 10; i++ {
p.UpdateRoundTripTime(time.Duration(i+1) * time.Second)
}

Expand All @@ -749,6 +753,13 @@ func TestCandidatePairsStats(t *testing.T) { //nolint:cyclop
default:
t.Fatal("invalid remote candidate ID")
}

if cps.FirstRequestTimestamp.IsZero() || cps.LastRequestTimestamp.IsZero() ||
cps.FirstResponseTimestamp.IsZero() || cps.LastResponseTimestamp.IsZero() ||
cps.FirstRequestReceivedTimestamp.IsZero() || cps.LastRequestReceivedTimestamp.IsZero() ||
cps.RequestsReceived == 0 || cps.RequestsSent == 0 || cps.ResponsesSent == 0 || cps.ResponsesReceived == 0 {
t.Fatal("failed to verify pair stats counter and timestamps", cps)
}
}

if relayPairStat.RemoteCandidateID != relayRemote.ID() {
Expand Down
110 changes: 109 additions & 1 deletion candidatepair.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,18 @@
// stats
currentRoundTripTime int64 // in ns
totalRoundTripTime int64 // in ns
responsesReceived uint64

requestsReceived uint64
requestsSent uint64
responsesReceived uint64
responsesSent uint64

firstRequestSentAt atomic.Value // time.Time
lastRequestSentAt atomic.Value // time.Time
firstReponseReceivedAt atomic.Value // time.Time
lastResponseReceivedAt atomic.Value // time.Time
firstRequestReceivedAt atomic.Value // time.Time
lastRequestReceivedAt atomic.Value // time.Time
}

func (p *CandidatePair) String() string {
Expand Down Expand Up @@ -127,6 +138,10 @@
atomic.StoreInt64(&p.currentRoundTripTime, rttNs)
atomic.AddInt64(&p.totalRoundTripTime, rttNs)
atomic.AddUint64(&p.responsesReceived, 1)

now := time.Now()
p.firstReponseReceivedAt.CompareAndSwap(nil, now)
p.lastResponseReceivedAt.Store(now)
}

// CurrentRoundTripTime returns the current round trip time in seconds
Expand All @@ -141,8 +156,101 @@
return time.Duration(atomic.LoadInt64(&p.totalRoundTripTime)).Seconds()
}

// RequestsReceived returns the total number of connectivity checks received
// https://www.w3.org/TR/webrtc-stats/#dom-rtcicecandidatepairstats-requestsreceived
func (p *CandidatePair) RequestsReceived() uint64 {
return atomic.LoadUint64(&p.requestsReceived)
}

// RequestsSent returns the total number of connectivity checks sent
// https://www.w3.org/TR/webrtc-stats/#dom-rtcicecandidatepairstats-requestssent
func (p *CandidatePair) RequestsSent() uint64 {
return atomic.LoadUint64(&p.requestsSent)
}

// ResponsesReceived returns the total number of connectivity responses received
// https://www.w3.org/TR/webrtc-stats/#dom-rtcicecandidatepairstats-responsesreceived
func (p *CandidatePair) ResponsesReceived() uint64 {
return atomic.LoadUint64(&p.responsesReceived)
}

// ResponsesSent returns the total number of connectivity responses sent
// https://www.w3.org/TR/webrtc-stats/#dom-rtcicecandidatepairstats-responsessent
func (p *CandidatePair) ResponsesSent() uint64 {
return atomic.LoadUint64(&p.responsesSent)
}

// FirstRequestSentAt returns the timestamp of the first connectivity check sent.
func (p *CandidatePair) FirstRequestSentAt() time.Time {
if v, ok := p.firstRequestSentAt.Load().(time.Time); ok {
return v
}

return time.Time{}

Check warning on line 189 in candidatepair.go

View check run for this annotation

Codecov / codecov/patch

candidatepair.go#L189

Added line #L189 was not covered by tests
}

// LastRequestSentAt returns the timestamp of the last connectivity check sent.
func (p *CandidatePair) LastRequestSentAt() time.Time {
if v, ok := p.lastRequestSentAt.Load().(time.Time); ok {
return v
}

return time.Time{}

Check warning on line 198 in candidatepair.go

View check run for this annotation

Codecov / codecov/patch

candidatepair.go#L198

Added line #L198 was not covered by tests
}

// FirstReponseReceivedAt returns the timestamp of the first connectivity response received.
func (p *CandidatePair) FirstReponseReceivedAt() time.Time {
if v, ok := p.firstReponseReceivedAt.Load().(time.Time); ok {
return v
}

return time.Time{}

Check warning on line 207 in candidatepair.go

View check run for this annotation

Codecov / codecov/patch

candidatepair.go#L207

Added line #L207 was not covered by tests
}

// LastResponseReceivedAt returns the timestamp of the last connectivity response received.
func (p *CandidatePair) LastResponseReceivedAt() time.Time {
if v, ok := p.lastResponseReceivedAt.Load().(time.Time); ok {
return v
}

return time.Time{}

Check warning on line 216 in candidatepair.go

View check run for this annotation

Codecov / codecov/patch

candidatepair.go#L216

Added line #L216 was not covered by tests
}

// FirstRequestReceivedAt returns the timestamp of the first connectivity check received.
func (p *CandidatePair) FirstRequestReceivedAt() time.Time {
if v, ok := p.firstRequestReceivedAt.Load().(time.Time); ok {
return v
}

return time.Time{}

Check warning on line 225 in candidatepair.go

View check run for this annotation

Codecov / codecov/patch

candidatepair.go#L225

Added line #L225 was not covered by tests
}

// LastRequestReceivedAt returns the timestamp of the last connectivity check received.
func (p *CandidatePair) LastRequestReceivedAt() time.Time {
if v, ok := p.lastRequestReceivedAt.Load().(time.Time); ok {
return v
}

return time.Time{}

Check warning on line 234 in candidatepair.go

View check run for this annotation

Codecov / codecov/patch

candidatepair.go#L234

Added line #L234 was not covered by tests
}

// UpdateRequestSent increments the number of requests sent and updates the timestamp.
func (p *CandidatePair) UpdateRequestSent() {
atomic.AddUint64(&p.requestsSent, 1)
now := time.Now()
p.firstRequestSentAt.CompareAndSwap(nil, now)
p.lastRequestSentAt.Store(now)
}

// UpdateResponseSent increments the number of responses sent.
func (p *CandidatePair) UpdateResponseSent() {
atomic.AddUint64(&p.responsesSent, 1)
Copy link
Contributor

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?

Copy link
Member Author

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).

Copy link
Contributor

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

}

// UpdateRequestReceived increments the number of requests received and updates the timestamp.
func (p *CandidatePair) UpdateRequestReceived() {
atomic.AddUint64(&p.requestsReceived, 1)
now := time.Now()
p.firstRequestReceivedAt.CompareAndSwap(nil, now)
p.lastRequestReceivedAt.Store(now)
}
5 changes: 4 additions & 1 deletion selection.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,12 @@ func (s *controllingSelector) HandleBindingRequest(message *stun.Message, local,
pair := s.agent.findPair(local, remote)

if pair == nil {
s.agent.addPair(local, remote)
pair = s.agent.addPair(local, remote)
pair.UpdateRequestReceived()

return
}
pair.UpdateRequestReceived()

if pair.state == CandidatePairStateSucceeded && s.nominatedPair == nil && s.agent.getSelectedPair() == nil {
bestPair := s.agent.getBestAvailableCandidatePair()
Expand Down Expand Up @@ -281,6 +283,7 @@ func (s *controlledSelector) HandleBindingRequest(message *stun.Message, local,
if pair == nil {
pair = s.agent.addPair(local, remote)
}
pair.UpdateRequestReceived()

if message.Contains(stun.AttrUseCandidate) { //nolint:nestif
// https://tools.ietf.org/html/rfc8445#section-7.3.1.5
Expand Down
12 changes: 12 additions & 0 deletions stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,22 @@ type CandidatePairStats struct {
// (LastRequestTimestamp - FirstRequestTimestamp) / RequestsSent.
LastRequestTimestamp time.Time

// FirstResponseTimestamp represents the timestamp at which the first STUN response
// was received on this particular candidate pair.
FirstResponseTimestamp time.Time

// LastResponseTimestamp represents the timestamp at which the last STUN response
// was received on this particular candidate pair.
LastResponseTimestamp time.Time

// FirstRequestReceivedTimestamp represents the timestamp at which the first
// connectivity check request was received.
FirstRequestReceivedTimestamp time.Time

// LastRequestReceivedTimestamp represents the timestamp at which the last
// connectivity check request was received.
LastRequestReceivedTimestamp time.Time

// TotalRoundTripTime represents the sum of all round trip time measurements
// in seconds since the beginning of the session, based on STUN connectivity
// check responses (ResponsesReceived), including those that reply to requests
Expand Down
Loading