Introduce Logging package and rework official pipelines by cheenamalhotra · Pull Request #3967 · dotnet/SqlClient

cheenamalhotra · 2026-02-19T18:17:25Z

Summary

Moves the ETW SqlClientEventSource tracing infrastructure (~1,180 lines) from the core driver into a new Microsoft.Data.SqlClient.Extensions.Logging (netstandard2.0) NuGet package. This enables independent versioning and shared use across SqlClient and the AKV provider (which had its own AKVEventSource, now deleted).

Key Changes

Area	What Changed
New package	`Microsoft.Data.SqlClient.Extensions.Logging` — contains `SqlClientEventSource`, `SqlClientMetrics`, `SqlClientEventScope`, and scope types
Core driver	`SqlClientEventSource.cs` reduced to a ~43-line `SqlClientDiagnostics` bridge; 28 source files updated from `TryEventScope`/`TrySNIEventScope` → `SqlClientEventScope`
AKV provider	Deleted `AKVEventSource.cs`; AKV now uses the shared `SqlClientEventSource.Log` from the Logging package
Build	`build.proj` gains Logging targets; nuspec files add Logging dependency across all TFMs
Pipelines	Consolidated into unified OneBranch official/non-official pipelines with selective build params, approval service, ESRP signing for Logging, reusable publish job with dry-run; deleted 9 standalone pipeline/variable files
Docs	Added `PackageReadme.md` for all 6 packages; new OneBranch pipeline design spec (522 lines); updated `AGENTS.md`

Checklist

No breaking changes — logging behavior preserved via the extracted package as a dependency

…gned DLLs).

Copilot

Pull request overview

Copilot reviewed 99 out of 99 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 100 out of 100 changed files in this pull request and generated 3 comments.

Copilot · 2026-02-20T06:15:16Z

src/Microsoft.Data.SqlClient.Extensions/Abstractions/src/AbstractionsVersions.props

    <_OurAssemblyFileVersion Condition="'$(AbstractionsAssemblyFileVersion)' == '' and '$(AbstractionsPackageVersion)' != ''">$(AbstractionsPackageVersion.Split('-')[0])</_OurAssemblyFileVersion>
    <!-- If neither property is set, use the default value.-->
-    <_OurAssemblyFileVersion Condition="'$(AbstractionsAssemblyFileVersion)' == '' and '$(AbstractionsPackageVersion)' == ''">$(AbstractionsDefaultMajorVersion).0.0.$(BuildNumber)</_OurAssemblyFileVersion>
+    <_OurAssemblyFileVersion Condition="'$(AbstractionsAssemblyFileVersion)' == '' and '$(AbstractionsPackageVersion)' == ''">$(AbstractionsDefaultMajorVersion).0.0.$(AssemblyBuildNumber)</_OurAssemblyFileVersion>


The AssemblyBuildNumber property is used here but is defined in tools/props/Versions.props. This creates an import order dependency: Versions.props must be imported before this file for AssemblyBuildNumber to be defined. Verify that the import order in all consuming projects ensures Versions.props is imported first, or consider defining a fallback value here if AssemblyBuildNumber is empty.

Copilot · 2026-02-20T06:15:17Z

src/Microsoft.Data.SqlClient.Extensions/Azure/src/AzureVersions.props

    <_OurAssemblyFileVersion Condition="'$(AzureAssemblyFileVersion)' == '' and '$(AzurePackageVersion)' != ''">$(AzurePackageVersion.Split('-')[0])</_OurAssemblyFileVersion>
    <!-- If neither property is set, use the default value.-->
-    <_OurAssemblyFileVersion Condition="'$(AzureAssemblyFileVersion)' == '' and '$(AzurePackageVersion)' == ''">$(AzureDefaultMajorVersion).0.0.$(BuildNumber)</_OurAssemblyFileVersion>
+    <_OurAssemblyFileVersion Condition="'$(AzureAssemblyFileVersion)' == '' and '$(AzurePackageVersion)' == ''">$(AzureDefaultMajorVersion).0.0.$(AssemblyBuildNumber)</_OurAssemblyFileVersion>


The AssemblyBuildNumber property is used here but is defined in tools/props/Versions.props. This creates an import order dependency: Versions.props must be imported before this file for AssemblyBuildNumber to be defined. Verify that the import order in all consuming projects ensures Versions.props is imported first, or consider defining a fallback value here if AssemblyBuildNumber is empty.

