Refactor Proxy to be an interface with fewer dependencies

Signed-off-by: Phan Le <[email protected]>

access_log/access_log_record.go

@@ -7,10 +7,7 @@ import (
 	"net/http"
 	"time"
 
-	"github.com/cloudfoundry/gorouter/log"
 	"github.com/cloudfoundry/gorouter/route"
-
-	"github.com/cloudfoundry/loggregatorlib/emitter"
 )
 
 type AccessLogRecord struct {
@@ -36,7 +33,7 @@ func (r *AccessLogRecord) FormatRequestHeader(k string) (v string) {
 }
 
 func (r *AccessLogRecord) ResponseTime() float64 {
-	return float64(r.FinishedAt.UnixNano() - r.StartedAt.UnixNano())/float64(time.Second)
+	return float64(r.FinishedAt.UnixNano()-r.StartedAt.UnixNano()) / float64(time.Second)
 }
 
 func (r *AccessLogRecord) makeRecord() *bytes.Buffer {
@@ -51,7 +48,6 @@ func (r *AccessLogRecord) makeRecord() *bytes.Buffer {
 		fmt.Fprintf(b, `%d `, r.Response.StatusCode)
 	}
 
-
 	fmt.Fprintf(b, `%d `, r.BodyBytesSent)
 	fmt.Fprintf(b, `"%s" `, r.FormatRequestHeader("Referer"))
 	fmt.Fprintf(b, `"%s" `, r.FormatRequestHeader("User-Agent"))
@@ -78,17 +74,23 @@ func (r *AccessLogRecord) WriteTo(w io.Writer) (int64, error) {
 	return recordBuffer.WriteTo(w)
 }
 
-func (r *AccessLogRecord) Emit(e emitter.Emitter) {
+func (r *AccessLogRecord) ApplicationId() string {
 	if r.RouteEndpoint == nil {
-		return
+		return ""
 	}
 
 	if r.RouteEndpoint.ApplicationId == "" {
-		return
+		return ""
+	}
+
+	return r.RouteEndpoint.ApplicationId
+}
+
+func (r *AccessLogRecord) LogMessage() string {
+	if r.ApplicationId() == "" {
+		return ""
 	}
-	recordBuffer := r.makeRecord()
-	message := recordBuffer.String()
-	log.Debugf("Logging to the loggregator: %s", message)
-	e.Emit(r.RouteEndpoint.ApplicationId, message)
 
+	recordBuffer := r.makeRecord()
+	return recordBuffer.String()
 }

access_log/access_log_record_test.go

@@ -14,17 +14,17 @@ type AccessLogRecordSuite struct{}
 var _ = Suite(&AccessLogRecordSuite{})
 
 func CompleteAccessLogRecord() AccessLogRecord {
-    return AccessLogRecord{
+	return AccessLogRecord{
 		Request: &http.Request{
-			Host: "FakeRequestHost",
+			Host:   "FakeRequestHost",
 			Method: "FakeRequestMethod",
-			Proto: "FakeRequestProto",
+			Proto:  "FakeRequestProto",
 			URL: &url.URL{
 				Opaque: "http://example.com/request",
 			},
 			Header: http.Header{
-				"Referer": []string {"FakeReferer"},
-				"User-Agent": []string {"FakeUserAgent"},
+				"Referer":    []string{"FakeReferer"},
+				"User-Agent": []string{"FakeUserAgent"},
 			},
 			RemoteAddr: "FakeRemoteAddr",
 		},
@@ -35,77 +35,69 @@ func CompleteAccessLogRecord() AccessLogRecord {
 		RouteEndpoint: &route.Endpoint{
 			ApplicationId: "FakeApplicationId",
 		},
-		StartedAt: time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
+		StartedAt:  time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
 		FinishedAt: time.Date(2000, time.January, 1, 0, 1, 0, 0, time.UTC),
 	}
 }
 
-
 func (s *AccessLogRecordSuite) TestMakeRecordWithAllValues(c *C) {
 	record := CompleteAccessLogRecord()
 
 	recordString := "FakeRequestHost - " +
-			"[01/01/2000:00:00:00 +0000] " +
-			"\"FakeRequestMethod http://example.com/request FakeRequestProto\" " +
-			"200 " +
-			"23 " +
-			"\"FakeReferer\" " +
-			"\"FakeUserAgent\" " +
-			"FakeRemoteAddr " +
-			"response_time:60.000000000 " +
-			"app_id:FakeApplicationId\n"
+		"[01/01/2000:00:00:00 +0000] " +
+		"\"FakeRequestMethod http://example.com/request FakeRequestProto\" " +
+		"200 " +
+		"23 " +
+		"\"FakeReferer\" " +
+		"\"FakeUserAgent\" " +
+		"FakeRemoteAddr " +
+		"response_time:60.000000000 " +
+		"app_id:FakeApplicationId\n"
 
 	c.Assert(record.makeRecord().String(), Equals, recordString)
 }
 
 func (s *AccessLogRecordSuite) TestMakeRecordWithValuesMissing(c *C) {
 	record := AccessLogRecord{
 		Request: &http.Request{
-			Host: "FakeRequestHost",
+			Host:   "FakeRequestHost",
 			Method: "FakeRequestMethod",
-			Proto: "FakeRequestProto",
+			Proto:  "FakeRequestProto",
 			URL: &url.URL{
 				Opaque: "http://example.com/request",
 			},
 			Header: http.Header{
-				"Referer": []string {"FakeReferer"},
-				"User-Agent": []string {"FakeUserAgent"},
+				"Referer":    []string{"FakeReferer"},
+				"User-Agent": []string{"FakeUserAgent"},
 			},
 			RemoteAddr: "FakeRemoteAddr",
 		},
 		StartedAt: time.Date(2000, time.January, 1, 0, 0, 0, 0, time.UTC),
 	}
 
 	recordString := "FakeRequestHost - " +
-			"[01/01/2000:00:00:00 +0000] " +
-			"\"FakeRequestMethod http://example.com/request FakeRequestProto\" " +
-			"MissingResponseStatusCode " +
-			"0 " +
-			"\"FakeReferer\" " +
-			"\"FakeUserAgent\" " +
-			"FakeRemoteAddr " +
-			"response_time:MissingFinishedAt " +
-			"app_id:MissingRouteEndpointApplicationId\n"
+		"[01/01/2000:00:00:00 +0000] " +
+		"\"FakeRequestMethod http://example.com/request FakeRequestProto\" " +
+		"MissingResponseStatusCode " +
+		"0 " +
+		"\"FakeReferer\" " +
+		"\"FakeUserAgent\" " +
+		"FakeRemoteAddr " +
+		"response_time:MissingFinishedAt " +
+		"app_id:MissingRouteEndpointApplicationId\n"
 
 	c.Assert(record.makeRecord().String(), Equals, recordString)
 }
 
-func (s *AccessLogRecordSuite) TestEmit(c *C) {
+func (s *AccessLogRecordSuite) TestLogMessage(c *C) {
 	record := CompleteAccessLogRecord()
 
 	recordString := record.makeRecord().String()
 
-	emit := NewMockEmitter()
-
-	record.Emit(emit)
-	c.Assert(emit.message, Equals, recordString)
+	c.Assert(record.LogMessage(), Equals, recordString)
 }
 
-func (s *AccessLogRecordSuite) TestEmitWithRouteEndpointMissing(c *C) {
-	record := AccessLogRecord{	}
-
-	emit := NewMockEmitter()
-
-	record.Emit(emit)
-	c.Assert(emit.emitted, Equals, false)
+func (s *AccessLogRecordSuite) TestLogMessageWithRouteEndpointMissing(c *C) {
+	record := AccessLogRecord{}
+	c.Assert(record.LogMessage(), Equals, "")
 }

access_log/create_running_access_logger.go

@@ -11,7 +11,7 @@ func CreateRunningAccessLogger(config *config.Config) (accessLogger AccessLogger
 	loggregatorSharedSecret := config.LoggregatorConfig.SharedSecret
 
 	if config.AccessLog != "" || loggregatorUrl != "" {
-		file, err := os.OpenFile(config.AccessLog, os.O_WRONLY | os.O_APPEND | os.O_CREATE, 0666)
+		file, err := os.OpenFile(config.AccessLog, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0666)
 		if err != nil && config.AccessLog != "" {
 			panic(err)
 		}

access_log/create_running_access_logger_test.go

@@ -1,11 +1,12 @@
 package access_log
 
 import (
-	. "launchpad.net/gocheck"
 	"github.com/cloudfoundry/gorouter/config"
+	. "launchpad.net/gocheck"
 )
 
 type CreateRunningAccessLoggerSuite struct{}
+
 var _ = Suite(&CreateRunningAccessLoggerSuite{})
 
 func (s *CreateRunningAccessLoggerSuite) TestCreateNullAccessLoggerIfNoAccesLogAndNoLoggregatorUrl(c *C) {
@@ -37,6 +38,6 @@ func (s *CreateRunningAccessLoggerSuite) TestPanicsIfInvalidAccessLogLocation(c
 	config := config.DefaultConfig()
 	config.AccessLog = "/this\\should/panic"
 	c.Assert(func() {
-			CreateRunningAccessLogger(config)
-		}, PanicMatches, "open /this\\\\should/panic: no such file or directory")
+		CreateRunningAccessLogger(config)
+	}, PanicMatches, "open /this\\\\should/panic: no such file or directory")
 }

access_log/file_and_loggregator_access_logger.go

@@ -7,22 +7,22 @@ import (
 
 	"github.com/cloudfoundry/gorouter/log"
 
-	"github.com/cloudfoundry/loggregatorlib/emitter"
 	steno "github.com/cloudfoundry/gosteno"
+	"github.com/cloudfoundry/loggregatorlib/emitter"
 )
 
 type FileAndLoggregatorAccessLogger struct {
-	emitter  emitter.Emitter
-	channel  chan AccessLogRecord
-	writer   io.Writer
-	index    uint
+	emitter emitter.Emitter
+	channel chan AccessLogRecord
+	writer  io.Writer
+	index   uint
 }
 
 func NewFileAndLoggregatorAccessLogger(f io.Writer, loggregatorUrl, loggregatorSharedSecret string, index uint) *FileAndLoggregatorAccessLogger {
 	a := &FileAndLoggregatorAccessLogger{
-		writer:     f,
-		channel:     make(chan AccessLogRecord, 128),
-		index: index,
+		writer:  f,
+		channel: make(chan AccessLogRecord, 128),
+		index:   index,
 	}
 
 	if isValidUrl(loggregatorUrl) {
@@ -39,8 +39,8 @@ func (x *FileAndLoggregatorAccessLogger) Run() {
 		if x.writer != nil {
 			access_record.WriteTo(x.writer)
 		}
-		if x.emitter != nil {
-			access_record.Emit(x.emitter)
+		if x.emitter != nil && access_record.ApplicationId() != "" {
+			x.emitter.Emit(access_record.ApplicationId(), access_record.LogMessage())
 		}
 	}
 }

access_log/file_and_loggregator_access_logger_test.go

@@ -81,7 +81,7 @@ func (m *mockEmitter) EmitLogMessage(l *logmessage.LogMessage) {
 func NewMockEmitter() *mockEmitter {
 	return &mockEmitter{
 		emitted: false,
-		done: make(chan bool, 1),
+		done:    make(chan bool, 1),
 	}
 }
 
@@ -96,17 +96,17 @@ func (s *AccessLoggerSuite) TestEmittingOfLogRecords(c *C) {
 
 	timeout := make(chan bool, 1)
 	go func() {
-		time.Sleep(1*time.Second)
+		time.Sleep(1 * time.Second)
 		timeout <- true
 	}()
 
 	select {
 	case <-testEmitter.done:
-			c.Check(testEmitter.emitted, Equals, true)
-			c.Check(testEmitter.appId, Equals, "my_awesome_id")
-			c.Check(testEmitter.message, Matches, "^.*foo.bar.*\n")
+		c.Check(testEmitter.emitted, Equals, true)
+		c.Check(testEmitter.appId, Equals, "my_awesome_id")
+		c.Check(testEmitter.message, Matches, "^.*foo.bar.*\n")
 	case <-timeout:
-			c.FailNow()
+		c.FailNow()
 	}
 
 	accessLogger.Stop()

access_log/init_test.go

@@ -1,8 +1,8 @@
 package access_log
 
 import (
-	"testing"
 	. "launchpad.net/gocheck"
+	"testing"
 )
 
 func Test(t *testing.T) { TestingT(t) }

access_log/null_access_logger.go

@@ -1,8 +1,8 @@
 package access_log
 
-type NullAccessLogger struct{
+type NullAccessLogger struct {
 }
 
-func(x *NullAccessLogger) Run() {}
-func(x *NullAccessLogger) Stop() {}
-func(x *NullAccessLogger) Log(AccessLogRecord) {}
+func (x *NullAccessLogger) Run()                {}
+func (x *NullAccessLogger) Stop()               {}
+func (x *NullAccessLogger) Log(AccessLogRecord) {}

main.go

@@ -3,9 +3,9 @@ package main
 import (
 	"flag"
 
-	"github.com/cloudfoundry/gorouter/router"
 	"github.com/cloudfoundry/gorouter/config"
 	"github.com/cloudfoundry/gorouter/log"
+	"github.com/cloudfoundry/gorouter/router"
 )
 
 var configFile string

perf_test.go

@@ -5,6 +5,7 @@ import (
 	"strconv"
 	"testing"
 
+	"github.com/cloudfoundry/gorouter/access_log"
 	"github.com/cloudfoundry/gorouter/config"
 	"github.com/cloudfoundry/gorouter/proxy"
 	"github.com/cloudfoundry/gorouter/registry"
@@ -20,13 +21,21 @@ const (
 func BenchmarkRegister(b *testing.B) {
 	c := config.DefaultConfig()
 	mbus := fakeyagnats.New()
-	r := registry.NewRegistry(c, mbus)
-	p := proxy.NewProxy(c, r, varz.NewVarz(r))
+	r := registry.NewCFRegistry(c, mbus)
+
+	proxy.NewProxy(proxy.ProxyArgs{
+		EndpointTimeout: c.EndpointTimeout,
+		Ip:              c.Ip,
+		TraceKey:        c.TraceKey,
+		Registry:        r,
+		Reporter:        varz.NewVarz(r),
+		Logger:          access_log.CreateRunningAccessLogger(c),
+	})
 
 	for i := 0; i < b.N; i++ {
 		str := strconv.Itoa(i)
 
-		p.Register(
+		r.Register(
 			route.Uri("bench.vcap.me."+str),
 			&route.Endpoint{
 				Host: "localhost",