Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable and enforce goleak in all packages #30438

Open
crobert-1 opened this issue Jan 11, 2024 · 8 comments
Open

Enable and enforce goleak in all packages #30438

crobert-1 opened this issue Jan 11, 2024 · 8 comments
Assignees
@crobert-1
Labels
enhancement New feature or request

Comments

@crobert-1
Copy link
Member

Component(s)

No response

Is your feature request related to a problem? Please describe.

There's currently no widespread testing in this repository for detecting leaking goroutines. Leaked goroutines are a result of improperly handling concurrency in golang, and a common cause of memory leaks.

Describe the solution you'd like

goleak is a testing tool used to detect leaking goroutines. This is currently being done in core (#9165) and would be useful in contrib as well.

Describe alternatives you've considered

No response

Additional context

No response
@crobert-1 crobert-1 added enhancement New feature or request needs triage New item requiring triage labels Jan 11, 2024
@crobert-1 crobert-1 self-assigned this Jan 11, 2024
@crobert-1
Copy link
Member Author

crobert-1 commented Jan 11, 2024 •
edited

A couple notes from my initial efforts:

  1. The tailsampling processor is the only existing package that already uses goleak.
  2. For goleak to work properly it's required to run from the TestMain function of testing. However, the following packages already use TestMain. I'll have to investigate to determine how to use TestMain to accomplish existing functionality as well as goleak for each of these packages.
./testbed/tests/trace_test.go
./testbed/stabilitytests/trace_test.go
./testbed/correctnesstests/traces/correctness_test.go
./exporter/splunkhecexporter/integration_test.go
./exporter/kineticaexporter/exporter_metric_test.go
./exporter/datadogexporter/traces_exporter_test.go

@crobert-1 crobert-1 mentioned this issue Jan 11, 2024
Merged
@crobert-1
Copy link
Member Author

I decided to upgrade my bash script for adding goleak to make it do everything in one go, without manual intervention. This used some commands from contrib's makefile to get all go packages, and then filter down to only packages containing tests. The script also now cleans up failing tests to clearly delineate which tests are successul, which are failing.

ALL_PKG_DIRS=$(go list -f '{{ .Dir }}' ./... | sort)
ALL_SRC=$(find $ALL_PKG_DIRS -name '*.go' -not -path '*/third_party/*' -not -path '*/local/*' -type f | sort)
ALL_PKGS_WITH_TESTS=$(dirname $(find $ALL_PKG_DIRS -name '*.go' -not -path '*/third_party/*' -not -path '*/local/*' -type f | grep test.go) | sort --unique)

FAILED_PACKAGES_LIST_FILE="/Users/crobert/dev/opentelemetry-collector-contrib/failed_packages.txt"

for pkg_dir in $ALL_PKGS_WITH_TESTS;
do
  if [[ "$pkg_dir" == '/Users/crobert/dev/opentelemetry-collector-contrib' ]]; then
    echo "Skipping repo head dir"
    continue
  fi

  echo "Attempting package: $pkg_dir"
  cp ./package_test.go $pkg_dir

  PACKAGE_NAME=$(basename $pkg_dir)

  sed -i '' -e "s|package component|package $PACKAGE_NAME|g" $pkg_dir/package_test.go

  pushd . > /dev/null
  cd $pkg_dir
  echo "Running go mod tidy"
  go mod tidy
  echo "Running go test"
  go test -v . > /dev/null
  if [ $? -eq 0 ]; then
    echo "Package was successful"
  else
    echo "Package was not successful"
    echo "$pkg_dir" >> $FAILED_PACKAGES_LIST_FILE
    rm $pkg_dir/package_test.go
    go mod tidy
  fi
  popd > /dev/null
done

Sample output:

~/dev/opentelemetry-collector-contrib $ ./add_leak_test.sh 
Skipping repo head dir
Attempting package: /Users/crobert/dev/opentelemetry-collector-contrib/cmd/configschema
Running go mod tidy
Running go test
Package was not successful
...
Attempting package: /Users/crobert/dev/opentelemetry-collector-contrib/cmd/mdatagen/internal/metadata
Running go mod tidy
Running go test
Package was successful

@crobert-1
Copy link
Member Author

crobert-1 commented Jan 11, 2024 •
edited

Here's the initial list of failing packages:

./cmd/configschema
./cmd/configschema/cfgmetadatagen/cfgmetadatagen
./cmd/configschema/docsgen/docsgen
./cmd/mdatagen
./cmd/mdatagen/internal/metadata
./cmd/opampsupervisor
./cmd/otelcontribcol
./cmd/oteltestbedcol
./cmd/telemetrygen
./cmd/telemetrygen/internal/e2etest
./connector/countconnector
./connector/datadogconnector
./connector/exceptionsconnector
./connector/failoverconnector
./connector/routingconnector
./connector/servicegraphconnector
./connector/spanmetricsconnector
./exporter/alertmanagerexporter
./exporter/alibabacloudlogserviceexporter
./exporter/awscloudwatchlogsexporter
./exporter/awsemfexporter
./exporter/awskinesisexporter
./exporter/awss3exporter
./exporter/awsxrayexporter
./exporter/azuredataexplorerexporter
./exporter/azuremonitorexporter
./exporter/carbonexporter
./exporter/cassandraexporter
./exporter/clickhouseexporter
./exporter/coralogixexporter
./exporter/datadogexporter
./exporter/datadogexporter/integrationtest
./exporter/datadogexporter/internal/clientutil
./exporter/datadogexporter/internal/hostmetadata
./exporter/datadogexporter/internal/hostmetadata/internal/gohai
./exporter/datadogexporter/internal/hostmetadata/provider
./exporter/datadogexporter/internal/logs
./exporter/datadogexporter/internal/metrics
./exporter/datadogexporter/internal/testutil
./exporter/datasetexporter
./exporter/dynatraceexporter
./exporter/dynatraceexporter/config
./exporter/elasticsearchexporter
./exporter/f5cloudexporter
./exporter/fileexporter
./exporter/googlecloudexporter
./exporter/googlecloudpubsubexporter
./exporter/googlemanagedprometheusexporter
./exporter/honeycombmarkerexporter
./exporter/influxdbexporter
./exporter/instanaexporter
./exporter/kafkaexporter
./exporter/kineticaexporter
./exporter/loadbalancingexporter
./exporter/logicmonitorexporter
./exporter/logicmonitorexporter/internal/logs
./exporter/logicmonitorexporter/internal/traces
./exporter/logzioexporter
./exporter/lokiexporter
./exporter/mezmoexporter
./exporter/opencensusexporter
./exporter/opensearchexporter
./exporter/prometheusexporter
./exporter/prometheusremotewriteexporter
./exporter/pulsarexporter
./exporter/sapmexporter
./exporter/sentryexporter
./exporter/signalfxexporter
./exporter/signalfxexporter/internal/correlation
./exporter/signalfxexporter/internal/dimensions
./exporter/signalfxexporter/internal/hostmetadata
./exporter/signalfxexporter/internal/translation
./exporter/skywalkingexporter
./exporter/splunkhecexporter
./exporter/sumologicexporter
./exporter/syslogexporter
./exporter/tencentcloudlogserviceexporter
./exporter/zipkinexporter
./extension/asapauthextension
./extension/awsproxy
./extension/basicauthextension
./extension/bearertokenauthextension
./extension/encoding/jaegerencodingextension
./extension/encoding/jsonlogencodingextension
./extension/encoding/otlpencodingextension
./extension/encoding/textencodingextension
./extension/encoding/zipkinencodingextension
./extension/headerssetterextension
./extension/healthcheckextension
./extension/httpforwarder
./extension/jaegerremotesampling
./extension/jaegerremotesampling/internal
./extension/oauth2clientauthextension
./extension/observer/dockerobserver
./extension/observer/ecsobserver
./extension/observer/ecstaskobserver
./extension/observer/hostobserver
./extension/observer/k8sobserver
./extension/oidcauthextension
./extension/opampextension
./extension/pprofextension
./extension/remotetapextension
./extension/sigv4authextension
./extension/storage/dbstorage
./extension/storage/filestorage
./extension/storage/storagetest
./internal/aws/ecsutil
./internal/aws/k8s/k8sclient
./internal/aws/proxy
./internal/aws/xray
./internal/aws/xray/telemetry
./internal/common/ttlmap
./internal/datadog
./internal/docker
./internal/filter/filterottl
./internal/metadataproviders/aws/ec2
./internal/metadataproviders/azure
./internal/sharedcomponent
./internal/splunk
./pkg/ottl
./pkg/ottl/e2e
./pkg/ottl/ottlfuncs
./pkg/stanza/adapter
./pkg/stanza/operator/helper
./pkg/stanza/operator/parser/regex
./pkg/stanza/operator/transformer/recombine
./processor/attributesprocessor
./processor/cumulativetodeltaprocessor
./processor/datadogprocessor
./processor/deltatorateprocessor
./processor/filterprocessor
./processor/groupbyattrsprocessor
./processor/groupbytraceprocessor
./processor/k8sattributesprocessor
./processor/k8sattributesprocessor/internal/kube
./processor/k8sattributesprocessor/internal/observability
./processor/logstransformprocessor
./processor/metricsgenerationprocessor
./processor/metricstransformprocessor
./processor/probabilisticsamplerprocessor
./processor/redactionprocessor
./processor/remotetapprocessor
./processor/resourcedetectionprocessor
./processor/resourcedetectionprocessor/internal
./processor/resourcedetectionprocessor/internal/aws/ec2
./processor/resourcedetectionprocessor/internal/aws/ecs
./processor/resourcedetectionprocessor/internal/aws/eks
./processor/resourcedetectionprocessor/internal/aws/elasticbeanstalk
./processor/resourcedetectionprocessor/internal/aws/lambda
./processor/resourcedetectionprocessor/internal/azure
./processor/resourcedetectionprocessor/internal/azure/aks
./processor/resourcedetectionprocessor/internal/docker
./processor/resourcedetectionprocessor/internal/env
./processor/resourcedetectionprocessor/internal/heroku
./processor/resourcedetectionprocessor/internal/k8snode
./processor/resourcedetectionprocessor/internal/system
./processor/resourceprocessor
./processor/routingprocessor
./processor/schemaprocessor
./processor/servicegraphprocessor
./processor/spanmetricsprocessor
./processor/spanprocessor
./processor/sumologicprocessor
./processor/tailsamplingprocessor
./processor/tailsamplingprocessor/internal/sampling
./processor/transformprocessor
./processor/transformprocessor/internal/logs
./processor/transformprocessor/internal/metrics
./processor/transformprocessor/internal/traces
./receiver/activedirectorydsreceiver
./receiver/activedirectorydsreceiver/internal/metadata
./receiver/aerospikereceiver
./receiver/aerospikereceiver/internal/metadata
./receiver/apachereceiver
./receiver/apachereceiver/internal/metadata
./receiver/apachesparkreceiver
./receiver/apachesparkreceiver/internal/metadata
./receiver/awscloudwatchmetricsreceiver
./receiver/awscloudwatchreceiver
./receiver/awscontainerinsightreceiver
./receiver/awscontainerinsightreceiver/internal/cadvisor
./receiver/awscontainerinsightreceiver/internal/ecsInfo
./receiver/awscontainerinsightreceiver/internal/host
./receiver/awscontainerinsightreceiver/internal/k8sapiserver
./receiver/awscontainerinsightreceiver/internal/stores
./receiver/awsecscontainermetricsreceiver
./receiver/awsfirehosereceiver
./receiver/awsxrayreceiver
./receiver/awsxrayreceiver/internal/udppoller
./receiver/azureblobreceiver
./receiver/azureeventhubreceiver
./receiver/azuremonitorreceiver
./receiver/azuremonitorreceiver/internal/metadata
./receiver/bigipreceiver
./receiver/bigipreceiver/internal/metadata
./receiver/carbonreceiver
./receiver/chronyreceiver
./receiver/chronyreceiver/internal/metadata
./receiver/cloudflarereceiver
./receiver/cloudfoundryreceiver
./receiver/collectdreceiver
./receiver/couchdbreceiver
./receiver/couchdbreceiver/internal/metadata
./receiver/datadogreceiver
./receiver/dockerstatsreceiver
./receiver/dockerstatsreceiver/internal/metadata
./receiver/elasticsearchreceiver
./receiver/elasticsearchreceiver/internal/metadata
./receiver/expvarreceiver
./receiver/expvarreceiver/internal/metadata
./receiver/filelogreceiver
./receiver/filereceiver
./receiver/filestatsreceiver
./receiver/filestatsreceiver/internal/metadata
./receiver/flinkmetricsreceiver
./receiver/flinkmetricsreceiver/internal/metadata
./receiver/fluentforwardreceiver
./receiver/fluentforwardreceiver/observ
./receiver/gitproviderreceiver
./receiver/gitproviderreceiver/internal/metadata
./receiver/gitproviderreceiver/internal/scraper/githubscraper
./receiver/googlecloudpubsubreceiver
./receiver/googlecloudpubsubreceiver/internal
./receiver/googlecloudspannerreceiver
./receiver/googlecloudspannerreceiver/internal/datasource
./receiver/googlecloudspannerreceiver/internal/filter
./receiver/googlecloudspannerreceiver/internal/filterfactory
./receiver/googlecloudspannerreceiver/internal/metadata
./receiver/googlecloudspannerreceiver/internal/metadataconfig
./receiver/googlecloudspannerreceiver/internal/metadataparser
./receiver/googlecloudspannerreceiver/internal/statsreader
./receiver/haproxyreceiver
./receiver/haproxyreceiver/internal/metadata
./receiver/hostmetricsreceiver
./receiver/hostmetricsreceiver/internal/scraper/cpuscraper
./receiver/hostmetricsreceiver/internal/scraper/cpuscraper/internal/metadata
./receiver/hostmetricsreceiver/internal/scraper/diskscraper
./receiver/hostmetricsreceiver/internal/scraper/diskscraper/internal/metadata
./receiver/hostmetricsreceiver/internal/scraper/filesystemscraper
./receiver/hostmetricsreceiver/internal/scraper/filesystemscraper/internal/metadata
./receiver/hostmetricsreceiver/internal/scraper/loadscraper
./receiver/hostmetricsreceiver/internal/scraper/loadscraper/internal/metadata
./receiver/hostmetricsreceiver/internal/scraper/memoryscraper
./receiver/hostmetricsreceiver/internal/scraper/memoryscraper/internal/metadata
./receiver/hostmetricsreceiver/internal/scraper/networkscraper
./receiver/hostmetricsreceiver/internal/scraper/networkscraper/internal/metadata
./receiver/hostmetricsreceiver/internal/scraper/pagingscraper
./receiver/hostmetricsreceiver/internal/scraper/pagingscraper/internal/metadata
./receiver/hostmetricsreceiver/internal/scraper/processesscraper
./receiver/hostmetricsreceiver/internal/scraper/processesscraper/internal/metadata
./receiver/hostmetricsreceiver/internal/scraper/processscraper
./receiver/hostmetricsreceiver/internal/scraper/processscraper/internal/metadata
./receiver/httpcheckreceiver
./receiver/httpcheckreceiver/internal/metadata
./receiver/iisreceiver
./receiver/iisreceiver/internal/metadata
./receiver/influxdbreceiver
./receiver/jaegerreceiver
./receiver/jmxreceiver
./receiver/jmxreceiver/internal/subprocess
./receiver/journaldreceiver
./receiver/k8sclusterreceiver
./receiver/k8sclusterreceiver/internal/clusterresourcequota
./receiver/k8sclusterreceiver/internal/collection
./receiver/k8sclusterreceiver/internal/cronjob
./receiver/k8sclusterreceiver/internal/demonset
./receiver/k8sclusterreceiver/internal/deployment
./receiver/k8sclusterreceiver/internal/hpa
./receiver/k8sclusterreceiver/internal/jobs
./receiver/k8sclusterreceiver/internal/metadata
./receiver/k8sclusterreceiver/internal/namespace
./receiver/k8sclusterreceiver/internal/node
./receiver/k8sclusterreceiver/internal/pod
./receiver/k8sclusterreceiver/internal/replicaset
./receiver/k8sclusterreceiver/internal/replicationcontroller
./receiver/k8sclusterreceiver/internal/resourcequota
./receiver/k8sclusterreceiver/internal/statefulset
./receiver/k8seventsreceiver
./receiver/k8sobjectsreceiver
./receiver/kafkametricsreceiver
./receiver/kafkametricsreceiver/internal/metadata
./receiver/kafkareceiver
./receiver/kubeletstatsreceiver
./receiver/kubeletstatsreceiver/internal/kubelet
./receiver/kubeletstatsreceiver/internal/metadata
./receiver/lokireceiver
./receiver/memcachedreceiver
./receiver/memcachedreceiver/internal/metadata
./receiver/mongodbatlasreceiver
./receiver/mongodbatlasreceiver/internal/metadata
./receiver/mongodbreceiver
./receiver/mongodbreceiver/internal/metadata
./receiver/mysqlreceiver
./receiver/mysqlreceiver/internal/metadata
./receiver/namedpipereceiver
./receiver/nginxreceiver
./receiver/nginxreceiver/internal/metadata
./receiver/nsxtreceiver
./receiver/nsxtreceiver/internal/metadata
./receiver/opencensusreceiver
./receiver/opencensusreceiver/internal/ocmetrics
./receiver/opencensusreceiver/internal/octrace
./receiver/oracledbreceiver
./receiver/oracledbreceiver/internal/metadata
./receiver/osqueryreceiver
./receiver/otlpjsonfilereceiver
./receiver/podmanreceiver
./receiver/postgresqlreceiver
./receiver/postgresqlreceiver/internal/metadata
./receiver/prometheusreceiver
./receiver/prometheusreceiver/internal
./receiver/pulsarreceiver
./receiver/purefareceiver
./receiver/purefareceiver/internal
./receiver/purefbreceiver
./receiver/purefbreceiver/internal
./receiver/rabbitmqreceiver
./receiver/rabbitmqreceiver/internal/metadata
./receiver/receivercreator
./receiver/redisreceiver
./receiver/redisreceiver/internal/metadata
./receiver/riakreceiver
./receiver/riakreceiver/internal/metadata
./receiver/saphanareceiver
./receiver/saphanareceiver/internal/metadata
./receiver/sapmreceiver
./receiver/signalfxreceiver
./receiver/simpleprometheusreceiver
./receiver/skywalkingreceiver
./receiver/snmpreceiver
./receiver/snowflakereceiver
./receiver/snowflakereceiver/internal/metadata
./receiver/solacereceiver
./receiver/splunkenterprisereceiver
./receiver/splunkenterprisereceiver/internal/metadata
./receiver/splunkhecreceiver
./receiver/sqlqueryreceiver
./receiver/sqlserverreceiver
./receiver/sqlserverreceiver/internal/metadata
./receiver/sshcheckreceiver
./receiver/sshcheckreceiver/internal/configssh
./receiver/sshcheckreceiver/internal/metadata
./receiver/statsdreceiver
./receiver/syslogreceiver
./receiver/tcplogreceiver
./receiver/udplogreceiver
./receiver/vcenterreceiver
./receiver/vcenterreceiver/internal/metadata
./receiver/wavefrontreceiver
./receiver/webhookeventreceiver
./receiver/windowseventlogreceiver
./receiver/windowsperfcountersreceiver
./receiver/zipkinreceiver
./receiver/zookeeperreceiver
./receiver/zookeeperreceiver/internal/metadata
./testbed/correctnesstests/metrics
./testbed/correctnesstests/traces
./testbed/stabilitytests
./testbed/testbed
./testbed/tests

This was referenced Jan 11, 2024
Merged
Closed
@crobert-1
Copy link
Member Author

crobert-1 commented Jan 12, 2024 •
edited

After ignoring opencensus-go issues, here's the remaining package list that needs to be enabled:

This was referenced Jan 12, 2024
Closed
Closed
Open
Merged
Closed
Merged
Merged
Closed
@github-actions github-actions bot mentioned this issue Jan 16, 2024
Closed
dmitryax pushed a commit that referenced this issue Jan 17, 2024
@crobert-1
[chore] Enable all (passing) goleak checks (#30451)
0f92b21 
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
This PR contains 3 main changes:
1. Make a copy of the same file into every package whose `goleak` check
is currently passing. The only change for the `package_test.go` file in
each package is the package name itself.
2. Rename `processor/tailsamplingprocessor/main_test.go` to
`package_test.go` for uniformity. `package_test.go` is the name we chose
for core (see discussion
[here](open-telemetry/opentelemetry-collector#9173 (comment))).
3. Run `make gotidy` to add `goleak` dependency.

I included the script used to make these changes [in the
issue](#30438 (comment)).

**Link to tracking Issue:** <Issue number if applicable>
#30438
@crobert-1 crobert-1 removed the needs triage New item requiring triage label Jan 17, 2024
mfyuce pushed a commit to mfyuce/opentelemetry-collector-contrib that referenced this issue Jan 18, 2024
@crobert-1
[chore] Enable all (passing) goleak checks (open-telemetry#30451)
a260b46 
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
This PR contains 3 main changes:
1. Make a copy of the same file into every package whose `goleak` check
is currently passing. The only change for the `package_test.go` file in
each package is the package name itself.
2. Rename `processor/tailsamplingprocessor/main_test.go` to
`package_test.go` for uniformity. `package_test.go` is the name we chose
for core (see discussion
[here](open-telemetry/opentelemetry-collector#9173 (comment))).
3. Run `make gotidy` to add `goleak` dependency.

I included the script used to make these changes [in the
issue](open-telemetry#30438 (comment)).

**Link to tracking Issue:** <Issue number if applicable>
open-telemetry#30438
This was referenced Jan 22, 2024
Merged
Closed
Merged
Merged
jpkrohling pushed a commit that referenced this issue Jan 25, 2024
@crobert-1
[chore][exporter/alertmanagerexporter] Enable goleak check (#30710)
2db76a6 
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
Enables goleak check on tests in the `alertmanager` exporter to ensure
it's not leaking goroutines. This is a test only change.

**Link to tracking Issue:** <Issue number if applicable>
#30438

**Testing:** <Describe what testing was performed and which tests were
added.>
All tests are passing, goleak check is passing.
This was referenced Jan 26, 2024
Open
Closed
Merged
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][exporter/prometheusexporter] Enable goleak check (open-teleme…
bb723d3 
…try#30965)

**Description:** 
This enables the `goleak` package for testing the Prometheus exporter.
This helps to ensure no goroutines are leaked by this component. This is
a test only change, the only updates are making sure all tests close the
response body of `Get` calls.

**Link to tracking Issue:**
open-telemetry#30438

**Testing:** 
All tests are passing, including added `goleak` test.
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@iblancasa @rimitchell
[extension/jaegarremotesamplingextension/internal] add goleak tests f…
fcec2a3 
…or jaegerremotesampling extension (open-telemetry#31661)

Related to
open-telemetry#30438
Resolves open-telemetry#31157

Signed-off-by: Israel Blancas <iblancasa@gmail.com>
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][internal/docker] Enable goleak check (open-telemetry#32046)
7ed3c03 
**Description:** 
This enables `goleak` to help ensure no goroutines are being leaked in
the `internal/docker` package. This is a test only change, an existing
test was leaking a goroutine because it wasn't cancelling a context
properly.

**Link to tracking Issue:** open-telemetry#30438 

**Testing:**
Existing tests are passing, as well as added `goleak` check.
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][receiver/snowflake] Enable goleak check (open-telemetry#32182)
f717b4f 
**Description:**
This enables the `goleak` check for the Snowflake receiver to help
ensure no goroutines are being leaked. This is a test only change, a
couple shutdown/close calls were missing.

**Link to tracking Issue:** open-telemetry#30438

**Testing:** 
All existing tests are passing, as well as added `goleak` check.
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][receiver/zookeeper] Enable goleak check (open-telemetry#32179)
466d4e1 
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
This change enables `goleak` to help ensure the ZooKeeper receiver does
not leak any goroutines. This is a test only change, the existing test
implementation needed a bit of modification to properly close the
network listener.

**Link to tracking Issue:** <Issue number if applicable>
open-telemetry#30438

**Testing:** <Describe what testing was performed and which tests were
added.>
Existing tests and new `goleak` check are all passing.
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][receiver/sshcheck] Enable goleak check (open-telemetry#32181)
6e84fc0 
This enables `goleak` checks for the sshcheck receiver to help ensure no
goroutines are being leaked. This is a test only change. The test
package's SSH Server needed to be modified to properly shutdown.

**Link to tracking Issue:** open-telemetry#30438
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][receiver/nginx] Enable goleak check (open-telemetry#32198)
3ff44a8 
Enable `goleak` check for the NGINX receiver to help ensure no
goroutines are being leaked. This is a test only change, a couple test
servers were missing `Close` calls.

**Link to tracking Issue:** open-telemetry#30438
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][receiver/podman] Enable goleak check (open-telemetry#32196)
0508df9 
Enable `goleak` checking on the Podman receiver to help ensure no
goroutines are being leaked. This is a test only change.

**Link to tracking Issue:** open-telemetry#30438
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][receiver/purefa] Enable goleak check (open-telemetry#32194)
b7f83bb 
Enable `goleak` check for the PureStorage Flash Array receiver to help
ensure no goroutines are being leaked. This is a test only change. The
`TestStart` test was missing a shutdown call, and once the shutdown call
was added, it was a duplicate test to `TestShutdown`. I consolidated
these two tests into one, called `TestStartAndShutdown`.

**Link to tracking Issue:** open-telemetry#30438
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][receiver/purefb] Enable goleak check (open-telemetry#32184)
a6f2378 
This change enables `goleak` checks on the Pure Storage FlashBlade
receiver to help ensure no goroutines are being leaked. This is a test
only change as a test was missing a shutdown call.

This looks like I deleted a test, but the two tests were identical,
other than a missing shutdown call. Once the missing call was added they
were identical, so I decided to delete one. I also consolidated err
checking to remove the `err` variable and directly check the result of
the `Start` and `Shutdown` methods.

**Link to tracking Issue:** open-telemetry#30438
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][receiver/nsxt] Enable goleak check (open-telemetry#32197)
ebe14f2 
Enable `goleak` checks for the NSX-T receiver to help ensure no
goroutines are being leaked. This is a test only change, some tests were
missing close calls to servers.

**Link to tracking Issue:** open-telemetry#30438
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][receiver/pulsar] Enable goleak check (open-telemetry#32195)
cfca62e 
Enable `goleak` testing on the Pulsar receiver to help ensure no
goroutines are being leaked.

**Link to tracking Issue:** open-telemetry#30438
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][receiver/saphana] Enable goleak check (open-telemetry#32183)
aa945c5 
Enable `goleak` check for the SAP HANA receiver to help ensure no
goroutines are being leaked.

**Link to tracking Issue:** open-telemetry#30438
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[receiver/jmx] Fix memory leak on shutdown (open-telemetry#32289)
6b1e5b1 
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
The receiver was starting a goroutine that would run without being
stopped during shutdown. This changes the goroutine to be stopped during
shutdown.

`goleak` is also added as a part of this change.

**Link to tracking Issue:** <Issue number if applicable>
open-telemetry#30438

**Testing:** <Describe what testing was performed and which tests were
added.>
All existing tests are passing, as well as added `goleak` check.
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][receiver/googlecloudspannerreceiver] Enable goleak checks (op…
9d9771f 
…en-telemetry#32307)

This is a test only change that enables `goleak` checks on some of the
internal packages of the Google Cloud Spanner receiver to help ensure no
goroutines are leaking. Some tests were missing shutdown calls so those
have been added.

**Link to tracking Issue:** open-telemetry#30438

All existing tests are passing, as well as added `goleak` checks.
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][receiver/googlecloudpubsubreceiver/internal] Enable goleak ch…
06d502a 
…eck (open-telemetry#32311)

Enable `goleak` checks inside the `internal` package of the Google Cloud
PubSub receiver to help ensure no goroutines are leaking. This is a test
only change, some closes/cancels were missing. Also, we need to pass a
context to the grpc dial functionality, so switch from`Dial` ->
`DialContext`. `Dial` is a wrapper for `DialContext` passing in
`context.Background()`.

**Link to tracking Issue:** open-telemetry#30438 

Existing tests are passing, as well as added `goleak` check.
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[receiver/fluentforward] Fix memory leak (open-telemetry#32363)
ee00ca1 
**Description:** 
This fixes a memory leak happening in the Fluent Forward receiver.
`time.Sleep` was being called for a duration of 10 seconds, with no way
to interrupt during shutdown. This updates the functionality to use a
timer that can be stopped in the case of the context being cancelled.

This also enables `goleak` checks on the Fluent Forward receiver to help
ensure no goroutines are being leaked, and adds a missing `Shutdown`
call.

**Link to tracking Issue:** open-telemetry#30438

**Testing:** All existing tests are passing as well as added `goleak` checks.
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][receiver/azureeventhub] Enable goleak check (open-telemetry#3…
55df7d0 
…2364)

**Description:** 
This enables `goleak` checks for the Azure Event Hub receiver to help
ensure no goroutines are being leaked. This is a test only change, some
goroutines were being detected as leaks simply because of the
context.Background lifecycle. Using a cancel shows everything gets
shutdown properly.

**Link to tracking Issue:** open-telemetry#30438 

**Testing:** 
All existing tests are passing, as well as added `goleak` check.
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][receiver/gitproviderreceiver] Enable goleak check in GitHub s…
b913968 
…craper (open-telemetry#32362)

**Description:** 
Enable `goleak` check in the `internal/scraper/githubscraper` package of
the Git Provider receiver to help ensure no goroutines are being leaked.
This is a test only change, a `Close` call was missing on a server in a
test.

**Link to tracking Issue:** open-telemetry#30438 

**Testing:**
All existing tests are passing, as well as added `goleak` check.
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][receiver/namedpipe] Enable goleak check (open-telemetry#32200)
4d4ed86 
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
Add `goleak` check to the namedpipe receiver to help ensure no
goroutines are being leaked.

**Link to tracking Issue:** <Issue number if applicable>
open-telemetry#30438

**Testing:** <Describe what testing was performed and which tests were
added.>
None yet, I'm running on macOS and this is a Linux-only receiver.
Relying on GitHub CI/CD to show results. I'll convert this to ready to
review if it passes.
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[receiver/azureeventhub] Fix memory leak on shutdown (open-telemetry#…
63985fe 
…32401)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
As explained
[here](open-telemetry#30438 (comment)),
components need to cancel all their work and background tasks before
returning from `Shutdown`. My original [change for this
component](open-telemetry#32364)
to add `goleak` testing worked around the proper behavior. This PR fully
addresses the `Shutdown` functionality to ensure the component behaves
the way the spec requires.

**Link to tracking Issue:** <Issue number if applicable>
open-telemetry#30438

**Testing:** <Describe what testing was performed and which tests were
added.>
Tests are passing.
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][receiver/awsxray] Enable goleak checks (open-telemetry#32402)
cdd73df 
Enable `goleak` checks in the `internal/udppoller` package of the AWS
X-Ray receiver to help ensure no goroutines are being leaked. This is a
test only change, simply adding a few missing `Close` calls.

**Link to tracking Issue:** open-telemetry#30438

All existing tests as well as added `goleak` checks are passing.
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][receiver/awscontainerinsight] Enable goleak check (open-telem…
3a55302 
…etry#32462)

**Description:** 
Enable `goleak` checks to help ensure no goroutines are being leaked.
This is a test only change.

**Link to tracking Issue:** open-telemetry#30438

**Testing:** 
All existing tests are passing, as well as added `goleak` check.
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][receiver/apache] Enable goleak check (open-telemetry#32472)
091719f 
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
This enables `goleak` on the Apache receiver to help ensure no
goroutines are leaking. This is a test only change, a test server wasn't
properly being shutdown in a test.

**Link to tracking Issue:** <Issue number if applicable>
open-telemetry#30438

**Testing:** <Describe what testing was performed and which tests were
added.>
All existing tests are passing, as well as added `goleak` checks.
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[internal/metadataproviders/azure] Fix memory leak on AKS (open-telem…
5488865 
…etry#32574)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
From [`client.Do` documentation](https://pkg.go.dev/net/http#Client.Do),
a client's response can safely be ignored when an error is returned, but
must be handled in all other cases, even when a response code is not
2XX. This fixes the internal AKS detection to properly close the
response body in the case of a non-200 status returned.

This also enables `goleak` on the `internal/metadata/azure` package
which helps ensure no goroutines are being leaked. `goleak` is what
detected this bug.

**Link to tracking Issue:** <Issue number if applicable>
open-telemetry#30438

**Testing:** <Describe what testing was performed and which tests were
added.>
All existing tests are passing as well as added `goleak` check.
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[chore][receiver/aerospike] Enable goleak check (open-telemetry#32567)
d31dcb9 
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
Enable `goleak` check on the Aerospike receiver to help ensure no
goroutines are being leaked.

**Link to tracking Issue:** <Issue number if applicable>
open-telemetry#30438
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @codeboten @rimitchell
[processor/remotetap] Fix memory leak on shutdown (open-telemetry#32571)
b30b2da 
The remotetap processor holds a map of channels for writing telemetry
data to WebSockets when a client connects. The write process works by
blocking on a given channel waiting for telemetry data coming through
the processor's pipeline, then writing as soon as it comes in. This
however can result in a leaked goroutine that blocks forever if data is
not received, which is always the case when shutting down.

This fixes the bug by closing and deleting all channels once all client
connections have been shutdown gracefully. This fixes the blocking
behavior during shutdown.

This change also enables `goleak` to help ensure no goroutines are being
leaked.

**Link to tracking Issue:** open-telemetry#30438

---------

Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com>
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @evan-bradley @rimitchell
[internal/common/ttlmap] Fix leak caused by time.Tick (open-telemetry…
afb92f1 
…#32044)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
Documentation shows the existing usage of [`time.Tick` will leak a
goroutine on shutdown](https://pkg.go.dev/time#Tick). This updates the
`internal/common/ttlmap` tick functionality to use a ticker with a
context that has a cancel function. This allows the ticker to be
properly shut down.

I don't believe this requires a changelog because no user-facing
components were updated to actually call the shutdown function. It makes
more sense to me to add those after this gets merged to keep this PR
more concise.

This also adds `goleak` to the package to help ensure goroutines aren't
being leaked.

**Link to tracking Issue:** <Issue number if applicable>
Related to open-telemetry#30438

**Testing:** <Describe what testing was performed and which tests were
added.>
Existing tests are still passing, goleak check was failing before
change, succeeds after.

---------

Co-authored-by: Evan Bradley <11745660+evan-bradley@users.noreply.github.com>
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @codeboten @rimitchell
[chore][processor/groupbytrace] Enable goleak check (open-telemetry#3…
2dd886c 
…2573)

Enable `goleak` checks on the `groupbytrace` processor to help ensure no
goroutines are being leaked. I filed
open-telemetry#32572
to address an existing leak that needs more discussion.

**Link to tracking Issue:** open-telemetry#32572
open-telemetry#30438

---------

Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com>
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
@crobert-1 @rimitchell
[receiver/googlecloudpubsubreceiver] Fix memory leak during shutdown (o…
59ac64d 
…pen-telemetry#32361)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
This PR contains the following changes:
1. Add `Close` call to the receiver's GRPC client. Without this,
goroutines were being leaked on shutdown.
2. Change `grpc.Dial` -> `grpc.NewClient`. They offer the same
functionality, but `Dial` is being deprecated in favor of `NewClient`.
3. Enable `goleak` checks on this receiver to help ensure no goroutines
are being leaked.
4. Change a couple `Assert.Nil` calls to `Assert.NoError`. The output of
`NoError` includes the error message if hit, `Nil` simply includes the
object's address, i.e. `&status.Error{s:(*status.Status)(0xc00007e158)}`

**Link to tracking Issue:** <Issue number if applicable>
open-telemetry#30438

**Testing:** <Describe what testing was performed and which tests were
added.>
All existing tests are passing, as well as added goleak check.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@crobert-1 crobert-1

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@iblancasa @crobert-1