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.

Sign up for GitHub

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

Jump to bottom

Add ice transport api to get selected pair stats #2923

Merged
merged 2 commits into from
Oct 7, 2024
Merged

Add ice transport api to get selected pair stats #2923

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5a44fc7
Add ice transport api to get selected pair stats
boks1971 Oct 7, 2024
70504e4
Merge branch 'master' into raja_selected_candidate_pair_stats
boks1971 Oct 7, 2024
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
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ go 1.20
require (
github.com/pion/datachannel v1.5.9
github.com/pion/dtls/v3 v3.0.3
github.com/pion/ice/v4 v4.0.1
github.com/pion/ice/v4 v4.0.2
github.com/pion/interceptor v0.1.37
github.com/pion/logging v0.2.2
github.com/pion/randutil v0.1.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ github.com/pion/datachannel v1.5.9 h1:LpIWAOYPyDrXtU+BW7X0Yt/vGtYxtXQ8ql7dFfYUVZ
github.com/pion/datachannel v1.5.9/go.mod h1:kDUuk4CU4Uxp82NH4LQZbISULkX/HtzKa4P7ldf9izE=
github.com/pion/dtls/v3 v3.0.3 h1:j5ajZbQwff7Z8k3pE3S+rQ4STvKvXUdKsi/07ka+OWM=
github.com/pion/dtls/v3 v3.0.3/go.mod h1:weOTUyIV4z0bQaVzKe8kpaP17+us3yAuiQsEAG1STMU=
github.com/pion/ice/v4 v4.0.1 h1:2d3tPoTR90F3TcGYeXUwucGlXI3hds96cwv4kjZmb9s=
github.com/pion/ice/v4 v4.0.1/go.mod h1:2dpakjpd7+74L5j3TAe6gvkbI5UIzOgAnkimm9SuHvA=
github.com/pion/ice/v4 v4.0.2 h1:1JhBRX8iQLi0+TfcavTjPjI6GO41MFn4CeTBX+Y9h5s=
github.com/pion/ice/v4 v4.0.2/go.mod h1:DCdqyzgtsDNYN6/3U8044j3U7qsJ9KFJC92VnOWHvXg=
github.com/pion/interceptor v0.1.37 h1:aRA8Zpab/wE7/c0O3fh1PqY0AJI3fCSEM5lRWJVorwI=
github.com/pion/interceptor v0.1.37/go.mod h1:JzxbJ4umVTlZAf+/utHzNesY8tmRkM2lVmkS82TTj8Y=
github.com/pion/logging v0.2.2 h1:M9+AIj/+pxNsDfAT64+MAVgJO0rsyLnoJKCqf//DoeY=
Expand Down
58 changes: 22 additions & 36 deletions icegatherer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,46 +308,12 @@
for _, candidatePairStats := range agent.GetCandidatePairsStats() {
collector.Collecting()

state, err := toStatsICECandidatePairState(candidatePairStats.State)
stats, err := toICECandidatePairStats(candidatePairStats)
if err != nil {
g.log.Error(err.Error())
continue

Check warning on line 314 in icegatherer.go

View check run for this annotation

Codecov / codecov/patch

icegatherer.go#L314 

Added line #L314 was not covered by tests
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A slight change in behaviour here. Consolidate the conversion from ice.CandidatePairStats -> webrtc.ICECandidatePairStats in stats.go so that it can be use both here and in getSelectedCandidatePairStats() 👇 . In fact, linter flagged it for repeated code 🎉 .

Previously, code continued to add stats after just logging the error. Now, it does not. The consolidation code in stats.go returns an empty stats if there is an error here.

I think the state error should never happen. There is also a comment in code like

    default:                                                                                                                                                    
        // NOTE: this should never happen[tm]                                                                                                                   
        err := fmt.Errorf("%w: %s", errStatsICECandidateStateInvalid, state.String())                                                                           
        return StatsICECandidatePairState("Unknown"), err

So, just skipping adding the stat on error.

}

pairID := newICECandidatePairStatsID(candidatePairStats.LocalCandidateID,
candidatePairStats.RemoteCandidateID)

stats := ICECandidatePairStats{
Timestamp: statsTimestampFrom(candidatePairStats.Timestamp),
Type: StatsTypeCandidatePair,
ID: pairID,
// TransportID:
LocalCandidateID: candidatePairStats.LocalCandidateID,
RemoteCandidateID: candidatePairStats.RemoteCandidateID,
State: state,
Nominated: candidatePairStats.Nominated,
PacketsSent: candidatePairStats.PacketsSent,
PacketsReceived: candidatePairStats.PacketsReceived,
BytesSent: candidatePairStats.BytesSent,
BytesReceived: candidatePairStats.BytesReceived,
LastPacketSentTimestamp: statsTimestampFrom(candidatePairStats.LastPacketSentTimestamp),
LastPacketReceivedTimestamp: statsTimestampFrom(candidatePairStats.LastPacketReceivedTimestamp),
FirstRequestTimestamp: statsTimestampFrom(candidatePairStats.FirstRequestTimestamp),
LastRequestTimestamp: statsTimestampFrom(candidatePairStats.LastRequestTimestamp),
LastResponseTimestamp: statsTimestampFrom(candidatePairStats.LastResponseTimestamp),
TotalRoundTripTime: candidatePairStats.TotalRoundTripTime,
CurrentRoundTripTime: candidatePairStats.CurrentRoundTripTime,
AvailableOutgoingBitrate: candidatePairStats.AvailableOutgoingBitrate,
AvailableIncomingBitrate: candidatePairStats.AvailableIncomingBitrate,
CircuitBreakerTriggerCount: candidatePairStats.CircuitBreakerTriggerCount,
RequestsReceived: candidatePairStats.RequestsReceived,
RequestsSent: candidatePairStats.RequestsSent,
ResponsesReceived: candidatePairStats.ResponsesReceived,
ResponsesSent: candidatePairStats.ResponsesSent,
RetransmissionsReceived: candidatePairStats.RetransmissionsReceived,
RetransmissionsSent: candidatePairStats.RetransmissionsSent,
ConsentRequestsSent: candidatePairStats.ConsentRequestsSent,
ConsentExpiredTimestamp: statsTimestampFrom(candidatePairStats.ConsentExpiredTimestamp),
}
collector.Collect(stats.ID, stats)
}

Expand Down Expand Up @@ -409,3 +375,23 @@
collector.Done()
}(collector, agent)
}