Copilot · 2026-02-20T06:15:17Z

src/Microsoft.Data.SqlClient.Extensions/Logging/src/LoggingVersions.props

+    <!-- If LoggingPackageVersion is set, use its trimmed value. -->
+    <_OurAssemblyFileVersion Condition="'$(LoggingAssemblyFileVersion)' == '' and '$(LoggingPackageVersion)' != ''">$(LoggingPackageVersion.Split('-')[0])</_OurAssemblyFileVersion>
+    <!-- If neither property is set, use the default value.-->
+    <_OurAssemblyFileVersion Condition="'$(LoggingAssemblyFileVersion)' == '' and '$(LoggingPackageVersion)' == ''">$(LoggingDefaultMajorVersion).0.0.$(AssemblyBuildNumber)</_OurAssemblyFileVersion>


The AssemblyBuildNumber property is used here but is defined in tools/props/Versions.props. This creates an import order dependency: Versions.props must be imported before this file for AssemblyBuildNumber to be defined. Verify that the import order in all consuming projects ensures Versions.props is imported first, or consider defining a fallback value here if AssemblyBuildNumber is empty.

Copilot

Pull request overview

Copilot reviewed 121 out of 122 changed files in this pull request and generated 2 comments.

Copilot · 2026-02-20T17:25:18Z

Directory.Packages.props

+    <PackageVersion
+      Include="Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider"
+      Version="$(AkvPackageVersion)" />
    <PackageVersion
      Include="Microsoft.Data.SqlClient.Extensions.Abstractions"
      Version="$(AbstractionsPackageVersion)" />
    <PackageVersion
      Include="Microsoft.Data.SqlClient.Extensions.Azure"
      Version="$(AzurePackageVersion)" />
    <PackageVersion
-      Include="Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider"
-      Version="$(AkvPackageVersion)" />
+      Include="Microsoft.Data.SqlClient.Extensions.Logging"
+      Version="$(LoggingPackageVersion)" />


The package ordering has been changed to place Microsoft.Data.SqlClient.AlwaysEncrypted.AzureKeyVaultProvider before the Extensions packages. For consistency with the rest of the file (alphabetical ordering within logical groups), consider either keeping the original order or documenting the reason for this specific ordering if it's intentional.

Copilot · 2026-02-20T17:25:18Z

build.proj

-  <Target Name="BuildAll" DependsOnTargets="BuildNetFx;BuildNetCore;BuildNetStandard" />
-  <Target Name="BuildAllConfigurations" DependsOnTargets="Restore;BuildTools;BuildSqlServerLib;BuildNetFx;BuildNetCoreAllOS;BuildNetStandard;GenerateMdsPackage" />
+  <Target Name="BuildAll" DependsOnTargets="BuildNetFx;BuildNetCore;BuildNetStandard;BuildLogging;BuildAzure" />
+  <Target Name="BuildAllConfigurations" DependsOnTargets="Restore;BuildTools;BuildSqlServerLib;BuildNetFx;BuildNetCoreAllOS;BuildNetStandard;BuildLogging;BuildSqlServerPackage;GenerateMdsPackage" />


The PackAbstractions target was split out from BuildAbstractions, but the BuildAllConfigurations target chain doesn't appear to call PackAbstractions, PackLogging, or PackAzure. This means the Pack step won't run during BuildAllConfigurations. Verify whether this is intentional or if these Pack targets should be added to the dependency chain.

Suggested change

<Target Name="BuildAllConfigurations" DependsOnTargets="Restore;BuildTools;BuildSqlServerLib;BuildNetFx;BuildNetCoreAllOS;BuildNetStandard;BuildLogging;BuildSqlServerPackage;GenerateMdsPackage" />

<Target Name="BuildAllConfigurations" DependsOnTargets="Restore;BuildTools;BuildSqlServerLib;BuildNetFx;BuildNetCoreAllOS;BuildNetStandard;BuildLogging;BuildSqlServerPackage;GenerateMdsPackage;PackAbstractions;PackLogging;PackAzure" />

Copilot

Pull request overview

Copilot reviewed 121 out of 122 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 125 out of 126 changed files in this pull request and generated 3 comments.

Copilot · 2026-02-20T19:28:20Z

eng/pipelines/common/templates/jobs/build-signed-package-job.yml

  - template: /eng/pipelines/common/templates/steps/copy-dlls-for-test-step.yml@self
