[Instrumentation.SqlClient] Introducing context propagation to SQL Server

[sincejune]

…on to propagate trace info to SQL Server

Changes

This PR introduces a new flag, ContextPropagationLevel, in SqlClientInstrumentation, which enables the library to propagate traceparent information to the SQL Server database.

The ContextPropagationLevel can be set to either trace or disabled (the default value). When set to trace, the instrumentation library uses the SET CONTEXT_INFO command to pass a traceparent ID of the current Activity to the SQL Server.

Next steps:

I plan to introduce a "service" level that will pass service information via SQL commands via sqlcommenter.

Merge requirement checklist

• CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
• Unit tests updated: Enable this flag by default in integration tests
• Appropriate CHANGELOG.md files updated for non-trivial changes
• Changes in public API reviewed (if applicable)

[codecov]

Codecov Report

Attention: Patch coverage is 91.42857% with 3 lines in your changes missing coverage. Please review.

Project coverage is 88.48%. Comparing base (8e317c1) to head (4ca9eab).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...lementation/SqlClientInstrumentationEventSource.cs	50.00%	2 Missing ⚠️
...ient/Implementation/SqlClientDiagnosticListener.cs	96.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2709       +/-   ##
===========================================
+ Coverage   69.74%   88.48%   +18.73%     
===========================================
  Files         394        9      -385     
  Lines       15980      495    -15485     
===========================================
- Hits        11146      438    -10708     
+ Misses       4834       57     -4777     

Flag	Coverage Δ
unittests-Contrib.Shared.Tests	?
unittests-Exporter.Geneva	?
unittests-Exporter.InfluxDB	?
unittests-Exporter.Instana	?
unittests-Exporter.OneCollector	?
unittests-Exporter.Stackdriver	?
unittests-Extensions	?
unittests-Extensions.Enrichment	?
unittests-Instrumentation.AWS	?
unittests-Instrumentation.AspNet	?
unittests-Instrumentation.AspNetCore	?
unittests-Instrumentation.ConfluentKafka	?
unittests-Instrumentation.ElasticsearchClient	?
unittests-Instrumentation.EntityFrameworkCore	?
unittests-Instrumentation.EventCounters	?
unittests-Instrumentation.GrpcCore	?
unittests-Instrumentation.GrpcNetClient	?
unittests-Instrumentation.Hangfire	?
unittests-Instrumentation.Http	?
unittests-Instrumentation.Owin	?
unittests-Instrumentation.Process	?
unittests-Instrumentation.Quartz	?
unittests-Instrumentation.Runtime	?
unittests-Instrumentation.ServiceFabricRemoting	?
unittests-Instrumentation.SqlClient	88.48% <91.42%> (+0.22%)	⬆️
unittests-Instrumentation.StackExchangeRedis	?
unittests-Instrumentation.Wcf	?
unittests-PersistentStorage	?
unittests-Resources.AWS	?
unittests-Resources.Azure	?
unittests-Resources.Container	?
unittests-Resources.Gcp	?
unittests-Resources.Host	?
unittests-Resources.OperatingSystem	?
unittests-Resources.Process	?
unittests-Resources.ProcessRuntime	?
unittests-Sampler.AWS	?

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

Files with missing lines	Coverage Δ
....SqlClient/SqlClientTraceInstrumentationOptions.cs	100.00% <100.00%> (ø)
...ient/Implementation/SqlClientDiagnosticListener.cs	88.60% <96.00%> (+1.38%)	⬆️
...lementation/SqlClientInstrumentationEventSource.cs	67.85% <50.00%> (-2.98%)	⬇️

... and 385 files with indirect coverage changes

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Kielek]

@sincejune, could you please rise issue under semantic convention repository? It is not something we should accept without common standard for all supported languages (including other db clients).

[sincejune]

@sincejune, could you please rise issue under semantic convention repository? It is not something we should accept without common standard for all supported languages (including other db clients).

@Kielek Sure, filed open-telemetry/semantic-conventions#2162

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[github-actions]

Closed as inactive. Feel free to reopen if this PR is still being worked on.

[Kielek]

Reopening, per request in private channel. open-telemetry/semantic-conventions#2363 is now merged

[Kielek]

There is some issues on .NET Framework 4.6.2 (locally executed) in integration tests
2 cases are failing on

        if (commandText == GetContextInfoQuery)
        {
            Assert.NotEqual(commandResult, DBNull.Value); // <---- failing here
            Assert.True(commandResult is byte[]);
            var contextInfo = Encoding.ASCII.GetString((byte[])commandResult).TrimEnd('\0');
            Assert.Equal(contextInfo, activity.Id);
        }

.NET 8 and 9 are passing.

[martincostello]

There is some issues on .NET Framework 4.6.2 (locally executed) in integration tests

Do the .NET Framework tests not run in CI (I thought they did)? That might explain why #2826 (comment) happened.

[Kielek]

There is some issues on .NET Framework 4.6.2 (locally executed) in integration tests

Do the .NET Framework tests not run in CI (I thought they did)? That might explain why #2826 (comment) happened.

It requires docker for linux. It is not available on the GH runners on Windows systems. So itnegration tests are not executed for such cases.

[cuichenli]

There is some issues on .NET Framework 4.6.2 (locally executed) in integration tests

The current implementation does not yet support .NET Framework. We may need to explore how to integrate it later.

opentelemetry-dotnet-contrib/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientInstrumentation.cs

Lines 51 to 55 in 07105a5

	#if NETFRAMEWORK
	this.sqlEventSourceListener = new SqlEventSourceListener();
	#else
	this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(
	name => new SqlClientDiagnosticListener(name),

The logic is currently implemented in the SqlClientDiagnosticListener.cs file, but it's only used when running on .NET.
Given that, the most straightforward solution for now is to disable the context info check in .NET Framework tests.

Would that be acceptable? If not, we can continue working on support in this PR. Otherwise, we can defer it to a follow-up PR. Thanks!

[YayBurritos]

@sincejune , @alanwest , @Kielek : Any concerns over using SET CONTEXT_INFO with connection pooling? Is there a risk of the TraceId that's propagated "contaminating" unrelated SQL queries? It's great that the instrumentation library provides support for TraceId propagation, but I'd like to have some confidence that if our applications use this, we can rely on the TraceIds associated with our SQL queries to be accurate. Any info you can provide would be appreciated. Thanks!