PoC | Remove Abstractions dependency on Logging by edwardneal · Pull Request #4024 · dotnet/SqlClient

edwardneal · 2026-03-10T07:01:00Z

Description

This is a proof of concept, largely to review the approach and to jump-start any discussions.

7.0 introduces the Abstractions and the Logging packages. Slightly unusually though, Abstractions depends upon Logging rather than vice versa. This is because the authentication provider manager needs to log - but it means that a fairly generic package is taking a hard dependency on a specific logging method. It also means that we're exposing SqlClientEventSource to API consumers.

This PR changes the dependency tree: Abstractions now depends upon Microsoft.Extensions.Logging.Abstractions instead of Logging. Logging no longer exposes SqlClientEventSource - instead, it exposes a new SqlClientEventSourceLoggerProvider class which implements ILoggerProvider. ILoggerProvider.CreateLogger returns an ILogger implementor for trace logging (and eventually for SNI trace logging, pool logging, etc.)

A general-purpose Logger class introduces the TraceLogger property, and this lazy-loads SqlClientEventSourceLoggerProvider using the same basic logic as the authentication provider does (with the same requirement to change the loading behaviour in GA.)

The primary point is to expose an ILogger abstraction rather than the implementation details of one logging method, and to keep Abstractions abstract. Subsequent work could eventually allow users to supply their own ILogger implementation (for example, to integrate with OpenTelemetry - we already have issues open for metrics and traces, so this would pre-empt a request for logs.)

A secondary benefit is being able to use source-generated log messages and gather the hardcoded logging strings together. I've used these in LogMessages.cs in a few places.

@paulmedynski - this provides a more concrete proposal than the comments I made in #3982.

Issues

None.

Testing

The Azure unit tests work and I can see that the logging code is triggered, but I've not spent a great deal of time on the rest of the unit tests; if people think this makes sense, this PR can be split up into a few others which are easier to review and test,

Include this in the Abstractions and the AzureKeyVaultProvider projects.

Copilot

Pull request overview

This PR is a proof-of-concept refactor of SqlClient’s extensions logging architecture to reduce coupling: moving Abstractions away from a direct dependency on the Logging package and introducing an ILogger-based surface (including source-generated log messages) while keeping existing EventSource-based tracing.

Changes:

Introduces a shared Logger helper that lazy-loads a SqlClientEventSourceLoggerProvider (via reflection) and exposes a TraceLogger.
Adds SqlClientEventSourceLoggerProvider + TraceLogger in the Logging package and migrates Azure/Akv providers to use ILogger source-generated messages.
Adds SqlClientDiagnostics bridge for metrics enablement and links SqlClientEventSource.cs into driver projects.

Reviewed changes

