Repository: pion/ice
Pull request #380 (2021-07-19 22:55:30)

Populate RelayProtocol for relay candidates
#### Description

Candidates have a field `RelayProtocol` on their stat as defined [here](https://www.w3.org/TR/webrtc-stats/#dom-rtcicecandidatestats-relayprotocol):
> It is the protocol used by the endpoint to communicate with the TURN
> server. This is only present for local candidates. Valid values are
> "udp", "tcp", or "tls".

Currently, the field is populated with "udp" for all local and remote candidates of all types. Instead, we should leave it blank for all but local relay candidates in which case we should populate it with the protocol used to talk to the TURN server.


Sean-Der: Great PR! Thank you @meelapshah 
codecov[bot]: # [Codecov](https://codecov.io/gh/pion/ice/pull/380?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) Report
> Merging [#380](https://codecov.io/gh/pion/ice/pull/380?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (7a83c13) into [master](https://codecov.io/gh/pion/ice/commit/2e7ada3136132c1a3e30aa6d6e50a3af11045045?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) (2e7ada3) will **decrease** coverage by `53.06%`.
> The diff coverage is `10.00%`.

[![Impacted file tree graph](https://codecov.io/gh/pion/ice/pull/380/graphs/tree.svg?width=650&height=150&src=pr&token=1ivIyHDzva&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)](https://codecov.io/gh/pion/ice/pull/380?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)

```diff
@@             Coverage Diff             @@
##           master     #380       +/-   ##
===========================================
- Coverage   78.62%   25.55%   -53.07%     
===========================================
  Files          33       33               
  Lines        2582     2598       +16     
===========================================
- Hits         2030      664     -1366     
- Misses        386     1906     +1520     
+ Partials      166       28      -138     
```

| Flag | Coverage Δ | |
|---|---|---|
| go | `?` | |
| wasm | `25.55% <10.00%> (-0.12%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/pion/ice/pull/380?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | Coverage Δ | |
|---|---|---|
| [agent\_stats.go](https://codecov.io/gh/pion/ice/pull/380/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-YWdlbnRfc3RhdHMuZ28=) | `0.00% <0.00%> (-82.36%)` | :arrow_down: |
| [gather.go](https://codecov.io/gh/pion/ice/pull/380/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-Z2F0aGVyLmdv) | `0.00% <0.00%> (-67.33%)` | :arrow_down: |
| [candidate\_relay.go](https://codecov.io/gh/pion/ice/pull/380/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-Y2FuZGlkYXRlX3JlbGF5Lmdv) | `69.69% <66.66%> (-17.40%)` | :arrow_down: |
| [candidate\_base.go](https://codecov.io/gh/pion/ice/pull/380/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-Y2FuZGlkYXRlX2Jhc2UuZ28=) | `55.17% <100.00%> (-26.11%)` | :arrow_down: |
| [candidate\_peer\_reflexive.go](https://codecov.io/gh/pion/ice/pull/380/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-Y2FuZGlkYXRlX3BlZXJfcmVmbGV4aXZlLmdv) | `0.00% <0.00%> (-84.62%)` | :arrow_down: |
| [agent\_config.go](https://codecov.io/gh/pion/ice/pull/380/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-YWdlbnRfY29uZmlnLmdv) | `0.00% <0.00%> (-84.38%)` | :arrow_down: |
| [transport.go](https://codecov.io/gh/pion/ice/pull/380/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-dHJhbnNwb3J0Lmdv) | `0.00% <0.00%> (-82.70%)` | :arrow_down: |
| [agent.go](https://codecov.io/gh/pion/ice/pull/380/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-YWdlbnQuZ28=) | `0.00% <0.00%> (-81.85%)` | :arrow_down: |
| [selection.go](https://codecov.io/gh/pion/ice/pull/380/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-c2VsZWN0aW9uLmdv) | `0.00% <0.00%> (-79.11%)` | :arrow_down: |
| [udp\_mux.go](https://codecov.io/gh/pion/ice/pull/380/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion#diff-dWRwX211eC5nbw==) | `0.00% <0.00%> (-78.69%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/pion/ice/pull/380/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pion/ice/pull/380?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pion/ice/pull/380?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Last update [2e7ada3...7a83c13](https://codecov.io/gh/pion/ice/pull/380?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pion).


Patch:
diff --git a/AUTHORS.txt b/AUTHORS.txt
index 6a1ddbb..68666a4 100644
--- a/AUTHORS.txt
+++ b/AUTHORS.txt
@@ -28,6 +28,7 @@ korymiller1489 <kmiller@unwiredrevolution.com>
 Kyle Carberry <kyle@carberry.com>
 Lander Noterman <lander.noterman@basalte.be>
 Luke Curley <kixelated@gmail.com>
+Meelap Shah <meelapshah@gmail.com>
 Michael MacDonald <github@macdonald.cx>
 Michael MacDonald <mike.macdonald@savantsystems.com>
 Nevio Vesic <nevio@subspace.com>
diff --git a/agent_stats.go b/agent_stats.go
index 88c39f7..b249560 100644
--- a/agent_stats.go
+++ b/agent_stats.go
@@ -58,6 +58,10 @@ func (a *Agent) GetLocalCandidatesStats() []CandidateStats {
 		result := make([]CandidateStats, 0, len(agent.localCandidates))
 		for networkType, localCandidates := range agent.localCandidates {
 			for _, c := range localCandidates {
+				relayProtocol := ""
+				if c.Type() == CandidateTypeRelay {
+					relayProtocol = c.(*CandidateRelay).RelayProtocol()
+				}
 				stat := CandidateStats{
 					Timestamp:     time.Now(),
 					ID:            c.ID(),
@@ -67,7 +71,7 @@ func (a *Agent) GetLocalCandidatesStats() []CandidateStats {
 					CandidateType: c.Type(),
 					Priority:      c.Priority(),
 					// URL string
-					RelayProtocol: "udp",
+					RelayProtocol: relayProtocol,
 					// Deleted bool
 				}
 				result = append(result, stat)
@@ -98,7 +102,7 @@ func (a *Agent) GetRemoteCandidatesStats() []CandidateStats {
 					CandidateType: c.Type(),
 					Priority:      c.Priority(),
 					// URL string
-					RelayProtocol: "udp",
+					RelayProtocol: "",
 				}
 				result = append(result, stat)
 			}
diff --git a/candidate_base.go b/candidate_base.go
index c4f6f1b..63231e6 100644
--- a/candidate_base.go
+++ b/candidate_base.go
@@ -492,7 +492,7 @@ func UnmarshalCandidate(raw string) (Candidate, error) {
 	case "prflx":
 		return NewCandidatePeerReflexive(&CandidatePeerReflexiveConfig{"", protocol, address, port, component, priority, foundation, relatedAddress, relatedPort})
 	case "relay":
-		return NewCandidateRelay(&CandidateRelayConfig{"", protocol, address, port, component, priority, foundation, relatedAddress, relatedPort, nil})
+		return NewCandidateRelay(&CandidateRelayConfig{"", protocol, address, port, component, priority, foundation, relatedAddress, relatedPort, "", nil})
 	default:
 	}
 
diff --git a/candidate_relay.go b/candidate_relay.go
index 44762f7..3d0be31 100644
--- a/candidate_relay.go
+++ b/candidate_relay.go
@@ -8,21 +8,23 @@ import (
 type CandidateRelay struct {
 	candidateBase
 
-	onClose func() error
+	relayProtocol string
+	onClose       func() error
 }
 
 // CandidateRelayConfig is the config required to create a new CandidateRelay
 type CandidateRelayConfig struct {
-	CandidateID string
-	Network     string
-	Address     string
-	Port        int
-	Component   uint16
-	Priority    uint32
-	Foundation  string
-	RelAddr     string
-	RelPort     int
-	OnClose     func() error
+	CandidateID   string
+	Network       string
+	Address       string
+	Port          int
+	Component     uint16
+	Priority      uint32
+	Foundation    string
+	RelAddr       string
+	RelPort       int
+	RelayProtocol string
+	OnClose       func() error
 }
 
 // NewCandidateRelay creates a new relay candidate
@@ -59,10 +61,16 @@ func NewCandidateRelay(config *CandidateRelayConfig) (*CandidateRelay, error) {
 				Port:    config.RelPort,
 			},
 		},
-		onClose: config.OnClose,
+		relayProtocol: config.RelayProtocol,
+		onClose:       config.OnClose,
 	}, nil
 }
 
+// RelayProtocol returns the protocol used between the endpoint and the relay server.
+func (c *CandidateRelay) RelayProtocol() string {
+	return c.relayProtocol
+}
+
 func (c *CandidateRelay) close() error {
 	err := c.candidateBase.close()
 	if c.onClose != nil {
diff --git a/gather.go b/gather.go
index e6ea827..6a6588a 100644
--- a/gather.go
+++ b/gather.go
@@ -422,10 +422,11 @@ func (a *Agent) gatherCandidatesRelay(ctx context.Context, urls []*URL) { //noli
 			defer wg.Done()
 			TURNServerAddr := fmt.Sprintf("%s:%d", url.Host, url.Port)
 			var (
-				locConn net.PacketConn
-				err     error
-				RelAddr string
-				RelPort int
+				locConn       net.PacketConn
+				err           error
+				RelAddr       string
+				RelPort       int
+				relayProtocol string
 			)
 
 			switch {
@@ -437,6 +438,7 @@ func (a *Agent) gatherCandidatesRelay(ctx context.Context, urls []*URL) { //noli
 
 				RelAddr = locConn.LocalAddr().(*net.UDPAddr).IP.String()
 				RelPort = locConn.LocalAddr().(*net.UDPAddr).Port
+				relayProtocol = udp
 			case a.proxyDialer != nil && url.Proto == ProtoTypeTCP &&
 				(url.Scheme == SchemeTypeTURN || url.Scheme == SchemeTypeTURNS):
 				conn, connectErr := a.proxyDialer.Dial(NetworkTypeTCP4.String(), TURNServerAddr)
@@ -447,6 +449,11 @@ func (a *Agent) gatherCandidatesRelay(ctx context.Context, urls []*URL) { //noli
 
 				RelAddr = conn.LocalAddr().(*net.TCPAddr).IP.String()
 				RelPort = conn.LocalAddr().(*net.TCPAddr).Port
+				if url.Scheme == SchemeTypeTURN {
+					relayProtocol = tcp
+				} else if url.Scheme == SchemeTypeTURNS {
+					relayProtocol = "tls"
+				}
 				locConn = turn.NewSTUNConn(conn)
 
 			case url.Proto == ProtoTypeTCP && url.Scheme == SchemeTypeTURN:
@@ -464,6 +471,7 @@ func (a *Agent) gatherCandidatesRelay(ctx context.Context, urls []*URL) { //noli
 
 				RelAddr = conn.LocalAddr().(*net.TCPAddr).IP.String()
 				RelPort = conn.LocalAddr().(*net.TCPAddr).Port
+				relayProtocol = tcp
 				locConn = turn.NewSTUNConn(conn)
 			case url.Proto == ProtoTypeUDP && url.Scheme == SchemeTypeTURNS:
 				udpAddr, connectErr := net.ResolveUDPAddr(network, TURNServerAddr)
@@ -483,6 +491,7 @@ func (a *Agent) gatherCandidatesRelay(ctx context.Context, urls []*URL) { //noli
 
 				RelAddr = conn.LocalAddr().(*net.UDPAddr).IP.String()
 				RelPort = conn.LocalAddr().(*net.UDPAddr).Port
+				relayProtocol = "dtls"
 				locConn = &fakePacketConn{conn}
 			case url.Proto == ProtoTypeTCP && url.Scheme == SchemeTypeTURNS:
 				conn, connectErr := tls.Dial(NetworkTypeTCP4.String(), TURNServerAddr, &tls.Config{
@@ -494,6 +503,7 @@ func (a *Agent) gatherCandidatesRelay(ctx context.Context, urls []*URL) { //noli
 				}
 				RelAddr = conn.LocalAddr().(*net.TCPAddr).IP.String()
 				RelPort = conn.LocalAddr().(*net.TCPAddr).Port
+				relayProtocol = "tls"
 				locConn = turn.NewSTUNConn(conn)
 			default:
 				a.log.Warnf("Unable to handle URL in gatherCandidatesRelay %v\n", url)
@@ -528,12 +538,13 @@ func (a *Agent) gatherCandidatesRelay(ctx context.Context, urls []*URL) { //noli
 
 			raddr := relayConn.LocalAddr().(*net.UDPAddr)
 			relayConfig := CandidateRelayConfig{
-				Network:   network,
-				Component: ComponentRTP,
-				Address:   raddr.IP.String(),
-				Port:      raddr.Port,
-				RelAddr:   RelAddr,
-				RelPort:   RelPort,
+				Network:       network,
+				Component:     ComponentRTP,
+				Address:       raddr.IP.String(),
+				Port:          raddr.Port,
+				RelAddr:       RelAddr,
+				RelPort:       RelPort,
+				RelayProtocol: relayProtocol,
 				OnClose: func() error {
 					client.Close()
 					return locConn.Close()


Test patch:
diff --git a/candidate_test.go b/candidate_test.go
index ed4e159..23745b0 100644
--- a/candidate_test.go
+++ b/candidate_test.go
@@ -262,6 +262,7 @@ func TestCandidateMarshal(t *testing.T) {
 					port:           5000,
 					relatedAddress: &CandidateRelatedAddress{"192.168.0.1", 5001},
 				},
+				"",
 				nil,
 			},
 			"848194626 1 udp 16777215 50.0.0.1 5000 typ relay raddr 192.168.0.1 rport 5001",