Adding a flag to enable/disable collection of SQL Command text

CHANGELOG.md

@@ -1,7 +1,8 @@
 # Changelog
 
 ## VNext
-
+- [Adding a flag to DependencyTrackingTelemetryModule to enable/disable collection of SQL Command text.](https://github.com/microsoft/ApplicationInsights-dotnet/pull/1514)
+   Collecting SQL Command Text will now be opt-in, so this value will default to false. This is a change from the current behavior on .NET Core. To see how to collect SQL Command Text see here for details: https://docs.microsoft.com/en-us/azure/azure-monitor/app/asp-net-dependencies#advanced-sql-tracking-to-get-full-sql-query
 
 ## Version 2.13.0-beta2
 - [Move FileDiagnosticTelemetryModule to Microsoft.ApplicationInsights assembly.](https://github.com/microsoft/ApplicationInsights-dotnet/issues/1059)

WEB/Src/DependencyCollector/Shared.Tests/Implementation/ProfilerSqlProcessingTest.cs

@@ -43,6 +43,7 @@ public sealed class ProfilerSqlProcessingTest
         private TelemetryConfiguration configuration;
         private List<ITelemetry> sendItems;
         private ProfilerSqlCommandProcessing sqlCommandProcessingProfiler;
+        private ProfilerSqlCommandProcessing sqlCommandProcessingProfilerWithDisabledCommandText;
         private ProfilerSqlConnectionProcessing sqlConnectionProcessingProfiler;
 
         [TestInitialize]
@@ -53,6 +54,8 @@ public void TestInitialize()
             this.configuration.TelemetryChannel = new StubTelemetryChannel { OnSend = item => this.sendItems.Add(item) };
             this.configuration.InstrumentationKey = Guid.NewGuid().ToString();
             this.sqlCommandProcessingProfiler = new ProfilerSqlCommandProcessing(this.configuration, null, new ObjectInstanceBasedOperationHolder());
+            this.sqlCommandProcessingProfiler = new ProfilerSqlCommandProcessing(this.configuration, null, new ObjectInstanceBasedOperationHolder(), true);
+            this.sqlCommandProcessingProfilerWithDisabledCommandText = new ProfilerSqlCommandProcessing(this.configuration, null, new ObjectInstanceBasedOperationHolder(), false);
             this.sqlConnectionProcessingProfiler = new ProfilerSqlConnectionProcessing(this.configuration, null, new ObjectInstanceBasedOperationHolder());
         }
 
@@ -303,6 +306,30 @@ public void CommandNameForNonStoredProcIsCollectedCorrectly()
             Assert.AreEqual(command.CommandText, actualCommandName);
         }
 
+        [TestMethod]
+        public void CommandNameForStoredProcIsCollectedCorrectly()
+        {
+            SqlCommand command = GetSqlCommandTestForStoredProc();
+            string actualCommandName = this.sqlCommandProcessingProfiler.GetCommandName(command);
+            Assert.AreEqual(command.CommandText, actualCommandName);
+        }
+
+        [TestMethod]
+        public void CommandNameForNonStoredProcIsNotCollectedWhenDisabled()
+        {
+            SqlCommand command = GetMoreComplexSqlCommandTestForQuery();
+            string actualCommandName = this.sqlCommandProcessingProfilerWithDisabledCommandText.GetCommandName(command);
+            Assert.AreEqual(string.Empty, actualCommandName);
+        }
+
+        [TestMethod]
+        public void CommandNameForStoredProcIsNotCollectedWhenDisabled()
+        {
+            SqlCommand command = GetSqlCommandTestForStoredProc();
+            string actualCommandName = this.sqlCommandProcessingProfilerWithDisabledCommandText.GetCommandName(command);
+            Assert.AreEqual(string.Empty, actualCommandName);
+        }
+
         [TestMethod]
         public void GetCommandNameReturnsEmptyStringForNullSqlCommand()
         {

WEB/Src/DependencyCollector/Shared.Tests/Implementation/SqlClientDiagnosticSourceListenerTests.cs

@@ -41,7 +41,7 @@ public SqlClientDiagnosticSourceListenerTests()
             };
 
             this.fakeSqlClientDiagnosticSource = new FakeSqlClientDiagnosticSource();
