-
Notifications
You must be signed in to change notification settings - Fork 318
[6.1] Backport initial metrics enablement #3718
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
[6.1] Backport initial metrics enablement #3718
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/6.1 #3718 +/- ##
================================================
- Coverage 69.69% 59.32% -10.37%
================================================
Files 281 273 -8
Lines 62413 61434 -979
================================================
- Hits 43500 36448 -7052
- Misses 18913 24986 +6073
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
This backports 66a67bc from #3575 to 6.1.
It's possible to start an application with the M.D.S EventSource already enabled, even before the static constructor for SqlClientEventSource has run. In this situation, the constructor for
Logwill runOnEventCommandwith an enablement command before it finishes executing. This enablement tries to callMetrics.EnableEventCounters, but this hasn't been initialized yet - it relies on theSqlClientEventSourceinstance.To break this cycle, we record the desired startup state of the metrics in this case, and pass it to the
SqlClientMetricsconstructor.Issues
Fixes #3711.
Testing
The timing makes this difficult to test. In the end, I built a test application locally, referenced 6.1.2 and ran
This allowed me to reproduce the problem: the counters appear in 6.0, don't appear in 6.1.2, appear again in 7.0-preview2 and now also appear in my local build.
Could someone run CI please? @empowerRob you should be able to verify with the nupkg build artifact.