Copilot reviewed 22 out of 23 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientDiagnostics.cs	Adds internal bridge wiring metrics enablement to EventSource enablement.
src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj	Links `SqlClientEventSource.cs` from Extensions.Logging into the main driver build.
src/Microsoft.Data.SqlClient/netfx/src/Microsoft.Data.SqlClient.csproj	Adjusts legacy netfx project compile items to use `SqlClientDiagnostics` and link `SqlClientEventSource.cs`.
src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj	Adjusts legacy netcore project compile items to use `SqlClientDiagnostics` and link `SqlClientEventSource.cs`.
src/Microsoft.Data.SqlClient/add-ons/AzureKeyVaultProvider/SqlColumnEncryptionAzureKeyVaultProvider.cs	Replaces direct EventSource calls with `ILogger`-based logging + member scopes.
src/Microsoft.Data.SqlClient/add-ons/AzureKeyVaultProvider/Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider.csproj	Links shared `Logger.cs` into the AKV provider project.
src/Microsoft.Data.SqlClient/add-ons/AzureKeyVaultProvider/LogMessages.cs	Adds source-generated log message definitions for AKV provider.
src/Microsoft.Data.SqlClient/add-ons/AzureKeyVaultProvider/LocalCache.cs	Replaces trace strings with `ILogger` message methods.
src/Microsoft.Data.SqlClient/add-ons/AzureKeyVaultProvider/AzureSqlKeyCryptographer.cs	Replaces trace strings with `ILogger` message methods.
src/Microsoft.Data.SqlClient.Extensions/Logging/src/SqlClientEventSourceLoggerProvider.cs	Adds `ILoggerProvider` that returns category-specific loggers (currently Trace).
src/Microsoft.Data.SqlClient.Extensions/Logging/src/SqlClientEventSource.cs	Makes EventSource internal and adds an async-safe scope type + enablement callback.
src/Microsoft.Data.SqlClient.Extensions/Logging/src/Logging.csproj	Makes Logging depend on Abstractions (project/package) instead of being depended on.
src/Microsoft.Data.SqlClient.Extensions/Logging/src/EventSourceCategories/TraceLogger.cs	Adds an `ILogger` implementation that writes to SqlClient’s EventSource.
src/Microsoft.Data.SqlClient.Extensions/Common/Abstractions/Logging/Logger.cs	Adds shared helper for lazy-loading the logging provider and exposing `TraceLogger`.
src/Microsoft.Data.SqlClient.Extensions/Azure/src/LogMessages.cs	Adds source-generated log message definitions for Azure auth provider.
src/Microsoft.Data.SqlClient.Extensions/Azure/src/Azure.csproj	Links shared `Logger.cs` into the Azure extensions project.
src/Microsoft.Data.SqlClient.Extensions/Azure/src/ActiveDirectoryAuthenticationProvider.cs	Replaces EventSource string logs with source-generated `ILogger` message methods.
src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/SqlAuthenticationProvider.Internal.cs	Removes direct EventSource dependency; routes diagnostics through `Logger.TraceLogger`.
src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/Logging/LoggerExtensions.cs	Adds `ILogger` extension for member scopes.
src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/Logging/LogMessages.cs	Adds source-generated log message definitions for Abstractions internals.
src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/Logging/CategoryNames.cs	Adds public category name constants for SqlClient logging.
src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/Abstractions.csproj	Switches Abstractions to depend on `Microsoft.Extensions.Logging.Abstractions` and links shared `Logger.cs`.
Directory.Packages.props	Adds `Microsoft.Extensions.Logging.Abstractions` version pin and updates `System.Diagnostics.DiagnosticSource`.

Copilot · 2026-03-10T08:21:45Z

src/Microsoft.Data.SqlClient.Extensions/Logging/src/SqlClientEventSource.cs

    /// ETW EventSource for Microsoft.Data.SqlClient tracing and diagnostics.
    /// </summary>
    [EventSource(Name = "Microsoft.Data.SqlClient.EventSource")]