func (g *ICEGatherer) getSelectedCandidatePairStats() (ICECandidatePairStats, bool) {
agent := g.getAgent()
if agent == nil {
return ICECandidatePairStats{}, false
}

selectedCandidatePairStats, isAvailable := agent.GetSelectedCandidatePairStats()
if !isAvailable {
return ICECandidatePairStats{}, false

Check warning on line 387 in icegatherer.go

View check run for this annotation

Codecov / codecov/patch

icegatherer.go#L387 

Added line #L387 was not covered by tests
}

stats, err := toICECandidatePairStats(selectedCandidatePairStats)
if err != nil {
g.log.Error(err.Error())
return ICECandidatePairStats{}, false

Check warning on line 393 in icegatherer.go

View check run for this annotation

Codecov / codecov/patch

icegatherer.go#L392-L393 

Added lines #L392 - L393 were not covered by tests
}

return stats, true
}
6 changes: 6 additions & 0 deletions icetransport.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@ func (t *ICETransport) GetSelectedCandidatePair() (*ICECandidatePair, error) {
return NewICECandidatePair(&local, &remote), nil
}

// GetSelectedCandidatePairStats returns the selected candidate pair stats on which packets are sent
// if there is no selected pair empty stats, false is returned to indicate stats not available
func (t *ICETransport) GetSelectedCandidatePairStats() (ICECandidatePairStats, bool) {
return t.gatherer.getSelectedCandidatePairStats()
}