+    parameters:
+      softwareFolder: '${{ parameters.softwareFolder }}'
+      symbolsFolder: '${{ parameters.symbolsFolder }}'


The step at line 113 includes a comment "Copy DLLs and PDBs to the software and symbols folders for APIScan and testing." but the template invocation doesn't show any actual copy operations visible in this diff. Verify that the compound-step template at copy-dlls-for-test-step.yml actually performs the copy operations needed for APIScan and that the softwareFolder and symbolsFolder parameters are correctly wired through.

Copilot · 2026-02-20T19:28:21Z

eng/pipelines/steps/compound-esrp-dll-signing-step.yml

@@ -43,8 +49,9 @@ steps:
      Pattern: '${{ parameters.pattern }}'


The ESRP task versions were upgraded from v5 to v6 (EsrpMalwareScanning@6 and EsrpCodeSigning@6). Ensure that the new task versions are compatible with the current OneBranch pipeline configuration and that all required parameters are still supported. Review the breaking changes documentation for ESRP v5→v6 migration.

Copilot · 2026-02-20T19:28:21Z

eng/pipelines/common/templates/jobs/build-signed-package-job.yml

+- job: build_package_SqlClient
+  displayName: 'Build Microsoft.Data.SqlClient'


The job name changed from build_signed_package to build_package_SqlClient, but validate-signed-package-job.yml references the old job name via stageDependencies.build_dependent.build_package_SqlClient. However, the stage name appears to be buildMDS in the old code (line 35 shows the old reference). Verify that the stage name has also been updated to build_dependent in the calling pipeline, otherwise this job dependency will fail at runtime.

- Replaced copy scripts with CopyFiles@2 tasks.

mdaigle

It may work as a whole, but this is a tough one to review because there are so many ideas combined into one changeset. I'd really like to see pieces broken out so that they can be effectively reviewed.

mdaigle · 2026-02-20T23:02:24Z

eng/pipelines/common/templates/jobs/build-signed-package-job.yml

+  - task: DownloadPipelineArtifact@2
+    displayName: 'Download Abstractions Package'
+    inputs:
+      artifactName: drop_build_independent_build_package_Abstractions


Probably best to have these come in as parameters. We don't have control over this artifact name and it could change in the future. The package step for the independent packages should output these values and then the pipeline should pass them in to this step. The ci-build-nugets-job.yml file is already doing this.

mdaigle · 2026-02-20T23:08:59Z

eng/pipelines/common/templates/steps/configure-sql-server-step.yml

    type: string
    default: MSSQLSERVER

+  # User.


None of these comments are really adding any value and they're greatly expanding the diff.

mdaigle · 2026-02-20T23:16:50Z

eng/pipelines/jobs/pack-logging-package-ci-job.yml

+        parameters:
+          debug: ${{ parameters.debug }}
+
+      # We use the 'custom' command because the DotNetCoreCLI@2 task doesn't


I'm a little suspicious of this. It's a big pain to maintain multi-step builds all over the place. All of these supporting packages, should be buildable with a single dotnet pack command.

mdaigle · 2026-02-20T23:21:24Z

eng/pipelines/jobs/pack-logging-package-ci-job.yml

+        value: >-
+          $(commonArguments)
+          --configuration ${{ parameters.buildConfiguration }}
+          -p:LoggingPackageVersion=${{ parameters.loggingPackageVersion }}


✏️ I think all of these package jobs could be factored out into a generic package job that takes package and assembly file version numbers as params. We could avoid a ton of extra stage and job files.

mdaigle · 2026-02-20T23:34:56Z

tools/specs/add-ons/PackageReadme.md

Should move to src folder like other readmes

mdaigle · 2026-02-20T23:35:41Z

tools/specs/PackageReadme.SqlServer.Server.md

should move to src folder with other readmes

- Fixed inconsistent/inaccurate official pipeline variable names.

Copilot

Pull request overview

Copilot reviewed 124 out of 125 changed files in this pull request and generated no new comments.

cheenamalhotra added 30 commits February 19, 2026 09:55

Initialize Logging package

88b5e91

Update Event Scope type names

ed287b5

Include Logging in package ref

f49e21a

Cleanup pipelines, add non-official as well

9a412a8

Include SNK signing key

c629a27

Rename pipeline

8aba060

Fix keywords

8c14916

Fix pipelines to not release if no release job is selected

4ae5f15

Notification based release stage + fix build number format

8b4ab2e

Conditional execution

991e2af

