census-ecosystem / opencensus-go-exporter-stackdriver

OpenCensus Go exporter for Stackdriver Monitoring and Trace
Apache License 2.0
67 stars 79 forks source link

Add the ability to close metric and trace clients #295

Closed dashpole closed 2 years ago

dashpole commented 2 years ago

The OpenTelemetry-collector googlecloud exporter component uses this exporter. If the exporter is Shutdown, it currently only calls StopMetricsExporter and Flush, which just stops the opencensus interval reader. This leaks GRPC client connections, and causes the collector's memory usage to increase.

dashpole commented 2 years ago

This isn't related to any thing we've seen recently.

I was playing around with the WatchableMapProvider interface in the collector, and noticed it OOMing frequently when I updated the collector config repeatedly. Memory metrics showed an increase in memory usage when the collector's config changed. To debug, I added the pprof extension, and looked at the heap. It showed most of the memory being used by grpc's newHTTP2Client function. Googling told me that it was likely because of grpc client connections not being closed, which made me look here. I found that we weren't closing them, and made this change. I then re-built the collector on top of this commit and verified that the memory leak was gone.

punya commented 2 years ago

@dashpole please address the lint failures:

staticcheck ./...
stackdriver.go:443:10: error strings should not be capitalized (ST1005)