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

CHANGELOG.md

@@ -6,6 +6,8 @@
 ## Version 2.12.0-beta1
  - [Enhancement to how QuickPulseTelemetryModule shares its ServiceEndpoint with QuickPulseTelemetryProcessor.](https://github.com/microsoft/ApplicationInsights-dotnet-server/pull/1266)
  - [QuickPulse will support SDK Connection String](https://github.com/microsoft/ApplicationInsights-dotnet/issues/1221)
+ - [Adding a flag to DependencyTrackingTelemetryModule to enable/disable collection of SQL Command text.](https://github.com/microsoft/ApplicationInsights-dotnet-server/pull/1285)
+   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.11.0
  - Updated Base SDK to 2.11.0

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]
@@ -52,7 +53,8 @@ public void TestInitialize()
             this.sendItems = new List<ITelemetry>();
             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 +305,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()
         {

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

@@ -40,7 +40,7 @@ public SqlClientDiagnosticSourceListenerTests()
             };
 
             this.fakeSqlClientDiagnosticSource = new FakeSqlClientDiagnosticSource();
-            this.sqlClientDiagnosticSourceListener = new SqlClientDiagnosticSourceListener(this.configuration);
+            this.sqlClientDiagnosticSourceListener = new SqlClientDiagnosticSourceListener(this.configuration, true);
         }
 
         public void Dispose()
@@ -582,7 +582,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,
@@ -596,6 +596,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()
         {
@@ -630,7 +670,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,
@@ -666,7 +706,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,

Src/DependencyCollector/Shared/DependencyTrackingTelemetryModule.cs

@@ -68,6 +68,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>
@@ -140,7 +145,7 @@ public void Initialize(TelemetryConfiguration configuration)
                                 this.telemetryDiagnosticSourceListener.Subscribe();
                             }
 
-                            this.sqlClientDiagnosticSourceListener = new SqlClientDiagnosticSourceListener(configuration);
+                            this.sqlClientDiagnosticSourceListener = new SqlClientDiagnosticSourceListener(configuration, this.EnableSqlCommandTextInstrumentation);
 
                             DependencyCollectorEventSource.Log.RemoteDependencyModuleVerbose("Initializing DependencyTrackingModule completed successfully.");
                         }
@@ -186,7 +191,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);
@@ -293,7 +298,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));
         }

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)
                 {

Src/DependencyCollector/Shared/Implementation/ProfilerSqlCommandProcessing.cs

@@ -10,12 +10,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 telemetryTupleHolder)
+        internal ProfilerSqlCommandProcessing(TelemetryConfiguration configuration, string agentVersion, ObjectInstanceBasedOperationHolder telemetryTupleHolder, bool collectCommandText)
             : base(configuration, agentVersion, telemetryTupleHolder)
-        {            
+        {
+            this.collectCommandText = collectCommandText;
         }
 
         /// <summary>
@@ -71,11 +74,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;

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

@@ -73,14 +73,16 @@ internal class SqlClientDiagnosticSourceListener : IObserver<KeyValuePair<string
         private readonly SqlClientDiagnosticSourceSubscriber subscriber;
 
         private readonly ObjectInstanceBasedOperationHolder operationHolder = new ObjectInstanceBasedOperationHolder();
+        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()
@@ -394,16 +396,21 @@ private void BeforeExecuteHelper(KeyValuePair<string, object> evnt,
             {
                 var dependencyName = string.Empty;
                 var target = string.Empty;
+                var commandType = (CommandType)commandTypeFetcher.Fetch(command);
+                var commandText = string.Empty;
+                if (this.collectCommandText && commandType == CommandType.Text)
+                {
+                    commandText = (string)commandTextFetcher.Fetch(command);
+                }
 
-                var commandText = (string)commandTextFetcher.Fetch(command);
                 var con = connectionFetcher.Fetch(command);
                 if (con != null)
                 {
                     var dataSource = dataSourceFetcher.Fetch(con);
                     var database = databaseFetcher.Fetch(con);
                     target = string.Join(" | ", dataSource, database);
 
-                    var commandName = (CommandType)commandTypeFetcher.Fetch(command) == CommandType.StoredProcedure
+                    var commandName = commandType == CommandType.StoredProcedure
                         ? commandText
                         : string.Empty;