SQL Collection fix (#1292)

* Fix 1291. SQL Listener had references to System.Data types in Microsoft.Data path. The class is modified to NOT import the system.data namespace now to ensure we rely purely on reflection. This should have been done in the original fix which would have caught this issue there itself.
microsoft · Oct 15, 2019 · 2825af8 · 2825af8
1 parent 1ef3d16
commit 2825af8
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,8 @@
 # Changelog
 
+## Version 2.11.2
+- [Fix Sql dependency collection bug in .NET Core 3.0 with Microsoft.Data.SqlClient.](https://github.com/Microsoft/ApplicationInsights-dotnet-server/issues/1291)
+
 ## Version 2.11.1
 - [Fix Sql dependency parent id to match W3CTraceContext format](https://github.com/Microsoft/ApplicationInsights-dotnet-server/issues/1277)
 - [Fix EventCounters so that it appear as CustomMetrics as opposed to PerformanceCounters.](https://github.com/Microsoft/ApplicationInsights-dotnet-server/issues/1280)

diff --git a/GlobalStaticVersion.props b/GlobalStaticVersion.props
@@ -7,7 +7,7 @@
     -->
     <SemanticVersionMajor>2</SemanticVersionMajor>
     <SemanticVersionMinor>11</SemanticVersionMinor>
-    <SemanticVersionPatch>1</SemanticVersionPatch>
+    <SemanticVersionPatch>2</SemanticVersionPatch>
     <!--Valid values: beta1, beta2, EMPTY for stable -->
     <PreReleaseMilestone></PreReleaseMilestone>
     <!-- 

diff --git a/...DependencyCollector/Shared.Tests/Implementation/SqlClientDiagnosticSourceListenerTests.cs b/...DependencyCollector/Shared.Tests/Implementation/SqlClientDiagnosticSourceListenerTests.cs
@@ -198,6 +198,53 @@ public void TracksCommandExecuted(string beforeCommand, string afterCommand)
             Assert.True(Math.Abs((start - dependencyTelemetry.Timestamp).TotalMilliseconds) <= 16);
         }
 
+        [Theory]
+        [InlineData(SqlClientDiagnosticSourceListener.SqlBeforeExecuteCommand, SqlClientDiagnosticSourceListener.SqlAfterExecuteCommand)]
+        [InlineData(SqlClientDiagnosticSourceListener.SqlMicrosoftBeforeExecuteCommand, SqlClientDiagnosticSourceListener.SqlMicrosoftAfterExecuteCommand)]
+        public void TracksCommandExecutedSP(string beforeCommand, string afterCommand)
+        {
+            var operationId = Guid.NewGuid();
+            var sqlConnection = new SqlConnection(TestConnectionString);
+            var sqlCommand = sqlConnection.CreateCommand();
+            sqlCommand.CommandText = "SP_GetOrders";
+            sqlCommand.CommandType = CommandType.StoredProcedure;
+
+            var beforeExecuteEventData = new
+            {
+                OperationId = operationId,
+                Command = sqlCommand,
+                Timestamp = (long?)1000000L
+            };
+
+            this.fakeSqlClientDiagnosticSource.Write(
+                beforeCommand,
+                beforeExecuteEventData);
+            var start = DateTimeOffset.UtcNow;
+
+            var afterExecuteEventData = new
+            {
+                OperationId = operationId,
+                Command = sqlCommand,
+                Timestamp = 2000000L
+            };
+
+            this.fakeSqlClientDiagnosticSource.Write(
+                afterCommand,
+                afterExecuteEventData);
+
+            var dependencyTelemetry = (DependencyTelemetry)this.sendItems.Single();
+
+            Assert.Equal(beforeExecuteEventData.OperationId.ToString("N"), dependencyTelemetry.Id);
+            Assert.Equal(sqlCommand.CommandText, dependencyTelemetry.Data);
+            Assert.Equal("(localdb)\\MSSQLLocalDB | master | SP_GetOrders", dependencyTelemetry.Name);
+            Assert.Equal("(localdb)\\MSSQLLocalDB | master", dependencyTelemetry.Target);
+            Assert.Equal(RemoteDependencyConstants.SQL, dependencyTelemetry.Type);
+            Assert.True((bool)dependencyTelemetry.Success);
+            Assert.True(dependencyTelemetry.Duration > TimeSpan.Zero);
+            Assert.True(dependencyTelemetry.Duration < TimeSpan.FromMilliseconds(500));
+            Assert.True(Math.Abs((start - dependencyTelemetry.Timestamp).TotalMilliseconds) <= 16);
+        }
+
         [Theory]
         [InlineData(SqlClientDiagnosticSourceListener.SqlBeforeExecuteCommand, SqlClientDiagnosticSourceListener.SqlAfterExecuteCommand)]
         [InlineData(SqlClientDiagnosticSourceListener.SqlMicrosoftBeforeExecuteCommand, SqlClientDiagnosticSourceListener.SqlMicrosoftAfterExecuteCommand)]

diff --git a/...cyCollector/Shared/Implementation/SqlClientDiagnostics/SqlClientDiagnosticFetcherTypes.cs b/...cyCollector/Shared/Implementation/SqlClientDiagnostics/SqlClientDiagnosticFetcherTypes.cs
@@ -11,7 +11,7 @@ internal static class SqlClientDiagnosticFetcherTypes
         //// http://github.com/dotnet/corefx/blob/master/src/System.Data.SqlClient/src/System/Data/SqlClient/SqlClientDiagnosticListenerExtensions.cs
         //// https://github.com/dotnet/SqlClient/blob/master/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlClientDiagnosticListenerExtensions.cs
 
-#region "System.Data fetchers"
+        #region "System.Data fetchers"
 
         /// <summary> Fetchers for execute command before event. </summary>
         internal static class CommandBefore
@@ -41,6 +41,7 @@ internal static class CommandError
             public static readonly PropertyFetcher Command = new PropertyFetcher(nameof(Command));
             public static readonly PropertyFetcher Exception = new PropertyFetcher(nameof(Exception));
             public static readonly PropertyFetcher Timestamp = new PropertyFetcher(nameof(Timestamp));
+            public static readonly PropertyFetcher Number = new PropertyFetcher(nameof(Number));
         }
 
         /// <summary> Fetchers for connection open/close before events. </summary>
@@ -68,6 +69,7 @@ internal static class ConnectionError
             public static readonly PropertyFetcher Connection = new PropertyFetcher(nameof(Connection));
             public static readonly PropertyFetcher Exception = new PropertyFetcher(nameof(Exception));
             public static readonly PropertyFetcher Timestamp = new PropertyFetcher(nameof(Timestamp));
+            public static readonly PropertyFetcher Number = new PropertyFetcher(nameof(Number));
         }
 
         /// <summary> Fetchers for transaction commit before events. </summary>
@@ -118,6 +120,7 @@ internal static class TransactionRollbackError
             public static readonly PropertyFetcher Connection = new PropertyFetcher(nameof(Connection));
             public static readonly PropertyFetcher Exception = new PropertyFetcher(nameof(Exception));
             public static readonly PropertyFetcher Timestamp = new PropertyFetcher(nameof(Timestamp));
+            public static readonly PropertyFetcher Number = new PropertyFetcher(nameof(Number));
         }
 
         /// <summary> Fetchers for transaction commit error events. </summary>
