SqlClient: Provide db.system when creating activity (#1979)

open-telemetry · Apr 14, 2021 · 6b7f2dd · 6b7f2dd
1 parent 6a7f321
commit 6b7f2dd
Show file tree

Hide file tree

Showing 6 changed files with 58 additions and 4 deletions.
diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md
@@ -3,6 +3,8 @@
 ## Unreleased
 
 * Instrumentation modified to depend only on the API.
+* Activities are now created with the `db.system` attribute set for usage
+  during sampling. ([#1979](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1979))
 
 ## 1.0.0-rc3
 

diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlActivitySourceHelper.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlActivitySourceHelper.cs
@@ -14,7 +14,9 @@
 // limitations under the License.
 // </copyright>
 using System;
+using System.Collections.Generic;
 using System.Diagnostics;
+using OpenTelemetry.Trace;
 
 namespace OpenTelemetry.Instrumentation.SqlClient.Implementation
 {
@@ -29,6 +31,11 @@ internal class SqlActivitySourceHelper
 
         public const string MicrosoftSqlServerDatabaseSystemName = "mssql";
 
+        public static readonly IEnumerable<KeyValuePair<string, object>> CreationTags = new[]
+        {
+            new KeyValuePair<string, object>(SemanticConventions.AttributeDbSystem, MicrosoftSqlServerDatabaseSystemName),
+        };
+
         private static readonly Version Version = typeof(SqlActivitySourceHelper).Assembly.GetName().Version;
 #pragma warning disable SA1202 // Elements should be ordered by access <- In this case, Version MUST come before ActivitySource otherwise null ref exception is thrown.
         internal static readonly ActivitySource ActivitySource = new ActivitySource(ActivitySourceName, Version.ToString());

diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs
@@ -57,7 +57,12 @@ public override void OnCustom(string name, Activity activity, object payload)
                 case SqlMicrosoftBeforeExecuteCommand:
                     {
                         // SqlClient does not create an Activity. So the activity coming in here will be null or the root span.
-                        activity = SqlActivitySourceHelper.ActivitySource.StartActivity(SqlActivitySourceHelper.ActivityName, ActivityKind.Client);
+                        activity = SqlActivitySourceHelper.ActivitySource.StartActivity(
+                            SqlActivitySourceHelper.ActivityName,
+                            ActivityKind.Client,
+                            default(ActivityContext),
+                            SqlActivitySourceHelper.CreationTags);
+
                         if (activity == null)
                         {
                             // There is no listener or it decided not to sample the current request.
@@ -82,7 +87,6 @@ public override void OnCustom(string name, Activity activity, object payload)
                             _ = this.dataSourceFetcher.TryFetch(connection, out var dataSource);
                             _ = this.commandTextFetcher.TryFetch(command, out var commandText);
 
-                            activity.SetTag(SemanticConventions.AttributeDbSystem, SqlActivitySourceHelper.MicrosoftSqlServerDatabaseSystemName);
                             activity.SetTag(SemanticConventions.AttributeDbName, (string)database);
 
                             this.options.AddConnectionLevelDetailsToActivity((string)dataSource, activity);

diff --git a/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs b/src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlEventSourceListener.netfx.cs
@@ -125,7 +125,12 @@ private void OnBeginExecute(EventWrittenEventArgs eventData)
                 return;
             }
 
-            var activity = SqlActivitySourceHelper.ActivitySource.StartActivity(SqlActivitySourceHelper.ActivityName, ActivityKind.Client);
+            var activity = SqlActivitySourceHelper.ActivitySource.StartActivity(
+                SqlActivitySourceHelper.ActivityName,
+                ActivityKind.Client,
+                default(ActivityContext),
+                SqlActivitySourceHelper.CreationTags);
+
             if (activity == null)
             {
                 // There is no listener or it decided not to sample the current request.
@@ -138,7 +143,6 @@ private void OnBeginExecute(EventWrittenEventArgs eventData)
 
             if (activity.IsAllDataRequested)
             {
-                activity.SetTag(SemanticConventions.AttributeDbSystem, SqlActivitySourceHelper.MicrosoftSqlServerDatabaseSystemName);
                 activity.SetTag(SemanticConventions.AttributeDbName, databaseName);
 
                 this.options.AddConnectionLevelDetailsToActivity((string)eventData.Payload[1], activity);

diff --git a/...etry.Instrumentation.SqlClient.Tests/OpenTelemetry.Instrumentation.SqlClient.Tests.csproj b/...etry.Instrumentation.SqlClient.Tests/OpenTelemetry.Instrumentation.SqlClient.Tests.csproj
@@ -10,6 +10,7 @@
     <Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Shared\SkipUnlessEnvVarFoundTheoryAttribute.cs" Link="SkipUnlessEnvVarFoundTheoryAttribute.cs" />
     <Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Shared\EventSourceTestHelper.cs" Link="EventSourceTestHelper.cs" />
     <Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Shared\TestEventListener.cs" Link="TestEventListener.cs" />
+    <Compile Include="$(RepoRoot)\test\OpenTelemetry.Tests\Shared\TestSampler.cs" Link="TestSampler.cs" />
   </ItemGroup>
 
   <ItemGroup>

diff --git a/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs b/test/OpenTelemetry.Instrumentation.SqlClient.Tests/SqlClientTests.cs
@@ -86,8 +86,10 @@ public void SuccessfulCommandTest(
             bool shouldEnrich = true)
         {
             var activityProcessor = new Mock<BaseProcessor<Activity>>();
+            var sampler = new TestSampler();
             using var shutdownSignal = Sdk.CreateTracerProviderBuilder()
                 .AddProcessor(activityProcessor.Object)
+                .SetSampler(sampler)
                 .AddSqlClientInstrumentation(options =>
                 {
 #if !NETFRAMEWORK
@@ -135,6 +137,7 @@ public void SuccessfulCommandTest(
             var activity = (Activity)activityProcessor.Invocations[1].Arguments[0];
 
             VerifyActivityData(commandType, commandText, captureStoredProcedureCommandName, captureTextCommandContent, isFailure, recordException, dataSource, activity);
+            VerifySamplingParameters(sampler.LatestSamplingParameters);
         }
 
         // DiagnosticListener-based instrumentation is only available on .NET Core
@@ -280,6 +283,30 @@ public void SqlClientErrorsAreCollectedSuccessfully(string beforeCommand, string
                 sqlConnection.DataSource,
                 (Activity)processor.Invocations[2].Arguments[0]);
         }
+
+        [Theory]
+        [InlineData(SqlClientDiagnosticListener.SqlDataBeforeExecuteCommand)]
+        [InlineData(SqlClientDiagnosticListener.SqlMicrosoftBeforeExecuteCommand)]
+        public void SqlClientCreatesActivityWithDbSystem(
+            string beforeCommand)
+        {
+            using var sqlConnection = new SqlConnection(TestConnectionString);
+            using var sqlCommand = sqlConnection.CreateCommand();
+
+            var sampler = new TestSampler
+            {
+                SamplingAction = _ => new SamplingResult(SamplingDecision.Drop),
+            };
+            using (Sdk.CreateTracerProviderBuilder()
+                .AddSqlClientInstrumentation()
+                .SetSampler(sampler)
+                .Build())
+            {
+                this.fakeSqlClientDiagnosticSource.Write(beforeCommand, new { });
+            }
+
+            VerifySamplingParameters(sampler.LatestSamplingParameters);
+        }
 #endif
 
         private static void VerifyActivityData(
@@ -351,6 +378,15 @@ private static void VerifyActivityData(
             Assert.Equal(dataSource, activity.GetTagValue(SemanticConventions.AttributePeerService));
         }
 
+        private static void VerifySamplingParameters(SamplingParameters samplingParameters)
+        {
+            Assert.NotNull(samplingParameters.Tags);
+            Assert.Contains(
+                samplingParameters.Tags,
+                kvp => kvp.Key == SemanticConventions.AttributeDbSystem
+                       && (string)kvp.Value == SqlActivitySourceHelper.MicrosoftSqlServerDatabaseSystemName);
+        }
+
         private static void ActivityEnrichment(Activity activity, string method, object obj)
         {
             switch (method)