Skip to content

Conversation

@edwardneal
Copy link
Contributor

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 Log will run OnEventCommand with an enablement command before it finishes executing. This enablement tries to call Metrics.EnableEventCounters, but this hasn't been initialized yet - it relies on the SqlClientEventSource instance.

To break this cycle, we record the desired startup state of the metrics in this case, and pass it to the SqlClientMetrics constructor.

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

dotnet counters monitor --counters Microsoft.Data.SqlClient.EventSource -- SqlClientTimeQuery.exe

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.

@edwardneal edwardneal requested a review from a team as a code owner October 24, 2025 16:34
@cheenamalhotra cheenamalhotra added this to the 6.1.3 milestone Oct 24, 2025
@cheenamalhotra
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@codecov
Copy link

codecov bot commented Oct 24, 2025

Codecov Report

❌ Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.32%. Comparing base (7a7b54e) to head (9bb2e60).
⚠️ Report is 17 commits behind head on release/6.1.

Files with missing lines Patch % Lines
...oft/Data/SqlClient/Diagnostics/SqlClientMetrics.cs 66.66% 1 Missing ⚠️
...c/Microsoft/Data/SqlClient/SqlClientEventSource.cs 80.00% 1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (7a7b54e) and HEAD (9bb2e60). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (7a7b54e) HEAD (9bb2e60)
addons 1 0
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     
Flag Coverage Δ
addons ?
netcore 63.84% <75.00%> (-8.92%) ⬇️
netfx 60.97% <100.00%> (-8.27%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cheenamalhotra cheenamalhotra merged commit b1bf339 into dotnet:release/6.1 Oct 28, 2025
236 checks passed
@edwardneal edwardneal deleted the port/pr-3575/release-6.1 branch October 28, 2025 16:03
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.

3 participants