Fix routeview prefix search (#49)

* Uncompress RouteViewIPv4.corrupt
* Fix routeview.Search
* Add three new metrics for parsing routeview input and ASN search results

Author: stephen-soltesz

asnannotator/asn.go

@@ -12,6 +12,7 @@ import (
 	"github.com/m-lab/tcp-info/inetdiag"
 	"github.com/m-lab/uuid-annotator/annotator"
 	"github.com/m-lab/uuid-annotator/ipinfo"
+	"github.com/m-lab/uuid-annotator/metrics"
 	"github.com/m-lab/uuid-annotator/routeview"
 	"github.com/m-lab/uuid-annotator/tarreader"
 )
@@ -93,13 +94,15 @@ func (a *asnAnnotator) annotateIPHoldingLock(src string) *annotator.Network {
 			ann.ASName = a.asnames[ann.ASNumber]
 		}
 		// The annotation succeeded with IPv4.
+		metrics.ASNSearches.WithLabelValues("ipv4-success").Inc()
 		return ann
 	}
 
 	ipnet, err = a.asn6.Search(src)
 	if err != nil {
 		// In this case, the search has failed twice.
 		ann.Missing = true
+		metrics.ASNSearches.WithLabelValues("missing").Inc()
 		return ann
 	}
 
@@ -110,6 +113,7 @@ func (a *asnAnnotator) annotateIPHoldingLock(src string) *annotator.Network {
 	}
 	ann.CIDR = ipnet.String()
 	// The annotation succeeded with IPv6.
+	metrics.ASNSearches.WithLabelValues("ipv6-success").Inc()
 	return ann
 }
 
@@ -194,15 +198,15 @@ func NewFake() ASNAnnotator {
 	rtx.Must(err, "Could not parse fixed string")
 	asn4Entry.IPNet = *v4net
 	asn4Entry.Systems = "5"
-	f.asn4 = routeview.Index{asn4Entry}
+	f.asn4 = routeview.Index{routeview.NetIndex{asn4Entry}}
 
 	// Set up v6 data for 1111:2222:3333:4444:5555:6666:7777:8888.
 	asn6Entry := routeview.IPNet{}
 	_, v6net, err := net.ParseCIDR("1111:2222:3333:4444:5555:6666:7777:8888/128")
 	rtx.Must(err, "Could not parse fixed string")
 	asn6Entry.IPNet = *v6net
 	asn6Entry.Systems = "9"
-	f.asn6 = routeview.Index{asn6Entry}
+	f.asn6 = routeview.Index{routeview.NetIndex{asn6Entry}}
 
 	// Set up AS name entries for AS5 and AS9
 	f.asnames = ipinfo.ASNames{

metrics/metrics.go

@@ -45,4 +45,24 @@ var (
 		},
 		[]string{"status"},
 	)
+	RouteViewRows = promauto.NewCounterVec(
+		prometheus.CounterOpts{
+			Name: "uuid_annotator_routeview_rows_total",
+			Help: "The number of routeview rows parsed or skipped",
+		},
+		[]string{"status"},
+	)
+	RouteViewParsed = promauto.NewCounter(
+		prometheus.CounterOpts{
+			Name: "uuid_annotator_routeview_parsed_total",
+			Help: "The number of times a routeview file has been parsed",
+		},
+	)
+	ASNSearches = promauto.NewCounterVec(
+		prometheus.CounterOpts{
+			Name: "uuid_annotator_asn_search_total",
+			Help: "The number of ASN annotator searches",
+		},
+		[]string{"status"},
+	)
 )

routeview/parse.go

@@ -11,7 +11,9 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/m-lab/go/logx"
 	"github.com/m-lab/uuid-annotator/annotator"
+	"github.com/m-lab/uuid-annotator/metrics"
 )
 
 // IPNet represents a parsed row in a RouteView file.
@@ -20,17 +22,20 @@ type IPNet struct {
 	Systems string
 }
 
-// Index is a sortable (and searchable) array of IPNets.
-type Index []IPNet
+// NetIndex is a sortable and searchable array of IPNets.
+type NetIndex []IPNet
+
+// Index is searchable array of NetIndexes.
+type Index []NetIndex
 
 // Len, Less, and Swap make Index sortable.
