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

Disposing of DependencyTrackingTelemetryModule before host is disposed #7448

Merged
merged 3 commits into from
Jun 10, 2021

Conversation

brettsam
Copy link
Member

@brettsam brettsam commented Jun 10, 2021

Note -- I originally made a commit has the fix commented out so I can verify it fails in CI. It failed 100% of the time locally.

This fixes the error:

The specified configuration does not have a telemetry channel. (Parameter 'configuration')

Tactical change to fix a race condition during disposal of the DependencyTrackingTelemetryModule. It must be disposed before TelemetryConfiguration. This change explicitly disposes it before disposing the host.

This issue is fixed in ApplicationInsights here -- microsoft/ApplicationInsights-dotnet#2294 -- but this won't be released until 2.0.18. Once we pull that version, we can delete this fix (but keep the test).

@brettsam brettsam requested a review from gzuber June 10, 2021 15:01
@brettsam
Copy link
Member Author

Related to #7446.

/cc @cijothomas

@brettsam brettsam requested review from fabiocav and ankitkumarr June 10, 2021 15:03
@brettsam
Copy link
Member Author

cijothomas
cijothomas previously approved these changes Jun 10, 2021
fabiocav
fabiocav previously approved these changes Jun 10, 2021
Copy link
Member

@fabiocav fabiocav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks good to me, but could we add a link to an issue tracking the permanent fix so we can reference that in the future when reviewing this code?

@brettsam brettsam dismissed stale reviews from fabiocav and cijothomas via fb24b5c June 10, 2021 17:02
@brettsam brettsam merged commit bc4cddd into dev Jun 10, 2021
@brettsam brettsam deleted the brettsam/dependency-fix branch June 10, 2021 18:47
@alejandromelis
Copy link

@brettsam @fabiocav

When do you expect this workaround to be released?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants