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]

The committers listed above are authorized under a signed CLA.

• ✅ login: madaraszg-tulip / name: Gergely Madarász (b302466, 1947f66, 080ecc2, d35c022, b237843, 65a424d, bed9d1a)
• ✅ login: codeboten / name: Alex Boten (8c7ef05)

[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 82.35294% with 6 lines in your changes missing coverage. Please review.

Project coverage is 91.54%. Comparing base (52d8a1a) to head (8c7ef05).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
exporter/exporterhelper/internal/queue_sender.go	85.71%	2 Missing and 1 partial ⚠️
...terhelper/internal/metadata/generated_telemetry.go	75.00%	2 Missing ⚠️
...ereceiver/internal/metadata/generated_telemetry.go	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #11745      +/-   ##
==========================================
- Coverage   91.55%   91.54%   -0.01%     
==========================================
  Files         445      445              
  Lines       23715    23733      +18     
==========================================
+ Hits        21712    21727      +15     
- Misses       1627     1629       +2     
- Partials      376      377       +1     

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

[jpkrohling]

@codeboten , I think you might be the right person to review and merge this :-)

[codeboten]

thanks @mahadzaryab1! is it possible to add a test to validate the shutdown behaviour in the queue sender?

I'm curious if there's a way we could prevent these in the future, it doesnt look like goleak is detecting this today

[madaraszg-tulip]

thanks @mahadzaryab1!

I assume this was supposed to be me :D

is it possible to add a test to validate the shutdown behaviour in the queue sender?

Will look into this. As far as I can tell, the callback is called during collection cycle, so if we trigger a metric collection after queue shutdown, we should not be seeing the metrics.

I'm curious if there's a way we could prevent these in the future, it doesnt look like goleak is detecting this today

I have no idea, I'm relatively new to go, and this was my first ever memory leak investigation.

[madaraszg-tulip]

is it possible to add a test to validate the shutdown behaviour in the queue sender?

Will look into this. As far as I can tell, the callback is called during collection cycle, so if we trigger a metric collection after queue shutdown, we should not be seeing the metrics.

I have now added a check in the queue sender metrics test to make sure the metrics are unregistered.

[codeboten]

Thanks for addressing my feedback 👍🏻