[tor-commits] [snowflake/master] Update broker stats to include info on NAT types

cohosh at torproject.org cohosh at torproject.org
Mon Aug 24 13:46:09 UTC 2020


commit 3c3317503eb8e83bbf5bebff411bbd722e60ee2f
Author: Cecylia Bocovich <cohosh at torproject.org>
Date:   Wed Aug 19 11:37:43 2020 -0400

    Update broker stats to include info on NAT types
    
    As we now partition proxies by NAT type, our stats are more useful if they
    capture how many proxies of each type we have, and information on
    whether we have enough proxies of the right NAT type for our clients.
    This change adds proxy counts by NAT type and binned counts of denied clients by NAT type.
---
 broker/broker.go                | 23 ++++++++++-
 broker/metrics.go               | 53 +++++++++++++++++++------
 broker/snowflake-broker_test.go | 88 +++++++++++++++++++++++++++++++++++++----
 broker/snowflake-heap.go        |  1 +
 doc/broker-spec.txt             | 34 +++++++++++++++-
 5 files changed, 177 insertions(+), 22 deletions(-)

diff --git a/broker/broker.go b/broker/broker.go
index 99f6c69..983f95d 100644
--- a/broker/broker.go
+++ b/broker/broker.go
@@ -165,6 +165,7 @@ func (ctx *BrokerContext) AddSnowflake(id string, proxyType string, natType stri
 	snowflake.id = id
 	snowflake.clients = 0
 	snowflake.proxyType = proxyType
+	snowflake.natType = natType
 	snowflake.offerChannel = make(chan *ClientOffer)
 	snowflake.answerChannel = make(chan []byte)
 	ctx.snowflakeLock.Lock()
@@ -201,7 +202,7 @@ func proxyPolls(ctx *BrokerContext, w http.ResponseWriter, r *http.Request) {
 		log.Println("Error processing proxy IP: ", err.Error())
 	} else {
 		ctx.metrics.lock.Lock()
-		ctx.metrics.UpdateCountryStats(remoteIP, proxyType)
+		ctx.metrics.UpdateCountryStats(remoteIP, proxyType, natType)
 		ctx.metrics.lock.Unlock()
 	}
 
@@ -275,6 +276,11 @@ func clientOffers(ctx *BrokerContext, w http.ResponseWriter, r *http.Request) {
 	if numSnowflakes <= 0 {
 		ctx.metrics.lock.Lock()
 		ctx.metrics.clientDeniedCount++
+		if offer.natType == NATUnrestricted {
+			ctx.metrics.clientUnrestrictedDeniedCount++
+		} else {
+			ctx.metrics.clientRestrictedDeniedCount++
+		}
 		ctx.metrics.lock.Unlock()
 		w.WriteHeader(http.StatusServiceUnavailable)
 		return
@@ -357,6 +363,7 @@ func proxyAnswers(ctx *BrokerContext, w http.ResponseWriter, r *http.Request) {
 func debugHandler(ctx *BrokerContext, w http.ResponseWriter, r *http.Request) {
 
 	var webexts, browsers, standalones, unknowns int
+	var natRestricted, natUnrestricted, natUnknown int
 	ctx.snowflakeLock.Lock()
 	s := fmt.Sprintf("current snowflakes available: %d\n", len(ctx.idToSnowflake))
 	for _, snowflake := range ctx.idToSnowflake {
@@ -370,12 +377,26 @@ func debugHandler(ctx *BrokerContext, w http.ResponseWriter, r *http.Request) {
 			unknowns++
 		}
 
+		switch snowflake.natType {
+		case NATRestricted:
+			natRestricted++
+		case NATUnrestricted:
+			natUnrestricted++
+		default:
+			natUnknown++
+		}
+
 	}
 	ctx.snowflakeLock.Unlock()
 	s += fmt.Sprintf("\tstandalone proxies: %d", standalones)
 	s += fmt.Sprintf("\n\tbrowser proxies: %d", browsers)
 	s += fmt.Sprintf("\n\twebext proxies: %d", webexts)
 	s += fmt.Sprintf("\n\tunknown proxies: %d", unknowns)
+
+	s += fmt.Sprintf("\nNAT Types available:")
+	s += fmt.Sprintf("\n\trestricted: %d", natRestricted)
+	s += fmt.Sprintf("\n\tunrestricted: %d", natUnrestricted)
+	s += fmt.Sprintf("\n\tunknown: %d", natUnknown)
 	if _, err := w.Write([]byte(s)); err != nil {
 		log.Printf("writing proxy information returned error: %v ", err)
 	}
diff --git a/broker/metrics.go b/broker/metrics.go
index ea4d220..d1beae2 100644
--- a/broker/metrics.go
+++ b/broker/metrics.go
@@ -25,7 +25,12 @@ type CountryStats struct {
 	badge      map[string]bool
 	webext     map[string]bool
 	unknown    map[string]bool
-	counts     map[string]int
+
+	natRestricted   map[string]bool
+	natUnrestricted map[string]bool
+	natUnknown      map[string]bool
+
+	counts map[string]int
 }
 
 // Implements Observable
@@ -34,11 +39,13 @@ type Metrics struct {
 	tablev4 *GeoIPv4Table
 	tablev6 *GeoIPv6Table
 
-	countryStats            CountryStats
-	clientRoundtripEstimate time.Duration
-	proxyIdleCount          uint
-	clientDeniedCount       uint
-	clientProxyMatchCount   uint
+	countryStats                  CountryStats
+	clientRoundtripEstimate       time.Duration
+	proxyIdleCount                uint
+	clientDeniedCount             uint
+	clientRestrictedDeniedCount   uint
+	clientUnrestrictedDeniedCount uint
+	clientProxyMatchCount         uint
 
 	//synchronization for access to snowflake metrics
 	lock sync.Mutex
@@ -58,7 +65,7 @@ func (s CountryStats) Display() string {
 	return output
 }
 
-func (m *Metrics) UpdateCountryStats(addr string, proxyType string) {
+func (m *Metrics) UpdateCountryStats(addr string, proxyType string, natType string) {
 
 	var country string
 	var ok bool
@@ -111,6 +118,15 @@ func (m *Metrics) UpdateCountryStats(addr string, proxyType string) {
 		m.countryStats.unknown[addr] = true
 	}
 
+	switch natType {
+	case NATRestricted:
+		m.countryStats.natRestricted[addr] = true
+	case NATUnrestricted:
+		m.countryStats.natUnrestricted[addr] = true
+	default:
+		m.countryStats.natUnknown[addr] = true
+	}
+
 }
 
 func (m *Metrics) LoadGeoipDatabases(geoipDB string, geoip6DB string) error {
@@ -139,11 +155,14 @@ func NewMetrics(metricsLogger *log.Logger) (*Metrics, error) {
 	m := new(Metrics)
 
 	m.countryStats = CountryStats{
-		counts:     make(map[string]int),
-		standalone: make(map[string]bool),
-		badge:      make(map[string]bool),
-		webext:     make(map[string]bool),
-		unknown:    make(map[string]bool),
+		counts:          make(map[string]int),
+		standalone:      make(map[string]bool),
+		badge:           make(map[string]bool),
+		webext:          make(map[string]bool),
+		unknown:         make(map[string]bool),
+		natRestricted:   make(map[string]bool),
+		natUnrestricted: make(map[string]bool),
+		natUnknown:      make(map[string]bool),
 	}
 
 	m.logger = metricsLogger
@@ -174,7 +193,12 @@ func (m *Metrics) printMetrics() {
 	m.logger.Println("snowflake-ips-webext", len(m.countryStats.webext))
 	m.logger.Println("snowflake-idle-count", binCount(m.proxyIdleCount))
 	m.logger.Println("client-denied-count", binCount(m.clientDeniedCount))
+	m.logger.Println("client-restricted-denied-count", binCount(m.clientRestrictedDeniedCount))
+	m.logger.Println("client-unrestricted-denied-count", binCount(m.clientUnrestrictedDeniedCount))
 	m.logger.Println("client-snowflake-match-count", binCount(m.clientProxyMatchCount))
+	m.logger.Println("snowflake-ips-nat-restricted", len(m.countryStats.natRestricted))
+	m.logger.Println("snowflake-ips-nat-unrestricted", len(m.countryStats.natUnrestricted))
+	m.logger.Println("snowflake-ips-nat-unknown", len(m.countryStats.natUnknown))
 	m.lock.Unlock()
 }
 
@@ -182,12 +206,17 @@ func (m *Metrics) printMetrics() {
 func (m *Metrics) zeroMetrics() {
 	m.proxyIdleCount = 0
 	m.clientDeniedCount = 0
+	m.clientRestrictedDeniedCount = 0
+	m.clientUnrestrictedDeniedCount = 0
 	m.clientProxyMatchCount = 0
 	m.countryStats.counts = make(map[string]int)
 	m.countryStats.standalone = make(map[string]bool)
 	m.countryStats.badge = make(map[string]bool)
 	m.countryStats.webext = make(map[string]bool)
 	m.countryStats.unknown = make(map[string]bool)
+	m.countryStats.natRestricted = make(map[string]bool)
+	m.countryStats.natUnrestricted = make(map[string]bool)
+	m.countryStats.natUnknown = make(map[string]bool)
 }
 
 // Rounds up a count to the nearest multiple of 8.
diff --git a/broker/snowflake-broker_test.go b/broker/snowflake-broker_test.go
index d03dca7..4e8e5f0 100644
--- a/broker/snowflake-broker_test.go
+++ b/broker/snowflake-broker_test.go
@@ -437,7 +437,7 @@ func TestGeoip(t *testing.T) {
 		if err := ctx.metrics.LoadGeoipDatabases("invalid_filename", "invalid_filename6"); err != nil {
 			log.Printf("loading geo ip databases returned error: %v", err)
 		}
-		ctx.metrics.UpdateCountryStats("127.0.0.1", "")
+		ctx.metrics.UpdateCountryStats("127.0.0.1", "", NATUnknown)
 		So(ctx.metrics.tablev4, ShouldEqual, nil)
 
 	})
@@ -507,7 +507,7 @@ func TestMetrics(t *testing.T) {
 			p.offerChannel <- nil
 			<-done
 			ctx.metrics.printMetrics()
-			So(buf.String(), ShouldResemble, "snowflake-stats-end "+time.Now().UTC().Format("2006-01-02 15:04:05")+" (86400 s)\nsnowflake-ips CA=4\nsnowflake-ips-total 4\nsnowflake-ips-standalone 1\nsnowflake-ips-badge 1\nsnowflake-ips-webext 1\nsnowflake-idle-count 8\nclient-denied-count 0\nclient-snowflake-match-count 0\n")
+			So(buf.String(), ShouldResemble, "snowflake-stats-end "+time.Now().UTC().Format("2006-01-02 15:04:05")+" (86400 s)\nsnowflake-ips CA=4\nsnowflake-ips-total 4\nsnowflake-ips-standalone 1\nsnowflake-ips-badge 1\nsnowflake-ips-webext 1\nsnowflake-idle-count 8\nclient-denied-count 0\nclient-restricted-denied-count 0\nclient-unrestricted-denied-count 0\nclient-snowflake-match-count 0\nsnowflake-ips-nat-restricted 0\nsnowflake-ips-nat-unrestricted 0\nsnowflake-ips-nat-unknown 1\n")
 
 		})
 
@@ -521,13 +521,13 @@ func TestMetrics(t *testing.T) {
 			clientOffers(ctx, w, r)
 
 			ctx.metrics.printMetrics()
-			So(buf.String(), ShouldResemble, "snowflake-stats-end "+time.Now().UTC().Format("2006-01-02 15:04:05")+" (86400 s)\nsnowflake-ips \nsnowflake-ips-total 0\nsnowflake-ips-standalone 0\nsnowflake-ips-badge 0\nsnowflake-ips-webext 0\nsnowflake-idle-count 0\nclient-denied-count 8\nclient-snowflake-match-count 0\n")
+			So(buf.String(), ShouldContainSubstring, "client-denied-count 8\nclient-restricted-denied-count 8\nclient-unrestricted-denied-count 0\nclient-snowflake-match-count 0")
 
 			// Test reset
 			buf.Reset()
 			ctx.metrics.zeroMetrics()
 			ctx.metrics.printMetrics()
-			So(buf.String(), ShouldResemble, "snowflake-stats-end "+time.Now().UTC().Format("2006-01-02 15:04:05")+" (86400 s)\nsnowflake-ips \nsnowflake-ips-total 0\nsnowflake-ips-standalone 0\nsnowflake-ips-badge 0\nsnowflake-ips-webext 0\nsnowflake-idle-count 0\nclient-denied-count 0\nclient-snowflake-match-count 0\n")
+			So(buf.String(), ShouldContainSubstring, "snowflake-ips \nsnowflake-ips-total 0\nsnowflake-ips-standalone 0\nsnowflake-ips-badge 0\nsnowflake-ips-webext 0\nsnowflake-idle-count 0\nclient-denied-count 0\nclient-restricted-denied-count 0\nclient-unrestricted-denied-count 0\nclient-snowflake-match-count 0\nsnowflake-ips-nat-restricted 0\nsnowflake-ips-nat-unrestricted 0\nsnowflake-ips-nat-unknown 0\n")
 		})
 		//Test addition of client matches
 		Convey("for client-proxy match", func() {
@@ -548,7 +548,7 @@ func TestMetrics(t *testing.T) {
 			<-done
 
 			ctx.metrics.printMetrics()
-			So(buf.String(), ShouldResemble, "snowflake-stats-end "+time.Now().UTC().Format("2006-01-02 15:04:05")+" (86400 s)\nsnowflake-ips \nsnowflake-ips-total 0\nsnowflake-ips-standalone 0\nsnowflake-ips-badge 0\nsnowflake-ips-webext 0\nsnowflake-idle-count 0\nclient-denied-count 0\nclient-snowflake-match-count 8\n")
+			So(buf.String(), ShouldContainSubstring, "client-denied-count 0\nclient-restricted-denied-count 0\nclient-unrestricted-denied-count 0\nclient-snowflake-match-count 8")
 		})
 		//Test rounding boundary
 		Convey("binning boundary", func() {
@@ -567,12 +567,12 @@ func TestMetrics(t *testing.T) {
 			clientOffers(ctx, w, r)
 
 			ctx.metrics.printMetrics()
-			So(buf.String(), ShouldResemble, "snowflake-stats-end "+time.Now().UTC().Format("2006-01-02 15:04:05")+" (86400 s)\nsnowflake-ips \nsnowflake-ips-total 0\nsnowflake-ips-standalone 0\nsnowflake-ips-badge 0\nsnowflake-ips-webext 0\nsnowflake-idle-count 0\nclient-denied-count 8\nclient-snowflake-match-count 0\n")
+			So(buf.String(), ShouldContainSubstring, "client-denied-count 8\nclient-restricted-denied-count 8\nclient-unrestricted-denied-count 0\n")
 
 			clientOffers(ctx, w, r)
 			buf.Reset()
 			ctx.metrics.printMetrics()
-			So(buf.String(), ShouldResemble, "snowflake-stats-end "+time.Now().UTC().Format("2006-01-02 15:04:05")+" (86400 s)\nsnowflake-ips \nsnowflake-ips-total 0\nsnowflake-ips-standalone 0\nsnowflake-ips-badge 0\nsnowflake-ips-webext 0\nsnowflake-idle-count 0\nclient-denied-count 16\nclient-snowflake-match-count 0\n")
+			So(buf.String(), ShouldContainSubstring, "client-denied-count 16\nclient-restricted-denied-count 16\nclient-unrestricted-denied-count 0\n")
 		})
 
 		//Test unique ip
@@ -605,7 +605,79 @@ func TestMetrics(t *testing.T) {
 			<-done
 
 			ctx.metrics.printMetrics()
-			So(buf.String(), ShouldResemble, "snowflake-stats-end "+time.Now().UTC().Format("2006-01-02 15:04:05")+" (86400 s)\nsnowflake-ips CA=1\nsnowflake-ips-total 1\nsnowflake-ips-standalone 0\nsnowflake-ips-badge 0\nsnowflake-ips-webext 0\nsnowflake-idle-count 8\nclient-denied-count 0\nclient-snowflake-match-count 0\n")
+			So(buf.String(), ShouldContainSubstring, "snowflake-ips CA=1\nsnowflake-ips-total 1")
+		})
+		//Test NAT types
+		Convey("proxy counts by NAT type", func() {
+			w := httptest.NewRecorder()
+			data := bytes.NewReader([]byte(`{"Sid":"ymbcCMto7KHNGYlp","Version":"1.2","Type":"unknown","NAT":"restricted"}`))
+			r, err := http.NewRequest("POST", "snowflake.broker/proxy", data)
+			r.RemoteAddr = "129.97.208.23:8888" //CA geoip
+			So(err, ShouldBeNil)
+			go func(ctx *BrokerContext) {
+				proxyPolls(ctx, w, r)
+				done <- true
+			}(ctx)
+			p := <-ctx.proxyPolls //manually unblock poll
+			p.offerChannel <- nil
+			<-done
+
+			ctx.metrics.printMetrics()
+			So(buf.String(), ShouldContainSubstring, "snowflake-ips-nat-restricted 1\nsnowflake-ips-nat-unrestricted 0\nsnowflake-ips-nat-unknown 0")
+
+			data = bytes.NewReader([]byte(`{"Sid":"ymbcCMto7KHNGYlp","Version":"1.2","Type":"unknown","NAT":"unrestricted"}`))
+			r, err = http.NewRequest("POST", "snowflake.broker/proxy", data)
+			if err != nil {
+				log.Printf("unable to get NewRequest with error: %v", err)
+			}
+			r.RemoteAddr = "129.97.208.24:8888" //CA geoip
+			go func(ctx *BrokerContext) {
+				proxyPolls(ctx, w, r)
+				done <- true
+			}(ctx)
+			p = <-ctx.proxyPolls //manually unblock poll
+			p.offerChannel <- nil
+			<-done
+
+			ctx.metrics.printMetrics()
+			So(buf.String(), ShouldContainSubstring, "snowflake-ips-nat-restricted 1\nsnowflake-ips-nat-unrestricted 1\nsnowflake-ips-nat-unknown 0")
+		})
+		//Test client failures by NAT type
+		Convey("client failures by NAT type", func() {
+			w := httptest.NewRecorder()
+			data := bytes.NewReader([]byte("test"))
+			r, err := http.NewRequest("POST", "snowflake.broker/client", data)
+			r.Header.Set("Snowflake-NAT-TYPE", "restricted")
+			So(err, ShouldBeNil)
+
+			clientOffers(ctx, w, r)
+
+			ctx.metrics.printMetrics()
+			So(buf.String(), ShouldContainSubstring, "client-denied-count 8\nclient-restricted-denied-count 8\nclient-unrestricted-denied-count 0\nclient-snowflake-match-count 0")
+
+			buf.Reset()
+			ctx.metrics.zeroMetrics()
+
+			r, err = http.NewRequest("POST", "snowflake.broker/client", data)
+			r.Header.Set("Snowflake-NAT-TYPE", "unrestricted")
+			So(err, ShouldBeNil)
+
+			clientOffers(ctx, w, r)
+
+			ctx.metrics.printMetrics()
+			So(buf.String(), ShouldContainSubstring, "client-denied-count 8\nclient-restricted-denied-count 0\nclient-unrestricted-denied-count 8\nclient-snowflake-match-count 0")
+
+			buf.Reset()
+			ctx.metrics.zeroMetrics()
+
+			r, err = http.NewRequest("POST", "snowflake.broker/client", data)
+			r.Header.Set("Snowflake-NAT-TYPE", "unknown")
+			So(err, ShouldBeNil)
+
+			clientOffers(ctx, w, r)
+
+			ctx.metrics.printMetrics()
+			So(buf.String(), ShouldContainSubstring, "client-denied-count 8\nclient-restricted-denied-count 8\nclient-unrestricted-denied-count 0\nclient-snowflake-match-count 0")
 		})
 	})
 }
diff --git a/broker/snowflake-heap.go b/broker/snowflake-heap.go
index 12fe557..16dd264 100644
--- a/broker/snowflake-heap.go
+++ b/broker/snowflake-heap.go
@@ -11,6 +11,7 @@ over the offer and answer channels.
 type Snowflake struct {
 	id            string
 	proxyType     string
+	natType       string
 	offerChannel  chan *ClientOffer
 	answerChannel chan []byte
 	clients       int
diff --git a/doc/broker-spec.txt b/doc/broker-spec.txt
index c3177e0..9e4b8ae 100644
--- a/doc/broker-spec.txt
+++ b/doc/broker-spec.txt
@@ -8,7 +8,7 @@ The Snowflake broker is used to hand out Snowflake proxies to clients using the
 
 This document specifies how the Snowflake broker interacts with other parts of the Tor ecosystem, starting with the metrics CollecTor module and to be expanded upon later.
 
-1. Metrics Reporting (version 1.0)
+1. Metrics Reporting (version 1.1)
 
 Metrics data from the Snowflake broker can be retrieved by sending an HTTP GET request to https://[Snowflake broker URL]/metrics and consists of the following items:
 
@@ -62,12 +62,44 @@ Metrics data from the Snowflake broker can be retrieved by sending an HTTP GET r
         from the broker but no proxies were available, rounded up to
         the nearest multiple of 8.
 
+    "client-restricted-denied-count" NUM NL
+        [At most once.]
+
+        A count of the number of times a client with a restricted or
+        unknown NAT type has requested a proxy from the broker but no
+        proxies were available, rounded up to the nearest multiple of 8.
+
+    "client-unrestricted-denied-count" NUM NL
+        [At most once.]
+
+        A count of the number of times a client with an unrestricted NAT
+        type has requested a proxy from the broker but no proxies were
+        available, rounded up to the nearest multiple of 8.
+
     "client-snowflake-match-count" NUM NL
         [At most once.]
 
         A count of the number of times a client successfully received a
         proxy from the broker, rounded up to the nearest multiple of 8.
 
+    "snowflake-ips-nat-restricted" NUM NL
+        [At most once.]
+
+        A count of the total number of unique IP addresses of snowflake
+        proxies that have a restricted NAT type.
+
+    "snowflake-ips-nat-unrestricted" NUM NL
+        [At most once.]
+
+        A count of the total number of unique IP addresses of snowflake
+        proxies that have an unrestricted NAT type.
+
+    "snowflake-ips-nat-unknown" NUM NL
+        [At most once.]
+
+        A count of the total number of unique IP addresses of snowflake
+        proxies that have an unknown NAT type.
+
 2. Broker messaging specification and endpoints
 
 The broker facilitates the connection of snowflake clients and snowflake proxies



More information about the tor-commits mailing list