From debdabb605e4dedcf935d8a1c9c1bbad6b839135 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sandor=20Sz=C3=BCcs?= Date: Thu, 3 Nov 2022 09:49:34 +0100 Subject: [PATCH] test: use goleak to autodetect leaks test that redistest does not leak goroutines fix: found goroutine leaks fix: wrap close in sync.once.Do fix: shallow copy can not copy by deref ptr in case it contains sync.Once refactor: *sync-Once to sync.Once as shown in stdlib Go example MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Sandor Szücs --- dataclients/kubernetes/kube_test.go | 3 ++ dataclients/kubernetes/main_test.go | 11 ++++ dataclients/kubernetes/routegroups_test.go | 6 +++ eskip/eskip_test.go | 47 +++++++++++------ eskipfile/remote.go | 16 ++++++ eskipfile/remote_test.go | 7 +++ eskipfile/watch.go | 7 ++- filters/serve/serve_test.go | 1 + go.mod | 3 +- go.sum | 6 +++ net/httpclient.go | 39 ++++++++++---- net/httpclient_test.go | 2 + net/main_test.go | 11 ++++ net/redisclient.go | 2 + net/redisclient_test.go | 18 +++---- net/redistest/main_test.go | 11 ++++ net/redistest/redistest_test.go | 17 ++++++ routesrv/routesrv.go | 2 +- routing/datasource_test.go | 1 + tracing/tracers/basic/basic.go | 60 +++++++++++++++++++--- 20 files changed, 222 insertions(+), 48 deletions(-) create mode 100644 dataclients/kubernetes/main_test.go create mode 100644 net/main_test.go create mode 100644 net/redistest/main_test.go create mode 100644 net/redistest/redistest_test.go diff --git a/dataclients/kubernetes/kube_test.go b/dataclients/kubernetes/kube_test.go index 6b8ee872a2..8c5ba7d49b 100644 --- a/dataclients/kubernetes/kube_test.go +++ b/dataclients/kubernetes/kube_test.go @@ -29,6 +29,7 @@ import ( log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/goleak" "github.com/zalando/skipper/dataclients/kubernetes/definitions" "github.com/zalando/skipper/eskip" @@ -645,6 +646,8 @@ func TestIngressClassFilter(t *testing.T) { } func TestIngress(t *testing.T) { + defer goleak.VerifyNone(t) + api := newTestAPI(t, nil, &definitions.IngressList{}) defer api.Close() diff --git a/dataclients/kubernetes/main_test.go b/dataclients/kubernetes/main_test.go new file mode 100644 index 0000000000..b1827b6a3c --- /dev/null +++ b/dataclients/kubernetes/main_test.go @@ -0,0 +1,11 @@ +package kubernetes_test + +import ( + "testing" + + "go.uber.org/goleak" +) + +func TestMain(m *testing.M) { + goleak.VerifyTestMain(m) +} diff --git a/dataclients/kubernetes/routegroups_test.go b/dataclients/kubernetes/routegroups_test.go index 82637c5c81..44cb86bf2a 100644 --- a/dataclients/kubernetes/routegroups_test.go +++ b/dataclients/kubernetes/routegroups_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/zalando/skipper/dataclients/kubernetes/kubernetestest" + "go.uber.org/goleak" ) func TestRouteGroupExamples(t *testing.T) { @@ -19,6 +20,7 @@ func TestRouteGroupClusterState(t *testing.T) { } func TestRouteGroupTraffic(t *testing.T) { + defer goleak.VerifyNone(t) kubernetestest.FixturesToTest(t, "testdata/routegroups/traffic") } @@ -27,6 +29,7 @@ func TestRouteGroupEastWest(t *testing.T) { } func TestRouteGroupEastWestRange(t *testing.T) { + defer goleak.VerifyNone(t) kubernetestest.FixturesToTest(t, "testdata/routegroups/east-west-range") } @@ -35,14 +38,17 @@ func TestRouteGroupHTTPSRedirect(t *testing.T) { } func TestRouteGroupDefaultFilters(t *testing.T) { + defer goleak.VerifyNone(t) kubernetestest.FixturesToTest(t, "testdata/routegroups/default-filters") } func TestRouteGroupWithIngress(t *testing.T) { + defer goleak.VerifyNone(t) kubernetestest.FixturesToTest(t, "testdata/routegroups/with-ingress") } func TestRouteGroupTracingTag(t *testing.T) { + defer goleak.VerifyNone(t) kubernetestest.FixturesToTest(t, "testdata/routegroups/tracing-tag") } diff --git a/eskip/eskip_test.go b/eskip/eskip_test.go index 12b0c6a1a4..b9e0d9bd17 100644 --- a/eskip/eskip_test.go +++ b/eskip/eskip_test.go @@ -7,6 +7,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/sanity-io/litter" + "go.uber.org/goleak" ) func checkItems(t *testing.T, message string, l, lenExpected int, checkItem func(int) bool) bool { @@ -172,6 +173,8 @@ func TestParseRouteExpression(t *testing.T) { false, }} { t.Run(ti.msg, func(t *testing.T) { + defer goleak.VerifyNone(t) + stringMapKeys := func(m map[string]string) []string { keys := make([]string, 0, len(m)) for k := range m { @@ -323,13 +326,17 @@ func TestParseFilters(t *testing.T) { []*Filter{{Name: "filter1", Args: []interface{}{3.14}}, {Name: "filter2", Args: []interface{}{"key", float64(42)}}}, false, }} { - fs, err := ParseFilters(ti.expression) - if err == nil && ti.err || err != nil && !ti.err { - t.Error(ti.msg, "failure case", err, ti.err) - return - } + t.Run(ti.msg, func(t *testing.T) { + defer goleak.VerifyNone(t) + + fs, err := ParseFilters(ti.expression) + if err == nil && ti.err || err != nil && !ti.err { + t.Error(ti.msg, "failure case", err, ti.err) + return + } - checkFilters(t, ti.msg, fs, ti.check) + checkFilters(t, ti.msg, fs, ti.check) + }) } } @@ -362,6 +369,8 @@ func TestPredicateParsing(t *testing.T) { input: `*`, }} { t.Run(test.title, func(t *testing.T) { + defer goleak.VerifyNone(t) + p, err := ParsePredicates(test.input) if err == nil && test.fail { @@ -382,6 +391,8 @@ func TestPredicateParsing(t *testing.T) { } func TestClone(t *testing.T) { + defer goleak.VerifyNone(t) + r := &Route{ Id: "foo", Path: "/bar", @@ -493,6 +504,8 @@ func TestDefaultFiltersDo(t *testing.T) { want: outputPrependAppend2, }} { t.Run(tt.name, func(t *testing.T) { + defer goleak.VerifyNone(t) + if got := tt.df.Do(tt.routes); !reflect.DeepEqual(got, tt.want) { t.Errorf("Want %v, got %v", tt.want, got) } @@ -502,6 +515,8 @@ func TestDefaultFiltersDo(t *testing.T) { } func TestDefaultFiltersDoCorrectPrependFilters(t *testing.T) { + defer goleak.VerifyNone(t) + filters, err := ParseFilters("status(1) -> status(2) -> status(3)") if err != nil { t.Errorf("Failed to parse filter: %v", err) @@ -597,6 +612,7 @@ func TestEditorPreProcessor(t *testing.T) { want: `r1_filter: Source("1.2.3.4/26") -> normalRequestLatency("100ms", "10ms") -> status(201) -> `, }} { t.Run(tt.name, func(t *testing.T) { + defer goleak.VerifyNone(t) routes, err := Parse(tt.routes) if err != nil { t.Errorf("Failed to parse route: %v", err) @@ -634,7 +650,7 @@ func TestClonePreProcessor(t *testing.T) { repl: "HostAny($1)", }, routes: `r0: Host("www[.]example[.]org") -> status(201) -> ;`, - want: `r0: Host("www[.]example[.]org") -> status(201) -> ; + want: `r0: Host("www[.]example[.]org") -> status(201) -> ; clone_r0: HostAny("www[.]example[.]org") -> status(201) -> ;`, }, { @@ -653,7 +669,7 @@ func TestClonePreProcessor(t *testing.T) { repl: "ClientIP($1)", }, routes: `r1: Source("1.2.3.4/26") -> status(201) -> ;`, - want: `r1: Source("1.2.3.4/26") -> status(201) -> ; + want: `r1: Source("1.2.3.4/26") -> status(201) -> ; clone_r1: ClientIP("1.2.3.4/26") -> status(201) -> ;`, }, { @@ -662,11 +678,11 @@ func TestClonePreProcessor(t *testing.T) { reg: regexp.MustCompile("Source[(](.*)[)]"), repl: "ClientIP($1)", }, - routes: `r0: Host("www[.]example[.]org") -> status(201) -> ; + routes: `r0: Host("www[.]example[.]org") -> status(201) -> ; r1: Source("1.2.3.4/26") -> status(201) -> ;`, - want: `r0: Host("www[.]example[.]org") -> status(201) -> ; - r1: Source("1.2.3.4/26") -> status(201) -> ; + want: `r0: Host("www[.]example[.]org") -> status(201) -> ; + r1: Source("1.2.3.4/26") -> status(201) -> ; clone_r1: ClientIP("1.2.3.4/26") -> status(201) -> ;`, }, { @@ -676,7 +692,7 @@ func TestClonePreProcessor(t *testing.T) { repl: "ClientIP($1)", }, routes: `rn: Source("1.2.3.4/26", "10.5.5.0/24") -> status(201) -> ;`, - want: `rn: Source("1.2.3.4/26", "10.5.5.0/24") -> status(201) -> ; + want: `rn: Source("1.2.3.4/26", "10.5.5.0/24") -> status(201) -> ; clone_rn: ClientIP("1.2.3.4/26", "10.5.5.0/24") -> status(201) -> ;`, }, { @@ -687,8 +703,8 @@ func TestClonePreProcessor(t *testing.T) { }, routes: `r0: Host("www[.]example[.]org") -> status(201) -> ; rn: Source("1.2.3.4/26", "10.5.5.0/24") -> status(201) -> ;`, - want: `r0: Host("www[.]example[.]org") -> status(201) -> ; - rn: Source("1.2.3.4/26", "10.5.5.0/24") -> status(201) -> ; + want: `r0: Host("www[.]example[.]org") -> status(201) -> ; + rn: Source("1.2.3.4/26", "10.5.5.0/24") -> status(201) -> ; clone_rn: ClientIP("1.2.3.4/26", "10.5.5.0/24") -> status(201) -> ;`, }, { @@ -698,10 +714,11 @@ func TestClonePreProcessor(t *testing.T) { repl: "normalRequestLatency($1)", }, routes: `r1_filter: Source("1.2.3.4/26") -> uniformRequestLatency("100ms", "10ms") -> status(201) -> ;`, - want: `r1_filter: Source("1.2.3.4/26") -> uniformRequestLatency("100ms", "10ms") -> status(201) -> ; + want: `r1_filter: Source("1.2.3.4/26") -> uniformRequestLatency("100ms", "10ms") -> status(201) -> ; clone_r1_filter: Source("1.2.3.4/26") -> normalRequestLatency("100ms", "10ms") -> status(201) -> ;`, }} { t.Run(tt.name, func(t *testing.T) { + defer goleak.VerifyNone(t) routes, err := Parse(tt.routes) if err != nil { t.Errorf("Failed to parse route: %v", err) diff --git a/eskipfile/remote.go b/eskipfile/remote.go index 1d8b9a5a12..363f35c569 100644 --- a/eskipfile/remote.go +++ b/eskipfile/remote.go @@ -5,6 +5,7 @@ import ( "io" "os" "strings" + "sync" "time" "github.com/zalando/skipper/eskip" @@ -15,6 +16,7 @@ import ( ) type remoteEskipFile struct { + once sync.Once preloaded bool remotePath string localPath string @@ -55,6 +57,7 @@ func RemoteWatch(o *RemoteWatchOptions) (routing.DataClient, error) { } dataClient := &remoteEskipFile{ + once: sync.Once{}, remotePath: o.RemoteFile, localPath: tempFilename.Name(), threshold: o.Threshold, @@ -135,6 +138,19 @@ func (client *remoteEskipFile) LoadUpdate() ([]*eskip.Route, []string, error) { return newRoutes, deletedRoutes, err } +func (client *remoteEskipFile) Close() { + if client != nil { + client.once.Do(func() { + if client.http != nil { + client.http.Close() + } + if client.eskipFileClient != nil { + client.eskipFileClient.Close() + } + }) + } +} + func isFileRemote(remotePath string) bool { return strings.HasPrefix(remotePath, "http://") || strings.HasPrefix(remotePath, "https://") } diff --git a/eskipfile/remote_test.go b/eskipfile/remote_test.go index e792461c38..c5ba223ec1 100644 --- a/eskipfile/remote_test.go +++ b/eskipfile/remote_test.go @@ -91,6 +91,13 @@ func TestLoadAll(t *testing.T) { t.Run(test.title, func(t *testing.T) { options := &RemoteWatchOptions{RemoteFile: s.URL, Threshold: 10, Verbose: true, FailOnStartup: true} client, err := RemoteWatch(options) + defer func() { + c, ok := client.(*remoteEskipFile) + if ok { + c.Close() + time.Sleep(time.Second) + } + }() if err == nil && test.fail { t.Error("failed to fail") return diff --git a/eskipfile/watch.go b/eskipfile/watch.go index 683bd52bb3..a86cf55924 100644 --- a/eskipfile/watch.go +++ b/eskipfile/watch.go @@ -3,6 +3,7 @@ package eskipfile import ( "os" "reflect" + "sync" "github.com/zalando/skipper/eskip" ) @@ -21,6 +22,7 @@ type WatchClient struct { getAll chan (chan<- watchResponse) getUpdates chan (chan<- watchResponse) quit chan struct{} + once sync.Once } // Watch creates a route configuration client with file watching. Watch doesn't follow file system nodes, it @@ -31,6 +33,7 @@ func Watch(name string) *WatchClient { getAll: make(chan (chan<- watchResponse)), getUpdates: make(chan (chan<- watchResponse)), quit: make(chan struct{}), + once: sync.Once{}, } go c.watch() @@ -157,5 +160,7 @@ func (c *WatchClient) LoadUpdate() ([]*eskip.Route, []string, error) { // Close stops watching the configured file and providing updates. func (c *WatchClient) Close() { - close(c.quit) + c.once.Do(func() { + close(c.quit) + }) } diff --git a/filters/serve/serve_test.go b/filters/serve/serve_test.go index e0a735d660..9ec6ccd61f 100644 --- a/filters/serve/serve_test.go +++ b/filters/serve/serve_test.go @@ -121,6 +121,7 @@ func TestServe(t *testing.T) { if err != nil || string(b) != strings.Join(parts, "") { t.Error("failed to serve body") } + ctx.Response().Body.Close() } func TestStreamBody(t *testing.T) { diff --git a/go.mod b/go.mod index 12cdf9f19c..642b95e503 100644 --- a/go.mod +++ b/go.mod @@ -33,7 +33,7 @@ require ( github.com/sarslanhan/cronmask v0.0.0-20190709075623-766eca24d011 github.com/sirupsen/logrus v1.8.1 github.com/sony/gobreaker v0.5.0 - github.com/stretchr/testify v1.7.0 + github.com/stretchr/testify v1.8.0 github.com/szuecs/rate-limit-buffer v0.7.1 github.com/testcontainers/testcontainers-go v0.12.0 github.com/tidwall/gjson v1.12.1 @@ -43,6 +43,7 @@ require ( github.com/yookoala/gofast v0.6.0 github.com/yuin/gopher-lua v0.0.0-20210529063254-f4c35e4016d9 go.uber.org/atomic v1.9.0 + go.uber.org/goleak v1.2.0 golang.org/x/crypto v0.0.0-20220817201139-bc19a97f63c8 golang.org/x/net v0.0.0-20220909164309-bea034e7d591 golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8 diff --git a/go.sum b/go.sum index c4b8cacadc..90cb160124 100644 --- a/go.sum +++ b/go.sum @@ -803,6 +803,7 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+ github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.2.0 h1:Hbg2NidpLE8veEBkEZTL3CvlkUIVzuU9jDplZO54c48= github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/testify v0.0.0-20161117074351-18a02ba4a312/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v0.0.0-20180303142811-b89eecf5ca5d/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= @@ -812,6 +813,9 @@ github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/syndtr/gocapability v0.0.0-20170704070218-db04d3cc01c8/go.mod h1:hkRG7XYTFWNJGYcbNJQlaLq0fg1yr4J4t/NcTQtrfww= github.com/syndtr/gocapability v0.0.0-20180916011248-d98352740cb2/go.mod h1:hkRG7XYTFWNJGYcbNJQlaLq0fg1yr4J4t/NcTQtrfww= github.com/syndtr/gocapability v0.0.0-20200815063812-42c35b437635/go.mod h1:hkRG7XYTFWNJGYcbNJQlaLq0fg1yr4J4t/NcTQtrfww= @@ -891,6 +895,8 @@ go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE= go.uber.org/atomic v1.9.0 h1:ECmE8Bn/WFTYwEW/bpKD3M8VtR/zQVbavAoalC1PYyE= go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/goleak v1.1.12/go.mod h1:cwTWslyiVhfpKIDGSZEM2HlOvcqm+tG4zioyIeLoqMQ= +go.uber.org/goleak v1.2.0 h1:xqgm/S+aQvhWFTtR0XK3Jvg7z8kGV8P4X14IzwN3Eqk= +go.uber.org/goleak v1.2.0/go.mod h1:XJYK+MuIchqpmGmUSAzotztawfKvYLUIgg7guXrwVUo= go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0= go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q= golang.org/x/crypto v0.0.0-20171113213409-9f005a07e0d3/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= diff --git a/net/httpclient.go b/net/httpclient.go index bcc4662529..cdf901d6bb 100644 --- a/net/httpclient.go +++ b/net/httpclient.go @@ -7,6 +7,7 @@ import ( "net/http/httptrace" "net/url" "strings" + "sync" "time" "github.com/opentracing/opentracing-go" @@ -24,6 +25,7 @@ const ( // opentracing to the wrapped http.Client with the same interface as // http.Client from the stdlib. type Client struct { + once sync.Once client http.Client tr *Transport log logging.Logger @@ -58,6 +60,7 @@ func NewClient(o Options) *Client { } c := &Client{ + once: sync.Once{}, client: http.Client{ Transport: tr, }, @@ -70,10 +73,14 @@ func NewClient(o Options) *Client { } func (c *Client) Close() { - c.tr.Close() - if c.sr != nil { - c.sr.Close() - } + c.once.Do(func() { + if c.tr != nil { + c.tr.Close() + } + if c.sr != nil { + c.sr.Close() + } + }) } func (c *Client) Head(url string) (*http.Response, error) { @@ -202,6 +209,7 @@ type Options struct { // Transport wraps an http.Transport and adds support for tracing and // bearerToken injection. type Transport struct { + once sync.Once quit chan struct{} closed bool tr *http.Transport @@ -265,6 +273,7 @@ func NewTransport(options Options) *Transport { } t := &Transport{ + once: sync.Once{}, quit: make(chan struct{}), tr: htransport, tracer: options.Tracer, @@ -320,15 +329,25 @@ func WithBearerToken(t *Transport, bearerToken string) *Transport { } func (t *Transport) shallowCopy() *Transport { - tt := *t - return &tt + return &Transport{ + once: sync.Once{}, + quit: t.quit, + closed: t.closed, + tr: t.tr, + tracer: t.tracer, + spanName: t.spanName, + componentName: t.componentName, + bearerToken: t.bearerToken, + } } func (t *Transport) Close() { - if !t.closed { - t.closed = true - close(t.quit) - } + t.once.Do(func() { + if !t.closed { + t.closed = true + close(t.quit) + } + }) } func (t *Transport) CloseIdleConnections() { diff --git a/net/httpclient_test.go b/net/httpclient_test.go index 1784498458..4a9d83bd93 100644 --- a/net/httpclient_test.go +++ b/net/httpclient_test.go @@ -36,6 +36,7 @@ func TestClient(t *testing.T) { if err != nil { t.Fatalf("Failed to get a tracer: %v", err) } + defer tracer.Close() for _, tt := range []struct { name string @@ -201,6 +202,7 @@ func TestTransport(t *testing.T) { if err != nil { t.Fatalf("Failed to get a tracer: %v", err) } + defer tracer.Close() for _, tt := range []struct { name string diff --git a/net/main_test.go b/net/main_test.go new file mode 100644 index 0000000000..ee7bce612c --- /dev/null +++ b/net/main_test.go @@ -0,0 +1,11 @@ +package net_test + +import ( + "testing" + + "go.uber.org/goleak" +) + +func TestMain(m *testing.M) { + goleak.VerifyTestMain(m) +} diff --git a/net/redisclient.go b/net/redisclient.go index cdb7a384ce..7083b68bc4 100644 --- a/net/redisclient.go +++ b/net/redisclient.go @@ -287,6 +287,7 @@ func (r *RedisRingClient) startUpdater(ctx context.Context) { } r.log.Infof("Start goroutine to update redis instances every %s", r.options.UpdateInterval) + defer r.log.Info("Stopped goroutine to update redis") for { select { @@ -353,6 +354,7 @@ func (r *RedisRingClient) Close() { r.once.Do(func() { r.closed = true close(r.quit) + r.ring.Close() }) } diff --git a/net/redisclient_test.go b/net/redisclient_test.go index e648a40f58..de74f14669 100644 --- a/net/redisclient_test.go +++ b/net/redisclient_test.go @@ -89,6 +89,7 @@ func TestRedisClient(t *testing.T) { if err != nil { t.Fatalf("Failed to get a tracer: %v", err) } + defer tracer.Close() redisAddr, done := redistest.NewTestRedis(t) defer done() @@ -110,17 +111,7 @@ func TestRedisClient(t *testing.T) { { name: "With AddrUpdater", options: &RedisOptions{ - AddrUpdater: func() []string { return []string{redisAddr} }, - // i := 0 - // return func() []string { - // i++ - // if i < 2 { - // return []string{redisAddr} - // } - // return []string{redisAddr, redisAddr2} - // }() - - // }, + AddrUpdater: func() []string { return []string{redisAddr} }, UpdateInterval: 10 * time.Millisecond, }, wantErr: false, @@ -156,7 +147,9 @@ func TestRedisClient(t *testing.T) { } } - go func() { <-ch }() // create client will block + if ch != nil { + go func() { <-ch }() // create client will block + } cli := NewRedisRingClient(tt.options) defer func() { if !cli.closed { @@ -1059,6 +1052,7 @@ func TestRedisClientSetAddr(t *testing.T) { } { t.Run(tt.name, func(t *testing.T) { r := NewRedisRingClient(tt.options) + defer r.Close() for i := 0; i < len(tt.keys); i++ { r.Set(context.Background(), tt.keys[i], tt.vals[i], time.Second) } diff --git a/net/redistest/main_test.go b/net/redistest/main_test.go new file mode 100644 index 0000000000..d0df07d1a1 --- /dev/null +++ b/net/redistest/main_test.go @@ -0,0 +1,11 @@ +package redistest_test + +import ( + "testing" + + "go.uber.org/goleak" +) + +func TestMain(m *testing.M) { + goleak.VerifyTestMain(m) +} diff --git a/net/redistest/redistest_test.go b/net/redistest/redistest_test.go new file mode 100644 index 0000000000..94c6f4dba9 --- /dev/null +++ b/net/redistest/redistest_test.go @@ -0,0 +1,17 @@ +package redistest + +import ( + "context" + "testing" + "time" +) + +func TestRedistest(t *testing.T) { + r, done := NewTestRedis(t) + defer done() + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + if err := ping(ctx, r, ""); err != nil { + t.Fatalf("Failed to ping redis: %v", err) + } +} diff --git a/routesrv/routesrv.go b/routesrv/routesrv.go index ecc71cacf4..d1306d6c9f 100644 --- a/routesrv/routesrv.go +++ b/routesrv/routesrv.go @@ -128,7 +128,7 @@ func (rs *RouteServer) ServeHTTP(w http.ResponseWriter, r *http.Request) { } func newShutdownFunc(rs *RouteServer) func(delay time.Duration) { - once := &sync.Once{} + once := sync.Once{} rs.wg.Add(1) return func(delay time.Duration) { diff --git a/routing/datasource_test.go b/routing/datasource_test.go index 38936fe8e5..ff5016e6b0 100644 --- a/routing/datasource_test.go +++ b/routing/datasource_test.go @@ -16,6 +16,7 @@ import ( ) func TestNoMultipleTreePredicates(t *testing.T) { + for _, ti := range []struct { routes string err bool diff --git a/tracing/tracers/basic/basic.go b/tracing/tracers/basic/basic.go index e5a9f15f45..cbcabb7c8b 100644 --- a/tracing/tracers/basic/basic.go +++ b/tracing/tracers/basic/basic.go @@ -4,13 +4,25 @@ import ( "fmt" "strconv" "strings" + "sync" "time" basic "github.com/opentracing/basictracer-go" opentracing "github.com/opentracing/opentracing-go" ) -func InitTracer(opts []string) (opentracing.Tracer, error) { +type CloseableTracer interface { + opentracing.Tracer + Close() +} + +type BasicTracer struct { + tracer opentracing.Tracer + quit chan struct{} + once sync.Once +} + +func InitTracer(opts []string) (CloseableTracer, error) { fmt.Printf("DO NOT USE IN PRODUCTION\n") var ( dropAllLogs bool @@ -56,6 +68,19 @@ func InitTracer(opts []string) (opentracing.Tracer, error) { } } } + + quit := make(chan struct{}) + bt := &BasicTracer{ + tracer: basic.NewWithOptions(basic.Options{ + DropAllLogs: dropAllLogs, + ShouldSample: func(traceID uint64) bool { return traceID%sampleModulo == 0 }, + MaxLogsPerSpan: maxLogsPerSpan, + Recorder: recorder, + }), + quit: quit, + once: sync.Once{}, + } + go func() { for { rec := recorder.(*basic.InMemorySpanRecorder) @@ -65,16 +90,17 @@ func InitTracer(opts []string) (opentracing.Tracer, error) { for _, span := range spans { fmt.Printf("SAMPLED=%#v\n", span) } - time.Sleep(1 * time.Second) + + select { + case <-time.After(1 * time.Second): + case <-quit: + return + } + } }() - return basic.NewWithOptions(basic.Options{ - DropAllLogs: dropAllLogs, - ShouldSample: func(traceID uint64) bool { return traceID%sampleModulo == 0 }, - MaxLogsPerSpan: maxLogsPerSpan, - Recorder: recorder, - }), nil + return bt, nil } func missingArg(opt string) error { @@ -84,3 +110,21 @@ func missingArg(opt string) error { func invalidArg(opt string, err error) error { return fmt.Errorf("invalid argument for %s option: %s", opt, err) } + +func (bt *BasicTracer) StartSpan(operationName string, opts ...opentracing.StartSpanOption) opentracing.Span { + return bt.tracer.StartSpan(operationName, opts...) +} + +func (bt *BasicTracer) Inject(sm opentracing.SpanContext, format interface{}, carrier interface{}) error { + return bt.tracer.Inject(sm, format, carrier) +} + +func (bt *BasicTracer) Extract(format interface{}, carrier interface{}) (opentracing.SpanContext, error) { + return bt.tracer.Extract(format, carrier) +} + +func (bt *BasicTracer) Close() { + bt.once.Do(func() { + close(bt.quit) + }) +}