From 36ce1089f4303282593cafe922fc2aca5fcb7cad Mon Sep 17 00:00:00 2001 From: Peter Bourgon Date: Fri, 2 Oct 2015 17:08:04 +0200 Subject: [PATCH] Don't export reverseResolver It's only used within package endpoint, so it shouldn't be exported. That means resolver_test becomes resolver_internal_test, and with the previous change to the fixture, we can avoid the dot-import. Also, update method names to reflect it's an unexported type. --- probe/endpoint/reporter.go | 14 +++++------ probe/endpoint/resolver.go | 23 +++++++++---------- ...lver_test.go => resolver_internal_test.go} | 9 ++++---- 3 files changed, 22 insertions(+), 24 deletions(-) rename probe/endpoint/{resolver_test.go => resolver_internal_test.go} (81%) diff --git a/probe/endpoint/reporter.go b/probe/endpoint/reporter.go index 7f4332652..98c086155 100644 --- a/probe/endpoint/reporter.go +++ b/probe/endpoint/reporter.go @@ -28,7 +28,7 @@ type Reporter struct { includeNAT bool conntracker Conntracker natmapper *NATMapper - revResolver *ReverseResolver + reverseResolver *reverseResolver } // SpyDuration is an exported prometheus metric @@ -73,7 +73,7 @@ func NewReporter(hostID, hostName string, includeProcesses bool, useConntrack bo includeProcesses: includeProcesses, conntracker: conntracker, natmapper: natmapper, - revResolver: NewReverseResolver(), + reverseResolver: newReverseResolver(), } } @@ -85,7 +85,7 @@ func (r *Reporter) Stop() { if r.natmapper != nil { r.natmapper.Stop() } - r.revResolver.Stop() + r.reverseResolver.stop() } // Report implements Reporter. @@ -164,9 +164,9 @@ func (r *Reporter) addConnection(rpt *report.Report, localAddr, remoteAddr strin // In case we have a reverse resolution for the IP, we can use it for // the name... - if revRemoteName, err := r.revResolver.Get(remoteAddr); err == nil { + if remoteName, err := r.reverseResolver.get(remoteAddr); err == nil { remoteNode = remoteNode.WithMetadata(map[string]string{ - "name": revRemoteName, + "name": remoteName, }) } @@ -210,9 +210,9 @@ func (r *Reporter) addConnection(rpt *report.Report, localAddr, remoteAddr strin // In case we have a reverse resolution for the IP, we can use it for // the name... - if revRemoteName, err := r.revResolver.Get(remoteAddr); err == nil { + if remoteName, err := r.reverseResolver.get(remoteAddr); err == nil { remoteNode = remoteNode.WithMetadata(map[string]string{ - "name": revRemoteName, + "name": remoteName, }) } diff --git a/probe/endpoint/resolver.go b/probe/endpoint/resolver.go index 12af6ee16..af11ad3d5 100644 --- a/probe/endpoint/resolver.go +++ b/probe/endpoint/resolver.go @@ -16,22 +16,22 @@ const ( rAddrCacheExpiration = 30 * time.Minute ) -var errNotFound = fmt.Errorf("Not found") +var errNotFound = fmt.Errorf("not found") type revResFunc func(addr string) (names []string, err error) -// ReverseResolver is a caching, reverse resolver. -type ReverseResolver struct { +// A caching, reverse resolver. +type reverseResolver struct { addresses chan string cache gcache.Cache Throttle <-chan time.Time // Made public for mocking Resolver revResFunc } -// NewReverseResolver starts a new reverse resolver that performs reverse +// newReverseResolver starts a new reverse resolver that performs reverse // resolutions and caches the result. -func NewReverseResolver() *ReverseResolver { - r := ReverseResolver{ +func newReverseResolver() *reverseResolver { + r := reverseResolver{ addresses: make(chan string, rAddrBacklog), cache: gcache.New(rAddrCacheLen).LRU().Expiration(rAddrCacheExpiration).Build(), Throttle: time.Tick(time.Second / 10), @@ -41,10 +41,10 @@ func NewReverseResolver() *ReverseResolver { return &r } -// Get the reverse resolution for an IP address if already in the cache, a +// get the reverse resolution for an IP address if already in the cache, a // gcache.NotFoundKeyError error otherwise. Note: it returns one of the // possible names that can be obtained for that IP. -func (r *ReverseResolver) Get(address string) (string, error) { +func (r *reverseResolver) get(address string) (string, error) { val, err := r.cache.Get(address) if hostname, ok := val.(string); err == nil && ok { return hostname, nil @@ -53,7 +53,7 @@ func (r *ReverseResolver) Get(address string) (string, error) { return "", errNotFound } if err == gcache.NotFoundKeyError { - // We trigger a asynchronous reverse resolution when not cached + // We trigger a asynchronous reverse resolution when not cached. select { case r.addresses <- address: default: @@ -62,7 +62,7 @@ func (r *ReverseResolver) Get(address string) (string, error) { return "", errNotFound } -func (r *ReverseResolver) loop() { +func (r *reverseResolver) loop() { for request := range r.addresses { // check if the answer is already in the cache if _, err := r.cache.Get(request); err == nil { @@ -80,7 +80,6 @@ func (r *ReverseResolver) loop() { } } -// Stop the async reverse resolver. -func (r *ReverseResolver) Stop() { +func (r *reverseResolver) stop() { close(r.addresses) } diff --git a/probe/endpoint/resolver_test.go b/probe/endpoint/resolver_internal_test.go similarity index 81% rename from probe/endpoint/resolver_test.go rename to probe/endpoint/resolver_internal_test.go index 096113620..a3ac4632e 100644 --- a/probe/endpoint/resolver_test.go +++ b/probe/endpoint/resolver_internal_test.go @@ -1,11 +1,10 @@ -package endpoint_test +package endpoint import ( "errors" "testing" "time" - . "github.com/weaveworks/scope/probe/endpoint" "github.com/weaveworks/scope/test" ) @@ -15,8 +14,8 @@ func TestReverseResolver(t *testing.T) { "4.3.2.1": {"im.a.little.tea.pot"}, } - revRes := NewReverseResolver() - defer revRes.Stop() + revRes := newReverseResolver() + defer revRes.stop() // Use a mocked resolver function. revRes.Resolver = func(addr string) (names []string, err error) { @@ -31,7 +30,7 @@ func TestReverseResolver(t *testing.T) { for ip, names := range tests { test.Poll(t, 100*time.Millisecond, names[0], func() interface{} { - result, _ := revRes.Get(ip) + result, _ := revRes.get(ip) return result }) }