-    public class SqlClientEventSource : EventSource
+    internal sealed class SqlClientEventSource : EventSource
    {


SqlClientEventSource was changed from public to internal. If any external consumers have taken a dependency on Microsoft.Data.SqlClient.Extensions.Logging.SqlClientEventSource, this is a breaking change for that package and should be reflected in versioning/release notes (or an alternative public surface should be provided).

The breaking change is fair, and part of the reason why I've pushed this forwards before the 7.0 GA is caught. I've seen some discussion about exempting SqlClientEventSource from the usual breaking change rules though, which might make the rush unnecessary.

Copilot · 2026-03-10T08:21:46Z

src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/Logging/LoggerExtensions.cs

+using System.Runtime.CompilerServices;
+using Microsoft.Extensions.Logging;
+
+namespace Microsoft.Data.SqlClient.Extensions.Abstractions.Logging;


This new source file is missing the standard .NET Foundation/MIT license header that appears at the top of other files in the repo. Please add the repo’s standard header before the first using directive.

Copilot · 2026-03-10T08:21:46Z

src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/Logging/LoggerExtensions.cs

+public static class LoggerExtensions
+{
+    /// <summary>
+    /// Begins a logical operation scope, defined by the class name, calling member and a scope identifier.
+    /// </summary>
+    /// <param name="logger"><see cref="ILogger"/> instance to use to begin the scope.</param>
+    /// <param name="className">Class containing the calling member.</param>
+    /// <param name="memberName">Name of the calling member.</param>
+    /// <returns>An <see cref="IDisposable"/> that ends the logical operation scope on dispose.</returns>
+    public static IDisposable? BeginMemberScope(this ILogger logger, string className, [CallerMemberName] string memberName = "") =>
+        logger.BeginScope($"{className}.{memberName} | INFO | SCOPE | Entering Scope {{0}}");
+}


BeginMemberScope is a public extension on ILogger, but it relies on SqlClient’s EventSource scope formatting behavior (embedding a "{0}" placeholder for a later scopeId substitution). With a normal ILogger implementation, this will likely produce confusing scopes that literally contain "{0}". Consider making this extension internal, or changing it to use a provider-agnostic scope state (e.g., structured values) that doesn’t rely on a second formatting pass.

Copilot · 2026-03-10T08:21:46Z

src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/Logging/LogMessages.cs

+using Microsoft.Extensions.Logging;
+
+namespace Microsoft.Data.SqlClient.Extensions.Abstractions.Logging;
+


This new source file is missing the standard .NET Foundation/MIT license header that appears at the top of other files in the repo. Please add the repo’s standard header before the first using directive.

Copilot · 2026-03-10T08:21:47Z

src/Microsoft.Data.SqlClient.Extensions/Common/Abstractions/Logging/Logger.cs

+                Assembly assembly = Assembly.Load(AssemblyName);
+
+                if (assembly is null)
+                {
+                    Debug.Fail($"MDS assembly={AssemblyName} not found; " +


Assembly.Load(...) never returns null (it either returns an Assembly instance or throws). The null-check that follows is unreachable and can be removed/simplified.

Copilot · 2026-03-10T08:21:47Z

src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj

+  <ItemGroup>
+    <Compile Include="$(RepoRoot)\src\Microsoft.Data.SqlClient.Extensions\Logging\src\SqlClientEventSource.cs">
+      <Link>Microsoft\Data\SqlClient\SqlClientEventSource.cs</Link>
+    </Compile>
+  </ItemGroup>


The main driver now compiles SqlClientEventSource.cs from the Extensions.Logging project. Since the Logging package also compiles that same file, an app that loads both assemblies can end up with two EventSource types using the same [EventSource(Name = "Microsoft.Data.SqlClient.EventSource")], which can throw at runtime when the second EventSource instance is created. Consider ensuring SqlClientEventSource is defined in exactly one assembly (e.g., have the Logging package reference the driver and use InternalsVisibleTo, or move EventSource into a shared assembly) rather than compiling it into both.

Copilot · 2026-03-10T08:21:47Z

src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj

+    <Compile Include="$(RepoRoot)\src\Microsoft.Data.SqlClient.Extensions\Logging\src\SqlClientEventSource.cs">
+      <Link>Microsoft\Data\SqlClient\SqlClientEventSource.cs</Link>
+    </Compile>


This Include path uses Windows-style backslashes ("$(RepoRoot)\src...") while most other projects use forward slashes. Using backslashes here can break builds on non-Windows agents if the resulting path doesn't resolve correctly. Prefer "$(RepoRoot)/src/..." for cross-platform consistency.

edwardneal added 10 commits March 8, 2026 20:19

Rename source file

8448030

Reference SqlClientEventSource from SqlClient directly

8fc206e

Add new SqlClientEventAsyncScope

a053532

Add initial logging implementation for generic tracing

4bbf86b

Create Lazy-loading Logger class

4af8e8a

Include this in the Abstractions and the AzureKeyVaultProvider projects.

Convert Abstractions logging to source-generated LoggerMessages

facb959

Convert Azure logging to source-generated LoggerMessages

24879bb

Convert AzureKeyVaultProvider logging to source-generated LoggerMessages

67a2982

Convert AzureKeyVaultProvider to use BeginScope

c3d3f38

Do not throw exception when tests are run in debug

bc76a5f

edwardneal requested a review from a team as a code owner March 10, 2026 07:01

github-project-automation bot added this to SqlClient Board Mar 10, 2026

github-project-automation bot moved this to To triage in SqlClient Board Mar 10, 2026

apoorvdeshmukh requested a review from Copilot March 10, 2026 08:14

Copilot started reviewing on behalf of apoorvdeshmukh March 10, 2026 08:15 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

apoorvdeshmukh self-assigned this Mar 10, 2026

apoorvdeshmukh moved this from To triage to In review in SqlClient Board Mar 10, 2026

apoorvdeshmukh added this to the 7.1.0-preview1 milestone Mar 10, 2026

cheenamalhotra added the Breaking Change 🔨 Issues/PRs that are related with breaking API changes in the driver. label Mar 11, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PoC | Remove Abstractions dependency on Logging#4024

PoC | Remove Abstractions dependency on Logging#4024
edwardneal wants to merge 10 commits intodotnet:mainfrom
edwardneal:poc/abstractions-ilogger

edwardneal commented Mar 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

edwardneal Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Copilot AI Mar 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		using Microsoft.Extensions.Logging;

		namespace Microsoft.Data.SqlClient.Extensions.Abstractions.Logging;

Conversation

edwardneal commented Mar 10, 2026

Description

Issues

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

edwardneal Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants