Skip to content

Commit

Permalink
Fix enumeration of local interface IPs (#8)
Browse files Browse the repository at this point in the history
* Extract only the local IPs and set log flags with shortfile name
* Assign bigquery names based on top level columns
* Log all annotation errors
* Add helper to find local IPs with unit tests
  • Loading branch information
stephen-soltesz authored Feb 14, 2020
1 parent 3465709 commit 3133ae4
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 9 deletions.
7 changes: 4 additions & 3 deletions annotator/annotator.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ var (

// Annotations contains the standard columns we would like to add as annotations for every UUID.
type Annotations struct {
UUID string
Timestamp time.Time
Server, Client api.Annotations
UUID string
Timestamp time.Time
Server api.Annotations `bigquery:"server"` // Use Standard Top-Level Column names.
Client api.Annotations `bigquery:"client"` // Use Standard Top-Level Column names.
}

// Annotator is the interface that all systems that want to add metadata should implement.
Expand Down
1 change: 1 addition & 0 deletions handler/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ func (h *handler) annotateAndSave(j *job) {
for _, ann := range h.annotators {
err := ann.Annotate(j.id, annotations)
if err != nil {
log.Println(err)
metrics.AnnotationErrors.Inc()
}
}
Expand Down
22 changes: 18 additions & 4 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"context"
"flag"
"log"
"net"
"net/url"
"sync"
Expand Down Expand Up @@ -40,6 +41,22 @@ var (
mainCtx, mainCancel = context.WithCancel(context.Background())
)

func init() {
log.SetFlags(log.LstdFlags | log.LUTC | log.Lshortfile)
}

func findLocalIPs(localAddrs []net.Addr) []net.IP {
localIPs := []net.IP{}
for _, addr := range localAddrs {
// By default, addr.String() includes the netblock suffix. By casting to
// the underlying net.IPNet we can extract just the IP.
if a, ok := addr.(*net.IPNet); ok {
localIPs = append(localIPs, a.IP)
}
}
return localIPs
}

func main() {
flag.Parse()
rtx.Must(flagx.ArgsFromEnv(flag.CommandLine), "Could not get args from environment variables")
Expand All @@ -59,11 +76,8 @@ func main() {

// Set up IP annotation, first by loading the initial config.
localAddrs, err := net.InterfaceAddrs()
localIPs := []net.IP{}
for _, addr := range localAddrs {
localIPs = append(localIPs, net.ParseIP(addr.String()))
}
rtx.Must(err, "Could not read local addresses")
localIPs := findLocalIPs(localAddrs)
u, err := url.Parse(*maxmindurl)
rtx.Must(err, "Could not parse URL")
p, err := zipfile.FromURL(mainCtx, u)
Expand Down
48 changes: 46 additions & 2 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@ package main
import (
"context"
"io/ioutil"
"net"
"os"
"reflect"
"testing"
"time"

"github.com/m-lab/tcp-info/eventsocket"

"github.com/m-lab/go/rtx"
"github.com/m-lab/tcp-info/eventsocket"
)

func TestMainSmokeTest(t *testing.T) {
Expand Down Expand Up @@ -37,3 +38,46 @@ func TestMainSmokeTest(t *testing.T) {
// Run main. Full coverage, no crash, and return == success!
main()
}

func Test_findLocalIPs(t *testing.T) {
tests := []struct {
name string
local []net.Addr
want []net.IP
}{
{
name: "success",
local: []net.Addr{
&net.IPNet{
IP: net.ParseIP("127.0.0.1"),
Mask: net.CIDRMask(24, 32),
},
&net.IPNet{
IP: net.ParseIP("2001:1900:2100:2d::75"),
Mask: net.CIDRMask(64, 128),
},
},
want: []net.IP{
net.ParseIP("127.0.0.1"),
net.ParseIP("2001:1900:2100:2d::75"),
},
},
{
name: "skip-all-return-empty",
local: []net.Addr{
&net.UnixAddr{
Name: "fake-unix",
Net: "unix",
},
},
want: []net.IP{},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := findLocalIPs(tt.local); !reflect.DeepEqual(got, tt.want) {
t.Errorf("findLocalIPs() = %v, want %v", got, tt.want)
}
})
}
}

0 comments on commit 3133ae4

Please sign in to comment.