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.

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

[BUG] Is ClientDiagnostics's DiagnosticListener leaking? #18199

Closed
tg-msft opened this issue Jan 26, 2021 · 3 comments · Fixed by #18206
Closed

[BUG] Is ClientDiagnostics's DiagnosticListener leaking? #18199

tg-msft opened this issue Jan 26, 2021 · 3 comments · Fixed by #18206
Assignees
Labels
Azure.Core bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library.

Comments

@tg-msft
Copy link
Member

tg-msft commented Jan 26, 2021

An internal partner is seeing a leak caused by:

Should we add a destructor on DiagnosticScopeFactory that disposes its listener?

@tg-msft tg-msft added Client This issue points to a problem in the data-plane of the library. Azure.Core bug This issue requires a change to an existing behavior in the product in order to be resolved. labels Jan 26, 2021
@pakrym
Copy link
Contributor

pakrym commented Jan 26, 2021

@pakrym
Copy link
Contributor

pakrym commented Jan 26, 2021

We can go about it in two ways:

  1. Add a finalizer to the DiagnosticScopeFactory
  2. Cache DiagnosticListener in a dictionary based on the namespace.

The latter would cause fewer allocations per-client instance and I suspect we'll need a similar thing for ActivitySource support pretty soon as well.

@kasobol-msft
Copy link
Contributor

Azure Storage SDK included this fix in the release today. Package versions can be found here

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Core bug This issue requires a change to an existing behavior in the product in order to be resolved. Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants