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

Add missing Microsoft.Data.SqlClient instrumentation for .NET Framework #1248

Merged
merged 5 commits into from
Sep 23, 2022

Conversation

JcolemanNR
Copy link
Contributor

@JcolemanNR JcolemanNR commented Sep 22, 2022

Description

Our existing Microsoft.Data.SqlClient instrumentation only had test coverage for .NET Core. This allowed a bug to slip under the radar where not all forms of ExecuteReader and ExecuteReaderAsync were properly profiled in .NET Framework. The existing instrumentation points worked fine for .NET Core, but the Microsoft Implementation for .NET Framework and .NET Core differ. A new internal form of ExecuteReaderAsync, and a new form of ExecuteReader are now included in the instrumentation XML for Microsoft.Data.SqlClient. The newly added integration tests will break if the new instrumentation entries are removed, so there is high confidence that this resolves the issues with missing instrumentation reported by customers.

Closes #620.

Author Checklist

Reviewer Checklist

  • Perform code review
  • Pull request was adequately tested (new/existing tests, performance tests)
  • Review Changelog

…for .NET Framework AsyncParameterizedQuery tests
… remaining test fixtures, fix formatting in all MSSQL tests
chynesNR
chynesNR previously approved these changes Sep 22, 2022
chynesNR
chynesNR previously approved these changes Sep 22, 2022
@JcolemanNR JcolemanNR merged commit d01d701 into main Sep 23, 2022
@nr-ahemsath nr-ahemsath deleted the jc/missing-sql-instrumentation branch April 24, 2023 17:31
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.

Missing Sql Instrumentation
2 participants