Add back the revision number decimal

2a20bcd

Update ESRP version + split build number

f1a52b6

Fix the build job to build and pack separately (as we need to pack si…

a5518e6

…gned DLLs).

Fix for APIScan failure, ensure DLLs are copied first.

7c651a4

Include readme files in NuGet packages

2f60302

Fix APIScan issue

0a19676

Fix build error and APIScan errors

69b55b5

download packages before building dependent packages

8c3a2b0

Fix azure project build by providing abstractions package version

f95382a

Rename file to Readme in all packages

e86f3f1

Enable APIScan and include instructions for onebranch pipelines.

0507f98

Pass effective logging package version to AKV build

eecfc11

Fix

71fa038

Include ob_outputDirectory variable, and other fixes

8fc0ec4

Implement approval service

95a1e5b

Configure ob_release params for approval service

24b64f7

Skip building packages when not requested.

c3f9d85

Split environments (prod v/s non-prod)

648a3e6

Touch-ups to job names

2f7a410

Fix release stage to be deployment job

2cedac3

cheenamalhotra force-pushed the dev/cheena/logging-package branch from 2d6835f to ec31d8d Compare February 20, 2026 04:58

Copilot started reviewing on behalf of cheenamalhotra February 20, 2026 04:58 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

cheenamalhotra added 2 commits February 19, 2026 21:17

Fix CI pipelines and remove unwanted pipeline

646e00b

Use build number as is

d2498bd

Copilot AI review requested due to automatic review settings February 20, 2026 06:11

Copilot started reviewing on behalf of cheenamalhotra February 20, 2026 06:12 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

paulmedynski added 2 commits February 20, 2026 09:15

Addressed a bunch of my comments in the PR.

cf4c99f

Further changes related to PR comments.

7129067

Copilot AI review requested due to automatic review settings February 20, 2026 17:22

Copilot started reviewing on behalf of paulmedynski February 20, 2026 17:23 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

paulmedynski added 2 commits February 20, 2026 13:45

Fixed missing assemblyBuildNumber argument.

a8f5bb2

Fixed another missing assemblyBuildNumber.

6e02918

Copilot AI review requested due to automatic review settings February 20, 2026 17:49

Copilot started reviewing on behalf of paulmedynski February 20, 2026 17:49 View session

Copilot AI reviewed Feb 20, 2026

View reviewed changes

paulmedynski added 2 commits February 20, 2026 14:27

Fixed expression variable expansion of build number.

569c902

Fixed reflection bug in the tests.

3f35416

Copilot AI review requested due to automatic review settings February 20, 2026 19:25

Copilot started reviewing on behalf of paulmedynski February 20, 2026 19:26 View session

Fixed stage name typo.

cbaf0f6

Copilot AI reviewed Feb 20, 2026

View reviewed changes

- Removed redundant SqlServer package templates.

e475f86

- Replaced copy scripts with CopyFiles@2 tasks.

mdaigle requested changes Feb 20, 2026

View reviewed changes

- Removed special build/pack setup for SqlServer package.

c3b9941

- Fixed inconsistent/inaccurate official pipeline variable names.

Copilot AI review requested due to automatic review settings February 22, 2026 22:09

Copilot started reviewing on behalf of paulmedynski February 22, 2026 22:10 View session

Copilot AI reviewed Feb 22, 2026

View reviewed changes

	<Target Name="BuildAllConfigurations" DependsOnTargets="Restore;BuildTools;BuildSqlServerLib;BuildNetFx;BuildNetCoreAllOS;BuildNetStandard;BuildLogging;BuildSqlServerPackage;GenerateMdsPackage" />
	<Target Name="BuildAllConfigurations" DependsOnTargets="Restore;BuildTools;BuildSqlServerLib;BuildNetFx;BuildNetCoreAllOS;BuildNetStandard;BuildLogging;BuildSqlServerPackage;GenerateMdsPackage;PackAbstractions;PackLogging;PackAzure" />

		@@ -43,8 +49,9 @@ steps:
		Pattern: '${{ parameters.pattern }}'

		- job: build_package_SqlClient
		displayName: 'Build Microsoft.Data.SqlClient'

Comments

Conversation

cheenamalhotra commented Feb 19, 2026

Summary

Key Changes

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

mdaigle left a comment

Choose a reason for hiding this comment

Uh oh!

mdaigle Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

mdaigle Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

mdaigle Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

mdaigle Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

mdaigle Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

mdaigle Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects