Skip to content

Commit

Permalink
Revert "[exporter/signalfx] Fix memory leak on shutdown (open-telemet…
Browse files Browse the repository at this point in the history
  • Loading branch information
dmitryax authored and XinRanZhAWS committed Mar 13, 2024
1 parent d6948ee commit 1f229c1
Show file tree
Hide file tree
Showing 8 changed files with 11 additions and 86 deletions.
27 changes: 0 additions & 27 deletions .chloggen/goleak_signalfx_correlations.yaml

This file was deleted.

7 changes: 0 additions & 7 deletions exporter/signalfxexporter/internal/apm/correlations/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ type CorrelationClient interface {
Delete(*Correlation, SuccessfulDeleteCB)
Get(dimName string, dimValue string, cb SuccessfulGetCB)
Start()
Shutdown()
}

type request struct {
Expand Down Expand Up @@ -388,9 +387,3 @@ func (cc *Client) Start() {
go cc.processChan()
go cc.processRetryChan()
}

// Shutdown the client. This will block until the context's cancel
// function is complete.
func (cc *Client) Shutdown() {
cc.wg.Wait()
}
23 changes: 10 additions & 13 deletions exporter/signalfxexporter/internal/apm/correlations/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,18 @@ func makeHandler(t *testing.T, corCh chan<- *request, forcedRespCode *atomic.Val
})
}

func setup(t *testing.T) (CorrelationClient, *httptest.Server, chan *request, *atomic.Value, *atomic.Value, context.CancelFunc, context.Context) {
func setup(t *testing.T) (CorrelationClient, chan *request, *atomic.Value, *atomic.Value, context.CancelFunc) {
serverCh := make(chan *request, 100)

var forcedRespCode atomic.Value
var forcedRespPayload atomic.Value
server := httptest.NewServer(makeHandler(t, serverCh, &forcedRespCode, &forcedRespPayload))

ctx, cancel := context.WithCancel(context.Background())
go func() {
<-ctx.Done()
server.Close()
}()

serverURL, err := url.Parse(server.URL)
if err != nil {
Expand Down Expand Up @@ -172,20 +176,13 @@ func setup(t *testing.T) (CorrelationClient, *httptest.Server, chan *request, *a
}
client.Start()

return client, server, serverCh, &forcedRespCode, &forcedRespPayload, cancel, ctx
}

func teardown(ctx context.Context, client CorrelationClient, server *httptest.Server, serverCh chan *request, cancel context.CancelFunc) {
close(serverCh)
cancel()
<-ctx.Done()
client.Shutdown()
server.Close()
return client, serverCh, &forcedRespCode, &forcedRespPayload, cancel
}

func TestCorrelationClient(t *testing.T) {
client, server, serverCh, forcedRespCode, forcedRespPayload, cancel, ctx := setup(t)
defer teardown(ctx, client, server, serverCh, cancel)
client, serverCh, forcedRespCode, forcedRespPayload, cancel := setup(t)
defer close(serverCh)
defer cancel()

for _, correlationType := range []Type{Service, Environment} {
for _, op := range []string{http.MethodPut, http.MethodDelete} {
Expand Down Expand Up @@ -245,7 +242,7 @@ func TestCorrelationClient(t *testing.T) {
client.Correlate(testData, CorrelateCB(func(_ *Correlation, _ error) {}))
// sending the testData twice tests deduplication, since the 500 status
// will trigger retries, and the requests should be deduped and the
// TotalRetriedUpdates should still only be 5
// TotalRertriedUpdates should still only be 5
client.Correlate(testData, CorrelateCB(func(_ *Correlation, _ error) {}))

cors := waitForCors(serverCh, 1, 4)
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,7 @@ type correlationTestClient struct {
correlateCounter int64
}

func (c *correlationTestClient) Start() { /*no-op*/ }
func (c *correlationTestClient) Shutdown() { /*no-op*/ }
func (c *correlationTestClient) Start() { /*no-op*/ }
func (c *correlationTestClient) Get(_ string, dimValue string, cb correlations.SuccessfulGetCB) {
atomic.AddInt64(&c.getCounter, 1)
go func() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ func (cor *Tracker) Shutdown(_ context.Context) error {
if cor != nil {
if cor.correlation != nil {
cor.correlation.cancel()
cor.correlation.CorrelationClient.Shutdown()
}

if cor.pTicker != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,6 @@ func TestTrackerStart(t *testing.T) {
} else {
require.NoError(t, err)
}

assert.NoError(t, tracker.Shutdown(context.Background()))
})
}
}
17 changes: 0 additions & 17 deletions exporter/signalfxexporter/internal/correlation/package_test.go

This file was deleted.

0 comments on commit 1f229c1

Please sign in to comment.