Use better function for determining host IP, add more tests (jaegertr…

…acing#32)

logger_test.go

@@ -0,0 +1,12 @@
+package jaeger
+
+import (
+	"testing"
+)
+
+func TestLogger(t *testing.T) {
+	for _, logger := range []Logger{StdLogger, NullLogger} {
+		logger.Infof("Hi %s", "there")
+		logger.Error("Bad wolf")
+	}
+}

span.go

@@ -217,7 +217,7 @@ func setSpanKind(s *span, key string, value interface{}) bool {
 
 func setPeerIPv4(s *span, key string, value interface{}) bool {
 	if val, ok := value.(string); ok {
-		if ip, err := utils.IPToUint32(val); err == nil {
+		if ip, err := utils.ParseIPToUint32(val); err == nil {
 			s.peer.Ipv4 = int32(ip)
 			return true
 		}

span_test.go

@@ -27,3 +27,12 @@ func TestBaggageIterator(t *testing.T) {
 	})
 	assert.Equal(t, 1, len(b), "only one baggage item should be extracted")
 }
+
+func TestSpanProperties(t *testing.T) {
+	tracer, closer := NewTracer("DOOP", NewConstSampler(true), NewNullReporter())
+	defer closer.Close()
+
+	sp1 := tracer.StartSpan("s1").(*span)
+	assert.Equal(t, tracer, sp1.Tracer())
+	assert.NotNil(t, sp1.Context())
+}

tracer.go

@@ -107,16 +107,17 @@ func NewTracer(
 	if t.timeNow == nil {
 		t.timeNow = time.Now
 	}
-	if t.hostIPv4 == 0 {
-		var localIPInt32 uint32
-		if localIP := utils.GetLocalIP(); localIP != nil {
-			localIPInt32, _ = utils.IPToUint32(localIP.String())
-		}
-		t.hostIPv4 = localIPInt32
-	}
 	if t.logger == nil {
 		t.logger = NullLogger
 	}
+	// TODO once on the new data model, support both v4 and v6 IPs
+	if t.hostIPv4 == 0 {
+		if ip, err := utils.HostIP(); err == nil {
+			t.hostIPv4 = utils.PackIPAsUint32(ip)
+		} else {
+			t.logger.Error("Unable to determine this host's IP address: " + err.Error())
+		}
+	}
 
 	return t, t
 }

tracer_test.go

@@ -170,6 +170,36 @@ func (s *tracerSuite) TestRandomIDNotZero() {
 	rng.Seed(1) // for test coverage
 }
 
+func TestTracerOptions(t *testing.T) {
+	t1, e := time.Parse(time.RFC3339, "2012-11-01T22:08:41+00:00")
+	assert.NoError(t, e)
+
+	timeNow := func() time.Time {
+		return t1
+	}
+	rnd := func() uint64 {
+		return 1
+	}
+
+	openTracer, closer := NewTracer("DOOP", // respect the classics, man!
+		NewConstSampler(true),
+		NewNullReporter(),
+		TracerOptions.Logger(StdLogger),
+		TracerOptions.TimeNow(timeNow),
+		TracerOptions.RandomNumber(rnd),
+		TracerOptions.PoolSpans(true),
+	)
+	defer closer.Close()
+
+	tracer := openTracer.(*tracer)
+	assert.Equal(t, StdLogger, tracer.logger)
+	assert.Equal(t, t1, tracer.timeNow())
+	assert.Equal(t, uint64(1), tracer.randomNumber())
+	assert.Equal(t, uint64(1), tracer.randomNumber())
+	assert.Equal(t, uint64(1), tracer.randomNumber()) // always 1
+	assert.Equal(t, true, tracer.poolSpans)
+}
+
 func TestInjectorExtractorOptions(t *testing.T) {
 	tracer, tc := NewTracer("x", NewConstSampler(true), NewNullReporter(),
 		TracerOptions.Injector("dummy", &dummyPropagator{}),

utils/localip.go

@@ -0,0 +1,90 @@
+// Copyright (c) 2016 Uber Technologies, Inc.
+
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+package utils
+
+import (
+	"errors"
+	"net"
+)
+
+// This code is borrowed from https://github.com/uber/tchannel-go/blob/dev/localip.go
+
+// scoreAddr scores how likely the given addr is to be a remote address and returns the
+// IP to use when listening. Any address which receives a negative score should not be used.
+// Scores are calculated as:
+// -1 for any unknown IP addresses.
+// +300 for IPv4 addresses
+// +100 for non-local addresses, extra +100 for "up" interaces.
+func scoreAddr(iface net.Interface, addr net.Addr) (int, net.IP) {
+	var ip net.IP
+	if netAddr, ok := addr.(*net.IPNet); ok {
+		ip = netAddr.IP
+	} else if netIP, ok := addr.(*net.IPAddr); ok {
+		ip = netIP.IP
+	} else {
+		return -1, nil
+	}
+
+	var score int
+	if ip.To4() != nil {
+		score += 300
+	}
+	if iface.Flags&net.FlagLoopback == 0 && !ip.IsLoopback() {
+		score += 100
+		if iface.Flags&net.FlagUp != 0 {
+			score += 100
+		}
+	}
+	return score, ip
+}
+
+// HostIP tries to find an IP that can be used by other machines to reach this machine.
+func HostIP() (net.IP, error) {
+	interfaces, err := net.Interfaces()
+	if err != nil {
+		return nil, err
+	}
+
+	bestScore := -1
+	var bestIP net.IP
+	// Select the highest scoring IP as the best IP.
+	for _, iface := range interfaces {
+		addrs, err := iface.Addrs()
+		if err != nil {
+			// Skip this interface if there is an error.
+			continue
+		}
+
+		for _, addr := range addrs {
+			score, ip := scoreAddr(iface, addr)
+			if score > bestScore {
+				bestScore = score
+				bestIP = ip
+			}
+		}
+	}
+
+	if bestScore == -1 {
+		return nil, errors.New("no addresses to listen on")
+	}
+
+	return bestIP, nil
+}

utils/utils.go

@@ -21,6 +21,7 @@
 package utils
 
 import (
+	"encoding/binary"
 	"errors"
 	"net"
 	"strconv"
@@ -38,27 +39,8 @@ var (
 	ErrNotFourOctets = errors.New("Wrong number of octets")
 )
 
-// GetLocalIP returns the IP of the host, preferring public over loopback.
-func GetLocalIP() net.IP {
-	addrs, err := net.InterfaceAddrs()
-	if err != nil {
-		return net.IPv4(127, 0, 0, 1)
-	}
-
-	for _, address := range addrs {
-		// check the address type and if it is not a loopback the display it
-		if ipnet, ok := address.(*net.IPNet); ok && !ipnet.IP.IsLoopback() {
-			if ipnet.IP.To4() != nil {
-				return ipnet.IP
-			}
-		}
-	}
-
-	return net.IPv4(127, 0, 0, 1)
-}
-
-// IPToUint32 converts a string ip to an uint32
-func IPToUint32(ip string) (uint32, error) {
+// ParseIPToUint32 converts a string ip (e.g. "x.y.z.w") to an uint32
+func ParseIPToUint32(ip string) (uint32, error) {
 	if ip == "" {
 		return 0, ErrEmptyIP
 	}
@@ -89,3 +71,11 @@ func ParsePort(portString string) (uint16, error) {
 	port, err := strconv.ParseUint(portString, 10, 16)
 	return uint16(port), err
 }
+
+// PackIPAsUint32 packs an IPv4 as uint32
+func PackIPAsUint32(ip net.IP) uint32 {
+	if ipv4 := ip.To4(); ipv4 != nil {
+		return binary.BigEndian.Uint32(ipv4)
+	}
+	return 0
+}

utils/utils_test.go

@@ -21,104 +21,77 @@
 package utils
 
 import (
+	"net"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/assert"
 )
 
 func TestGetLocalIP(t *testing.T) {
-	ip := GetLocalIP()
+	ip, _ := HostIP()
 	assert.NotNil(t, ip, "assert we have an ip")
 }
 
-func TestIPToUint32(t *testing.T) {
-	intIP, err := IPToUint32("127.0.0.1")
-	assert.NoError(t, err)
-	assert.Equal(t, uint32((127<<24)|1), intIP, "expected ip is equal to actual ip")
-
-	intIP, err = IPToUint32("127.xxx.0.1")
-	assert.Error(t, err)
-	assert.EqualValues(t, 0, intIP)
-
-	intIP, err = IPToUint32("")
-	assert.Equal(t, ErrEmptyIP, err)
-	assert.EqualValues(t, 0, intIP)
-}
-
-func TestIPToInt32Error(t *testing.T) {
-	ip := "tcollector-21"
-	intIP, err := IPToUint32(ip)
-	assert.Equal(t, ErrNotFourOctets, err)
-	assert.Equal(t, uint32(0), intIP, "expected ip of 0")
-}
-
-// Various benchmarks
-
-// Passing time by value or by pointer
-
-func passTimeByValue(t time.Time) int64 {
-	return t.UnixNano() / 1000
-}
-
-func passTimeByPtr(t *time.Time) int64 {
-	return t.UnixNano() / 1000
-}
-
-func BenchmarkTimeByValue(b *testing.B) {
-	t := time.Now()
-	for i := 0; i < b.N; i++ {
-		passTimeByValue(t)
+func TestParseIPToUint32(t *testing.T) {
+	tests := []struct {
+		in  string
+		out uint32
+		err error
+	}{
+		{"1.2.3.4", 1<<24 | 2<<16 | 3<<8 | 4, nil},
+		{"127.0.0.1", 127<<24 | 1, nil},
+		{"localhost", 127<<24 | 1, nil},
+		{"127.xxx.0.1", 0, nil},
+		{"", 0, ErrEmptyIP},
+		{"hostname", 0, ErrNotFourOctets},
 	}
-}
 
-func BenchmarkTimeByPtr(b *testing.B) {
-	t := time.Now()
-	for i := 0; i < b.N; i++ {
-		passTimeByPtr(&t)
-	}
-}
+	for _, test := range tests {
+		intIP, err := ParseIPToUint32(test.in)
+		if test.err != nil {
+			assert.Equal(t, test.err, err)
+		} else {
+			assert.Equal(t, test.out, intIP)
+		}
 
-// Checking type via casting vs. via .(type)
-
-func checkTypeViaCast(v interface{}) interface{} {
-	if vv, ok := v.(int64); ok {
-		return vv
-	} else if vv, ok := v.(uint64); ok {
-		return vv
-	} else if vv, ok := v.(int32); ok {
-		return vv
-	} else if vv, ok := v.(uint32); ok {
-		return vv
 	}
-	return nil
 }
 
-func checkTypeViaDotType(v interface{}) interface{} {
-	switch v.(type) {
-	case int64:
-		return v.(int64)
-	case uint64:
-		return v.(uint64)
-	case int32:
-		return v.(int32)
-	case uint32:
-		return v.(uint32)
-	default:
-		return nil
+func TestParsePort(t *testing.T) {
+	tests := []struct {
+		in  string
+		out uint16
+		err bool
+	}{
+		{"123", 123, false},
+		{"77777", 0, true}, // too large for 16bit
+		{"bad-wolf", 0, true},
 	}
-}
-
-func BenchmarkCheckTypeViaCast(b *testing.B) {
-	x := uint32(123)
-	for i := 0; i < b.N; i++ {
-		checkTypeViaCast(x)
+	for _, test := range tests {
+		p, err := ParsePort(test.in)
+		if test.err {
+			assert.Error(t, err)
+		} else {
+			assert.Equal(t, test.out, p)
+		}
 	}
 }
 
-func BenchmarkCheckTypeViaDotType(b *testing.B) {
-	x := uint32(123)
-	for i := 0; i < b.N; i++ {
-		checkTypeViaDotType(x)
+func TestPackIPAsUint32(t *testing.T) {
+	ipv6a := net.IP{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 1, 2, 3, 4}
+	ipv6b := net.ParseIP("2001:0db8:85a3:0000:0000:8a2e:0370:7334")
+	assert.NotNil(t, ipv6a)
+
+	tests := []struct {
+		in  net.IP
+		out uint32
+	}{
+		{net.IPv4(1, 2, 3, 4), 1<<24 | 2<<16 | 3<<8 | 4},
+		{ipv6a, 1<<24 | 2<<16 | 3<<8 | 4}, // IPv6 but convertible to IPv4
+		{ipv6b, 0},
+	}
+	for _, test := range tests {
+		ip := PackIPAsUint32(test.in)
+		assert.Equal(t, test.out, ip)
 	}
 }