// NewICETransport creates a new NewICETransport.
func NewICETransport(gatherer *ICEGatherer, loggerFactory logging.LoggerFactory) *ICETransport {
iceTransport := &ICETransport{
Expand Down
8 changes: 8 additions & 0 deletions icetransport_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,21 +98,29 @@ func TestICETransport_GetSelectedCandidatePair(t *testing.T) {
offererSelectedPair, err := offerer.SCTP().Transport().ICETransport().GetSelectedCandidatePair()
assert.NoError(t, err)
assert.Nil(t, offererSelectedPair)
_, statsAvailable := offerer.SCTP().Transport().ICETransport().GetSelectedCandidatePairStats()
assert.False(t, statsAvailable)

answererSelectedPair, err := answerer.SCTP().Transport().ICETransport().GetSelectedCandidatePair()
assert.NoError(t, err)
assert.Nil(t, answererSelectedPair)
_, statsAvailable = answerer.SCTP().Transport().ICETransport().GetSelectedCandidatePairStats()
assert.False(t, statsAvailable)

assert.NoError(t, signalPair(offerer, answerer))
peerConnectionConnected.Wait()

offererSelectedPair, err = offerer.SCTP().Transport().ICETransport().GetSelectedCandidatePair()
assert.NoError(t, err)
assert.NotNil(t, offererSelectedPair)
_, statsAvailable = offerer.SCTP().Transport().ICETransport().GetSelectedCandidatePairStats()
assert.True(t, statsAvailable)

answererSelectedPair, err = answerer.SCTP().Transport().ICETransport().GetSelectedCandidatePair()
assert.NoError(t, err)
assert.NotNil(t, answererSelectedPair)
_, statsAvailable = answerer.SCTP().Transport().ICETransport().GetSelectedCandidatePairStats()
assert.True(t, statsAvailable)

closePairNow(t, offerer, answerer)
}
Expand Down
40 changes: 40 additions & 0 deletions stats.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1965,6 +1965,46 @@
}
}

func toICECandidatePairStats(candidatePairStats ice.CandidatePairStats) (ICECandidatePairStats, error) {
state, err := toStatsICECandidatePairState(candidatePairStats.State)
if err != nil {
return ICECandidatePairStats{}, err

Check warning on line 1971 in stats.go

View check run for this annotation

Codecov / codecov/patch

stats.go#L1971 

Added line #L1971 was not covered by tests
}

return ICECandidatePairStats{
Timestamp: statsTimestampFrom(candidatePairStats.Timestamp),
Type: StatsTypeCandidatePair,
ID: newICECandidatePairStatsID(candidatePairStats.LocalCandidateID, candidatePairStats.RemoteCandidateID),
// TransportID:
LocalCandidateID: candidatePairStats.LocalCandidateID,
RemoteCandidateID: candidatePairStats.RemoteCandidateID,
State: state,
Nominated: candidatePairStats.Nominated,
PacketsSent: candidatePairStats.PacketsSent,
PacketsReceived: candidatePairStats.PacketsReceived,
BytesSent: candidatePairStats.BytesSent,
BytesReceived: candidatePairStats.BytesReceived,
LastPacketSentTimestamp: statsTimestampFrom(candidatePairStats.LastPacketSentTimestamp),
LastPacketReceivedTimestamp: statsTimestampFrom(candidatePairStats.LastPacketReceivedTimestamp),
FirstRequestTimestamp: statsTimestampFrom(candidatePairStats.FirstRequestTimestamp),
LastRequestTimestamp: statsTimestampFrom(candidatePairStats.LastRequestTimestamp),
LastResponseTimestamp: statsTimestampFrom(candidatePairStats.LastResponseTimestamp),
TotalRoundTripTime: candidatePairStats.TotalRoundTripTime,
CurrentRoundTripTime: candidatePairStats.CurrentRoundTripTime,
AvailableOutgoingBitrate: candidatePairStats.AvailableOutgoingBitrate,
AvailableIncomingBitrate: candidatePairStats.AvailableIncomingBitrate,
CircuitBreakerTriggerCount: candidatePairStats.CircuitBreakerTriggerCount,
RequestsReceived: candidatePairStats.RequestsReceived,
RequestsSent: candidatePairStats.RequestsSent,
ResponsesReceived: candidatePairStats.ResponsesReceived,
ResponsesSent: candidatePairStats.ResponsesSent,
RetransmissionsReceived: candidatePairStats.RetransmissionsReceived,
RetransmissionsSent: candidatePairStats.RetransmissionsSent,
ConsentRequestsSent: candidatePairStats.ConsentRequestsSent,
ConsentExpiredTimestamp: statsTimestampFrom(candidatePairStats.ConsentExpiredTimestamp),
}, nil
}

const (
// StatsICECandidatePairStateFrozen means a check for this pair hasn't been
// performed, and it can't yet be performed until some other check succeeds,
Expand Down
Loading