@@ -127,9 +130,10 @@ internal static class TransactionCommitError
             public static readonly PropertyFetcher Connection = new PropertyFetcher(nameof(Connection));
             public static readonly PropertyFetcher Exception = new PropertyFetcher(nameof(Exception));
             public static readonly PropertyFetcher Timestamp = new PropertyFetcher(nameof(Timestamp));
+            public static readonly PropertyFetcher Number = new PropertyFetcher(nameof(Number));
         }
         #endregion
-        
+
         #region "Microsoft.Data fetchers"
 
         /// <summary> Fetchers for execute command before event. </summary>
@@ -160,6 +164,7 @@ internal static class CommandErrorMicrosoft
             public static readonly PropertyFetcher Command = new PropertyFetcher(nameof(Command));
             public static readonly PropertyFetcher Exception = new PropertyFetcher(nameof(Exception));
             public static readonly PropertyFetcher Timestamp = new PropertyFetcher(nameof(Timestamp));
+            public static readonly PropertyFetcher Number = new PropertyFetcher(nameof(Number));
         }
 
         /// <summary> Fetchers for connection open/close before events. </summary>
@@ -187,6 +192,7 @@ internal static class ConnectionErrorMicrosoft
             public static readonly PropertyFetcher Connection = new PropertyFetcher(nameof(Connection));
             public static readonly PropertyFetcher Exception = new PropertyFetcher(nameof(Exception));
             public static readonly PropertyFetcher Timestamp = new PropertyFetcher(nameof(Timestamp));
+            public static readonly PropertyFetcher Number = new PropertyFetcher(nameof(Number));
         }
 
         /// <summary> Fetchers for transaction commit before events. </summary>
@@ -237,6 +243,7 @@ internal static class TransactionRollbackErrorMicrosoft
             public static readonly PropertyFetcher Connection = new PropertyFetcher(nameof(Connection));
             public static readonly PropertyFetcher Exception = new PropertyFetcher(nameof(Exception));
             public static readonly PropertyFetcher Timestamp = new PropertyFetcher(nameof(Timestamp));
+            public static readonly PropertyFetcher Number = new PropertyFetcher(nameof(Number));
         }
 
         /// <summary> Fetchers for transaction commit error events. </summary>
@@ -246,6 +253,7 @@ internal static class TransactionCommitErrorMicrosoft
             public static readonly PropertyFetcher Connection = new PropertyFetcher(nameof(Connection));
             public static readonly PropertyFetcher Exception = new PropertyFetcher(nameof(Exception));
             public static readonly PropertyFetcher Timestamp = new PropertyFetcher(nameof(Timestamp));
+            public static readonly PropertyFetcher Number = new PropertyFetcher(nameof(Number));
         }
         #endregion
     }