-            this.sqlClientDiagnosticSourceListener = new SqlClientDiagnosticSourceListener(this.configuration);
+            this.sqlClientDiagnosticSourceListener = new SqlClientDiagnosticSourceListener(this.configuration, true);
         }
 
         public void Dispose()
@@ -605,7 +605,7 @@ public void MultiHost_OneListenerIsActive()
                 Timestamp = 2000000L
             };
 
-            using (var listener2 = new SqlClientDiagnosticSourceListener(this.configuration))
+            using (var listener2 = new SqlClientDiagnosticSourceListener(this.configuration, true))
             {
                 this.fakeSqlClientDiagnosticSource.Write(
                     SqlClientDiagnosticSourceListener.SqlBeforeExecuteCommand,
@@ -619,6 +619,46 @@ public void MultiHost_OneListenerIsActive()
             }
         }
 
+        [Fact]
+        public void DoesNotTrackCommandTextWhenDisabled()
+        {
+            var operationId = Guid.NewGuid();
+            var sqlConnection = new SqlConnection(TestConnectionString);
+            var sqlCommand = sqlConnection.CreateCommand();
+            sqlCommand.CommandText = "select * from orders";
+
+            var beforeExecuteEventData = new
+            {
+                OperationId = operationId,
+                Command = sqlCommand,
+                Timestamp = (long?)1000000L
+            };
+
+            var afterExecuteEventData = new
+            {
+                OperationId = operationId,
+                Command = sqlCommand,
+                Timestamp = 2000000L
+            };
+
+            this.sqlClientDiagnosticSourceListener.Dispose();
+
+            using (var listener2 = new SqlClientDiagnosticSourceListener(this.configuration, false))
+            {
+                this.fakeSqlClientDiagnosticSource.Write(
+                    SqlClientDiagnosticSourceListener.SqlBeforeExecuteCommand,
+                    beforeExecuteEventData);
+
+                this.fakeSqlClientDiagnosticSource.Write(
+                    SqlClientDiagnosticSourceListener.SqlAfterExecuteCommand,
+                    afterExecuteEventData);
+
+                Assert.Equal(1, this.sendItems.Count(t => t is DependencyTelemetry));
+                var telemetry = this.sendItems[0] as DependencyTelemetry;
+                Assert.Equal(string.Empty, telemetry.Data);
+            }
+        }
+
         [Fact]
         public void MultiHost_OneListenerThenAnotherIsActive()
         {
@@ -653,7 +693,7 @@ public void MultiHost_OneListenerThenAnotherIsActive()
 
             this.sqlClientDiagnosticSourceListener.Dispose();
 
-            using (var listener = new SqlClientDiagnosticSourceListener(this.configuration))
+            using (var listener = new SqlClientDiagnosticSourceListener(this.configuration, true))
             {
                 this.fakeSqlClientDiagnosticSource.Write(
                     SqlClientDiagnosticSourceListener.SqlBeforeExecuteCommand,
@@ -689,7 +729,7 @@ public void MultiHost_OneListnerIsActiveAfterDispose()
                 Timestamp = 2000000L
             };
 
-            using (var listener2 = new SqlClientDiagnosticSourceListener(this.configuration))
+            using (var listener2 = new SqlClientDiagnosticSourceListener(this.configuration, true))
             {
                 this.fakeSqlClientDiagnosticSource.Write(
                     SqlClientDiagnosticSourceListener.SqlBeforeExecuteCommand,

WEB/Src/DependencyCollector/Shared.Tests/Implementation/SqlClientDiagnosticSourceListenerTestsCopy.cs

@@ -2,18 +2,16 @@ namespace Microsoft.ApplicationInsights.Tests
 {
     using System;
     using System.Collections.Generic;
-    using System.Data;
     using System.Data.SqlClient;
     using System.Diagnostics;
     using System.Linq;
     using System.Reflection;
+
     using Microsoft.ApplicationInsights.Channel;
     using Microsoft.ApplicationInsights.DataContracts;
-    using Microsoft.ApplicationInsights.DependencyCollector.Implementation;
     using Microsoft.ApplicationInsights.DependencyCollector.Implementation.SqlClientDiagnostics;
     using Microsoft.ApplicationInsights.Extensibility;
     using Microsoft.ApplicationInsights.Extensibility.Implementation.Tracing;
-    using Microsoft.ApplicationInsights.W3C.Internal;
     using Microsoft.ApplicationInsights.Web.TestFramework;
     using Microsoft.VisualStudio.TestTools.UnitTesting;
 
@@ -23,7 +21,7 @@ namespace Microsoft.ApplicationInsights.Tests
     public class SqlClientDiagnosticSourceListenerTestsCopy : IDisposable
     {
         private const string TestConnectionString = "Data Source=(localdb)\\MSSQLLocalDB;Database=master";
-        
+
         private IList<ITelemetry> sendItems;
         private StubTelemetryChannel stubTelemetryChannel;
         private TelemetryConfiguration configuration;
@@ -42,7 +40,7 @@ public SqlClientDiagnosticSourceListenerTestsCopy()
             };
 
             this.fakeSqlClientDiagnosticSource = new FakeSqlClientDiagnosticSource();
-            this.sqlClientDiagnosticSourceListener = new SqlClientDiagnosticSourceListener(this.configuration);
+            this.sqlClientDiagnosticSourceListener = new SqlClientDiagnosticSourceListener(this.configuration, true);
         }
 
         public void Dispose()
@@ -114,7 +112,7 @@ var sqlErrorCollectionCtor
 
             typeof(SqlErrorCollection).GetMethod("Add", BindingFlags.NonPublic | BindingFlags.Instance)
                 .Invoke(sqlErrorCollection, new object[] { sqlError });
-            
+
             var sqlExceptionCtor = typeof(SqlException).GetConstructors(BindingFlags.NonPublic | BindingFlags.Instance)[0];
 
             var sqlException = (SqlException)sqlExceptionCtor.Invoke(

WEB/Src/DependencyCollector/Shared/DependencyTrackingTelemetryModule.cs

@@ -4,6 +4,7 @@
     using System.Collections.Generic;
     using System.Diagnostics;
     using System.Runtime.CompilerServices;
+
     using Microsoft.ApplicationInsights.DependencyCollector.Implementation;
     using Microsoft.ApplicationInsights.DependencyCollector.Implementation.EventHandlers;
     using Microsoft.ApplicationInsights.DependencyCollector.Implementation.SqlClientDiagnostics;
@@ -69,6 +70,11 @@ public class DependencyTrackingTelemetryModule : ITelemetryModule, IDisposable
         /// </summary>
         public bool EnableRequestIdHeaderInjectionInW3CMode { get; set; } = true;
 
+        /// <summary>
+        /// Gets or sets a value indicating whether to track the SQL command text in SQL dependencies.
+        /// </summary>
+        public bool EnableSqlCommandTextInstrumentation { get; set; } = false;
+
         /// <summary>
         /// Gets the component correlation configuration.
         /// </summary>
@@ -146,7 +152,7 @@ public void Initialize(TelemetryConfiguration configuration)
                                 this.telemetryDiagnosticSourceListener.Subscribe();
                             }
 
-                            this.sqlClientDiagnosticSourceListener = new SqlClientDiagnosticSourceListener(configuration);
+                            this.sqlClientDiagnosticSourceListener = new SqlClientDiagnosticSourceListener(configuration, this.EnableSqlCommandTextInstrumentation);
 
                             if (this.EnableAzureSdkTelemetryListener)
                             {
@@ -198,7 +204,7 @@ internal virtual void InitializeForRuntimeProfiler()
                 this.ExcludeComponentCorrelationHttpHeadersOnDomains,
                 this.EnableLegacyCorrelationHeadersInjection,
                 this.EnableRequestIdHeaderInjectionInW3CMode);
-            this.sqlCommandProcessing = new ProfilerSqlCommandProcessing(this.telemetryConfiguration, agentVersion, DependencyTableStore.Instance.SqlRequestConditionalHolder);
+            this.sqlCommandProcessing = new ProfilerSqlCommandProcessing(this.telemetryConfiguration, agentVersion, DependencyTableStore.Instance.SqlRequestConditionalHolder, this.EnableSqlCommandTextInstrumentation);
             this.sqlConnectionProcessing = new ProfilerSqlConnectionProcessing(this.telemetryConfiguration, agentVersion, DependencyTableStore.Instance.SqlRequestConditionalHolder);
 
             ProfilerRuntimeInstrumentation.DecorateProfilerForHttp(ref this.httpProcessing);
@@ -298,8 +304,8 @@ private void InitializeForDiagnosticAndFrameworkEventSource()
 
             FrameworkHttpProcessing frameworkHttpProcessing = new FrameworkHttpProcessing(
                 this.telemetryConfiguration,
-                DependencyTableStore.Instance.WebRequestCacheHolder, 
-                this.SetComponentCorrelationHttpHeaders, 
+                DependencyTableStore.Instance.WebRequestCacheHolder,
+                this.SetComponentCorrelationHttpHeaders,
                 this.ExcludeComponentCorrelationHttpHeadersOnDomains,
                 this.EnableLegacyCorrelationHeadersInjection);
 
@@ -310,7 +316,7 @@ private void InitializeForDiagnosticAndFrameworkEventSource()
                 TimeSpan.FromMilliseconds(10));
 
             this.sqlEventListener = RetryPolicy.Retry<InvalidOperationException, TelemetryConfiguration, FrameworkSqlEventListener>(
-                config => new FrameworkSqlEventListener(config, DependencyTableStore.Instance.SqlRequestCacheHolder),
+                config => new FrameworkSqlEventListener(config, DependencyTableStore.Instance.SqlRequestCacheHolder, this.EnableSqlCommandTextInstrumentation),
                 this.telemetryConfiguration,
                 TimeSpan.FromMilliseconds(10));
         }

WEB/Src/DependencyCollector/Shared/Implementation/FrameworkSqlEventListener.cs

@@ -32,9 +32,15 @@ internal class FrameworkSqlEventListener : EventListener
         /// </summary>
         private const int EndExecuteEventId = 2;
 
-        internal FrameworkSqlEventListener(TelemetryConfiguration configuration, CacheBasedOperationHolder telemetryTupleHolder)
+        /// <summary>
+        /// Indicates whether SQL command text should be collected or not.
+        /// </summary>
+        private readonly bool collectCommandText;
+
+        internal FrameworkSqlEventListener(TelemetryConfiguration configuration, CacheBasedOperationHolder telemetryTupleHolder, bool collectCommandText)
         {
             this.SqlProcessingFramework = new FrameworkSqlProcessing(configuration, telemetryTupleHolder);
+            this.collectCommandText = collectCommandText;
         }
 
         private enum CompositeState
@@ -99,7 +105,7 @@ private void OnBeginExecute(EventWrittenEventArgs eventData)
                 var id = Convert.ToInt64(eventData.Payload[0], CultureInfo.InvariantCulture);
                 var dataSource = Convert.ToString(eventData.Payload[1], CultureInfo.InvariantCulture);
                 var database = Convert.ToString(eventData.Payload[2], CultureInfo.InvariantCulture);
-                var commandText = Convert.ToString(eventData.Payload[3], CultureInfo.InvariantCulture);
+                var commandText = this.collectCommandText ? Convert.ToString(eventData.Payload[3], CultureInfo.InvariantCulture) : string.Empty;
 
                 if (this.SqlProcessingFramework != null)
                 {

WEB/Src/DependencyCollector/Shared/Implementation/ProfilerSqlCommandProcessing.cs

@@ -2,6 +2,7 @@
 {
     using System.Data;
     using System.Data.SqlClient;
+
     using Microsoft.ApplicationInsights.DataContracts;
     using Microsoft.ApplicationInsights.DependencyCollector.Implementation.Operation;
     using Microsoft.ApplicationInsights.Extensibility;
@@ -11,12 +12,15 @@
     /// </summary>
     internal sealed class ProfilerSqlCommandProcessing : ProfilerSqlProcessingBase
     {
+        private readonly bool collectCommandText;
+
         /// <summary>
         /// Initializes a new instance of the <see cref="ProfilerSqlCommandProcessing"/> class.
         /// </summary>
-        internal ProfilerSqlCommandProcessing(TelemetryConfiguration configuration, string agentVersion, ObjectInstanceBasedOperationHolder<DependencyTelemetry> telemetryTupleHolder)
+        internal ProfilerSqlCommandProcessing(TelemetryConfiguration configuration, string agentVersion, ObjectInstanceBasedOperationHolder<DependencyTelemetry> telemetryTupleHolder, bool collectCommandText)
             : base(configuration, agentVersion, telemetryTupleHolder)
-        {            
+        {
+            this.collectCommandText = collectCommandText;
         }
 
         /// <summary>
@@ -72,11 +76,14 @@ internal override string GetDependencyTarget(object thisObj)
         /// <returns>Returns the command text or empty.</returns>
         internal override string GetCommandName(object thisObj)
         {
-            SqlCommand command = thisObj as SqlCommand;
-
-            if (command != null)
+            if (this.collectCommandText)
             {
-                return command.CommandText ?? string.Empty;
+                SqlCommand command = thisObj as SqlCommand;
+
+                if (command != null)
+                {
+                    return command.CommandText ?? string.Empty;
+                }
             }
 
             return string.Empty;

WEB/Src/DependencyCollector/Shared/Implementation/SqlClientDiagnostics/SqlClientDiagnosticSourceListener.cs

@@ -11,7 +11,7 @@ namespace Microsoft.ApplicationInsights.DependencyCollector.Implementation.SqlCl
     using Microsoft.ApplicationInsights.Extensibility;
     using Microsoft.ApplicationInsights.Extensibility.Implementation;
     using Microsoft.ApplicationInsights.Extensibility.Implementation.Tracing;
-    using Microsoft.ApplicationInsights.W3C.Internal;
+
     using static Microsoft.ApplicationInsights.DependencyCollector.Implementation.SqlClientDiagnostics.SqlClientDiagnosticFetcherTypes;
 
     internal class SqlClientDiagnosticSourceListener : IObserver<KeyValuePair<string, object>>, IDisposable
@@ -72,14 +72,16 @@ internal class SqlClientDiagnosticSourceListener : IObserver<KeyValuePair<string
         private readonly SqlClientDiagnosticSourceSubscriber subscriber;
 
         private readonly ObjectInstanceBasedOperationHolder<DependencyTelemetry> operationHolder = new ObjectInstanceBasedOperationHolder<DependencyTelemetry>();
+        private readonly bool collectCommandText;
 
-        public SqlClientDiagnosticSourceListener(TelemetryConfiguration configuration)
+        public SqlClientDiagnosticSourceListener(TelemetryConfiguration configuration, bool collectCommandText)
         {
             this.client = new TelemetryClient(configuration);
             this.client.Context.GetInternalContext().SdkVersion =
                 SdkVersionUtils.GetSdkVersion("rdd" + RddSource.DiagnosticSourceCore + ":");
 
             this.subscriber = new SqlClientDiagnosticSourceSubscriber(this);
+            this.collectCommandText = collectCommandText;
         }
 
         public void Dispose()
@@ -412,8 +414,16 @@ private void BeforeExecuteHelper(KeyValuePair<string, object> evnt,
             {
                 var dependencyName = string.Empty;
                 var target = string.Empty;
+                var commandType = (int)commandTypeFetcher.Fetch(command);
+                var commandText = string.Empty;
+
+                // https://docs.microsoft.com/dotnet/api/system.data.commandtype
+                // 4 indicate StoredProcedure
+                if (this.collectCommandText)
+                {
+                    commandText = (string)commandTextFetcher.Fetch(command);
+                }
 
-                var commandText = (string)commandTextFetcher.Fetch(command);
                 var con = connectionFetcher.Fetch(command);
                 if (con != null)
                 {
@@ -423,7 +433,7 @@ private void BeforeExecuteHelper(KeyValuePair<string, object> evnt,
 
                     // https://docs.microsoft.com/dotnet/api/system.data.commandtype
                     // 4 indicate StoredProcedure
-                    var commandName = (int)commandTypeFetcher.Fetch(command) == 4
+                    var commandName = commandType == 4
                         ? commandText
                         : string.Empty;