-func (ns Index) Len() int {
+func (ns NetIndex) Len() int {
 	return len(ns)
 }
-func (ns Index) Less(i, j int) bool {
+func (ns NetIndex) Less(i, j int) bool {
 	return bytes.Compare(ns[i].IP, ns[j].IP) < 0
 }
-func (ns Index) Swap(i, j int) {
+func (ns NetIndex) Swap(i, j int) {
 	n := ns[j]
 	ns[j] = ns[i]
 	ns[i] = n
@@ -66,62 +71,94 @@ func ParseSystems(s string) []annotator.System {
 
 // ParseRouteView reads the given csv file and generates a sorted IP list.
 func ParseRouteView(file []byte) Index {
-	result := Index{}
 	sm := map[string]string{}
 
 	skip := 0
+	parsed := 0
 	b := bytes.NewBuffer(file)
 	r := csv.NewReader(b)
 	r.Comma = '\t'
 	r.ReuseRecord = true
 
+	nim := map[int64]NetIndex{}
+
 	for {
 		record, err := r.Read()
 		if err == io.EOF {
+			metrics.RouteViewParsed.Inc()
 			break
 		}
 		if len(record) < 3 {
+			metrics.RouteViewRows.WithLabelValues("missing-fields").Inc()
+			continue
+		}
+		nb, err := strconv.ParseInt(record[1], 10, 32)
+		if err != nil {
+			// Skip malformed line.
+			skip++
+			log.Println("failed to convert netblock size:", record[1])
+			metrics.RouteViewRows.WithLabelValues("corrupt-netblock").Inc()
 			continue
 		}
 		_, n, err := net.ParseCIDR(record[0] + "/" + record[1])
+		if err != nil {
+			// Skip malformed line.
+			skip++
+			log.Println("failed to parse CIDR prefix:", record[0], "with netblock:", record[1])
+			metrics.RouteViewRows.WithLabelValues("corrupt-prefix").Inc()
+			continue
+		}
 		if _, ok := sm[record[2]]; !ok {
 			// Break string connection to underlying RAM allocated by the CSV reader.
 			sm[record[2]] = strings.Repeat(record[2], 1)
 		}
-		if len(result) > 1 && result[len(result)-1].Contains(n.IP) && result[len(result)-1].Systems == record[2] {
-			// If the last network contains the current one with the same systems, skip it.
-			skip++
-			continue
-		}
-		result = append(result, IPNet{IPNet: *n, Systems: sm[record[2]]})
+		parsed++
+		metrics.RouteViewRows.WithLabelValues("parsed").Inc()
+		nim[nb] = append(nim[nb], IPNet{IPNet: *n, Systems: sm[record[2]]})
 	}
-	log.Println("Skipped:", skip, "routeview netblocks of", len(result)+skip)
+	logx.Debug.Println("Skipped:", skip, "routeview netblocks of", parsed+skip)
 
-	// Sort list so that it can be searched.
-	sort.Sort(result)
-	return result
+	// For each netblock, sort each NetIndex array.
+	netblocks := []int64{}
+	for k := range nim {
+		netblocks = append(netblocks, k)
+		sort.Sort(nim[k])
+	}
+	// Sort descending order.
+	sort.Slice(netblocks, func(i, j int) bool { return netblocks[i] > netblocks[j] })
+
+	// Construct the final index, from largest to smallest netblock.
+	ix := Index{}
+	for _, k := range netblocks {
+		ix = append(ix, nim[k])
+	}
+	return ix
 }
 
 // ErrNoASNFound is returned when search fails to identify a network for the given src IP.
-var ErrNoASNFound = errors.New("No ASN found for address")
+var ErrNoASNFound = errors.New("no ASN found for address")
 
 // Search attempts to find the given IP in the Index.
-func (ns Index) Search(s string) (IPNet, error) {
+func (ix Index) Search(s string) (IPNet, error) {
 	// bytes.Compare will only work correctly when both net.IPs have the same byte count.
 	ip := net.ParseIP(s)
 	if ip.To4() != nil {
 		ip = ip.To4()
 	}
-	node := sort.Search(len(ns), func(i int) bool {
-		if ns[i].Contains(ip) {
-			// Becaue sort.Search finds the lowest index where f(i) is true, we must return
-			// true when the IPNet contains the given IP to prevent off by 1 errors.
-			return true
+	// Search each set of NetIndexes from longest to shortest, returning the first (longest) match.
+	for i := range ix {
+		ns := ix[i]
+		node := sort.Search(len(ns), func(i int) bool {
+			if ns[i].Contains(ip) {
+				// Becaue sort.Search finds the lowest index where f(i) is true, we must return
+				// true when the IPNet contains the given IP to prevent off by 1 errors.
+				return true
+			}
+			return bytes.Compare(ns[i].IP, ip) >= 0
+		})
+		if node < len(ns) && ns[node].Contains(ip) {
+			return ns[node], nil
 		}
-		return bytes.Compare(ns[i].IP, ip) >= 0
-	})
-	if node < len(ns) && ns[node].Contains(ip) {
-		return ns[node], nil
 	}
 	return IPNet{}, ErrNoASNFound
 }

routeview/parse_test.go

@@ -1,37 +1,19 @@
 package routeview
 
 import (
-	"bytes"
 	"fmt"
 	"io/ioutil"
 	"log"
 	"net"
 	"reflect"
 	"testing"
 
-	"github.com/m-lab/annotation-service/api"
-	"github.com/m-lab/annotation-service/asn"
 	"github.com/m-lab/go/rtx"
 	"github.com/m-lab/uuid-annotator/annotator"
 	"github.com/m-lab/uuid-annotator/tarreader"
 )
 
-var ns Index
-var an api.Annotator
-
 func init() {
-	var err error
-	// Load file in setup for Benchmark.
-	b, err := ioutil.ReadFile("../testdata/RouteViewIPv4.pfx2as.gz")
-	rtx.Must(err, "Failed to read routeview data")
-	b2, err := tarreader.FromGZ(b)
-	rtx.Must(err, "Failed to decompress routeview")
-	ns = ParseRouteView(b2)
-
-	// Only used for Benchmark.
-	an, err = asn.LoadASNDatasetFromReader(bytes.NewBuffer(b2))
-	rtx.Must(err, "Failed to load api.Annotator")
-
 	log.SetFlags(0)
 }
 
@@ -44,34 +26,46 @@ func TestParseRouteView(t *testing.T) {
 		{
 			name:      "success-ipv4",
 			filename:  "../testdata/RouteViewIPv4.pfx2as.gz",
-			wantCount: 545957,
+			wantCount: 845161,
 		},
 		{
 			name:      "success-ipv6",
 			filename:  "../testdata/RouteViewIPv6.pfx2as.gz",
-			wantCount: 54317,
+			wantCount: 83125,
 		},
 		{
 			name:      "corrupt-ipv4",
-			filename:  "../testdata/RouteViewIPv4.corrupt.gz",
-			wantCount: 6,
+			filename:  "../testdata/RouteViewIPv4.corrupt",
+			wantCount: 50,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			gz, err := ioutil.ReadFile(tt.filename)
+			b, err := ioutil.ReadFile(tt.filename)
 			rtx.Must(err, "Failed to read routeview data")
-			b, err := tarreader.FromGZ(gz)
-			rtx.Must(err, "Failed to decompress routeview")
+			if tt.filename[len(tt.filename)-3:] == ".gz" {
+				b, err = tarreader.FromGZ(b)
+				rtx.Must(err, "Failed to decompress routeview")
+			}
 
 			ns := ParseRouteView(b)
-			if len(ns) != tt.wantCount {
-				t.Errorf("Parse() = %v, want %v", len(ns), tt.wantCount)
+			c := countIndex(ns)
+			if c != tt.wantCount {
+				t.Errorf("Parse() = %v, want %v", c, tt.wantCount)
 			}
 		})
 	}
 }
 
+// Count returns the total number of networks in the index.
+func countIndex(ix Index) int {
+	total := 0
+	for i := range ix {
+		total += len(ix[i])
+	}
+	return total
+}
+
 func TestParseSystems(t *testing.T) {
 	tests := []struct {
 		name string
@@ -138,7 +132,7 @@ func TestIndex_Search(t *testing.T) {
 			filename: "../testdata/RouteViewIPv4.pfx2as.gz",
 			src:      "1.0.192.1",
 			want: IPNet{
-				IPNet:   net.IPNet{IP: net.ParseIP("1.0.128.0").To4(), Mask: net.CIDRMask(17, 32)},
+				IPNet:   net.IPNet{IP: net.ParseIP("1.0.192.0").To4(), Mask: net.CIDRMask(21, 32)},
 				Systems: "23969",
 			},
 		},
@@ -192,36 +186,26 @@ func TestIndex_Search(t *testing.T) {
 }
 
 func BenchmarkSearch(b *testing.B) {
-	found := 0
-	missing := 0
-	src := "1.0.192.1"
-	b.ResetTimer()
-	for i := 0; i < b.N; i++ {
-		r, err := ns.Search(src)
-		if err != nil {
-			missing++
-		} else {
-			found++
-		}
-		_ = ParseSystems(r.Systems)
-	}
-	fmt.Println("f:", found, "m:", missing)
-}
+	gz, err := ioutil.ReadFile("../testdata/RouteViewIPv4.pfx2as.gz")
+	rtx.Must(err, "Failed to read routeview data")
+	raw, err := tarreader.FromGZ(gz)
+	rtx.Must(err, "Failed to decompress routeview")
+	ns := ParseRouteView(raw)
 
-func BenchmarkAnnotate(b *testing.B) {
 	found := 0
 	missing := 0
-	src := "1.0.192.1"
-	ann := api.Annotations{}
+	src := []string{"1.0.192.1", "12.189.157.193"}
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
-		err := an.Annotate(src, &ann)
-		if err != nil {
-			missing++
-		} else {
-			found++
+		for _, s := range src {
+			r, err := ns.Search(s)
+			if err != nil {
+				missing++
+			} else {
+				found++
+			}
+			_ = ParseSystems(r.Systems)
 		}
-		ann.Network = nil
 	}
 	fmt.Println("f:", found, "m:", missing)
 }