Remove memory leak at exporter shutdown

[madaraszg-tulip]

Description

Fix memory leak at exporter shutdown. At startup time the exporter creates metric callbacks that hold references to the exporter queue, these need to be unregistered at shutdown.

Link to tracking issue

Fixes #11401

[linux-foundation-easycla]

• ❌ - login: @madaraszg-tulip / name: Gergely Madarász . The commit (011d62e, c42de6c, ae1515d, a98ddce) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

[dmathieu]

You will need to sign the CLA.

[madaraszg-tulip]

You will need to sign the CLA.

Yep, waiting for our CLA manager to approve

[codecov]

Codecov Report

Attention: Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 91.61%. Comparing base (f74890a) to head (f592107).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...terhelper/internal/metadata/generated_telemetry.go	75.00%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #11745   +/-   ##
=======================================
  Coverage   91.61%   91.61%           
=======================================
  Files         443      443           
  Lines       23770    23783   +13     
=======================================
+ Hits        21776    21789   +13     
  Misses       1620     1620           
  Partials      374      374           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

---

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests

[jpkrohling]

I think we have the same pattern elsewhere, using io.Closer instead. I think we could use the same here, with the added benefit that we can keep the error returned by Unregister.

[madaraszg-tulip]

I was actually looking for an established pattern, but didn't find one. Can you point me to it? I searched for io.Closer in the code, and found only one reference which is unrelated.

[jpkrohling]

Interestingly, I wasn't able to find one example either in this repo :-) There is exactly one usage of this pattern in contrib.

We do have a func Shutdown(context.Context) error as part of our component.Component interface, but I'd argue in favor of io.Closer in this case.

[dehaansa]

This was the only thing I found that was in the same neighborhood: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/processor/logstransformprocessor/processor.go#L66

[madaraszg-tulip]

So where can I actually see that io.Closer use? I found the component.ShutdownFunc thing, but not the other one

[jpkrohling]

I misread the code in contrib, it was only a type check 🤦🏽 I scratched something here, but I think I prefer your approach, perhaps with the only improvement that it could return an error, to be logged on your iterator.

[madaraszg-tulip]

I'm now logging (warn level) if there's any error unregistering the callbacks.