From c0b5a1dff2a8922c6b502d63bab328ae01b190a4 Mon Sep 17 00:00:00 2001 From: Curtis Robert Date: Mon, 11 Mar 2024 02:53:38 -0700 Subject: [PATCH] [receiver/dockerstats] Fix leaking goroutines (#31668) **Description:** The dockerstats receiver starts a go routine that isn't shutdown until its context is done. This adds a shutdown method to trigger the end of this running goroutine in a timely manner. This also enables goleak to check for leaking goroutines on this receiver. **Link to tracking Issue:** #30438 **Testing:** All existing and added checks are passing. --- .chloggen/goleak_dockerstatsreceiver.yaml | 27 +++++++++++++++++++ receiver/dockerstatsreceiver/factory.go | 2 +- receiver/dockerstatsreceiver/package_test.go | 14 ++++++++++ receiver/dockerstatsreceiver/receiver.go | 13 ++++++++- receiver/dockerstatsreceiver/receiver_test.go | 1 + 5 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 .chloggen/goleak_dockerstatsreceiver.yaml create mode 100644 receiver/dockerstatsreceiver/package_test.go diff --git a/.chloggen/goleak_dockerstatsreceiver.yaml b/.chloggen/goleak_dockerstatsreceiver.yaml new file mode 100644 index 000000000000..c3e1b2063e97 --- /dev/null +++ b/.chloggen/goleak_dockerstatsreceiver.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: dockerstatsreceiver + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Add shutdown method to fix leaking goroutines + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [30438] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/receiver/dockerstatsreceiver/factory.go b/receiver/dockerstatsreceiver/factory.go index c96a9dfaed55..773a8b36ec90 100644 --- a/receiver/dockerstatsreceiver/factory.go +++ b/receiver/dockerstatsreceiver/factory.go @@ -52,7 +52,7 @@ func createMetricsReceiver( dockerConfig := config.(*Config) dsr := newMetricsReceiver(params, dockerConfig) - scrp, err := scraperhelper.NewScraper(metadata.Type.String(), dsr.scrapeV2, scraperhelper.WithStart(dsr.start)) + scrp, err := scraperhelper.NewScraper(metadata.Type.String(), dsr.scrapeV2, scraperhelper.WithStart(dsr.start), scraperhelper.WithShutdown(dsr.shutdown)) if err != nil { return nil, err } diff --git a/receiver/dockerstatsreceiver/package_test.go b/receiver/dockerstatsreceiver/package_test.go new file mode 100644 index 000000000000..1e0123693b89 --- /dev/null +++ b/receiver/dockerstatsreceiver/package_test.go @@ -0,0 +1,14 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package dockerstatsreceiver + +import ( + "testing" + + "go.uber.org/goleak" +) + +func TestMain(m *testing.M) { + goleak.VerifyTestMain(m) +} diff --git a/receiver/dockerstatsreceiver/receiver.go b/receiver/dockerstatsreceiver/receiver.go index 5581ed1021a4..071d737d6329 100644 --- a/receiver/dockerstatsreceiver/receiver.go +++ b/receiver/dockerstatsreceiver/receiver.go @@ -40,6 +40,7 @@ type metricsReceiver struct { settings receiver.CreateSettings client *docker.Client mb *metadata.MetricsBuilder + cancel context.CancelFunc } func newMetricsReceiver(set receiver.CreateSettings, config *Config) *metricsReceiver { @@ -65,7 +66,17 @@ func (r *metricsReceiver) start(ctx context.Context, _ component.Host) error { return err } - go r.client.ContainerEventLoop(ctx) + cctx, cancel := context.WithCancel(ctx) + r.cancel = cancel + + go r.client.ContainerEventLoop(cctx) + return nil +} + +func (r *metricsReceiver) shutdown(context.Context) error { + if r.cancel != nil { + r.cancel() + } return nil } diff --git a/receiver/dockerstatsreceiver/receiver_test.go b/receiver/dockerstatsreceiver/receiver_test.go index 26d973647e4f..31202fc2f53e 100644 --- a/receiver/dockerstatsreceiver/receiver_test.go +++ b/receiver/dockerstatsreceiver/receiver_test.go @@ -301,6 +301,7 @@ func TestScrapeV2(t *testing.T) { receivertest.NewNopCreateSettings(), tc.cfgBuilder.withEndpoint(mockDockerEngine.URL).build()) err := receiver.start(context.Background(), componenttest.NewNopHost()) require.NoError(t, err) + defer func() { require.NoError(t, receiver.shutdown(context.Background())) }() actualMetrics, err := receiver.scrapeV2(context.Background()) require.NoError(t, err)