Merge pull request #206 from karolz-ms/develop

Disable Microsoft-ApplicationInsights-Data EventSource by default

Author: SergeyKanzhelev

CHANGELOG.md

@@ -3,6 +3,7 @@
 ### Version 2.7.1
 - [NLog can perform Layout of InstrumentationKey](https://github.com/Microsoft/ApplicationInsights-dotnet-logging/pull/203)
 - Upgrade `System.Diagnostics.DiagnosticSource` to version 4.5.0
+- [Event Source telemetry module: Microsoft-ApplicationInsights-Data id disabled by default to work around CLR bug](https://github.com/Microsoft/ApplicationInsights-dotnet-logging/pull/206)
 
 ### Version 2.6.4
 - [Log4Net new supports NetStandard 1.3!](https://github.com/Microsoft/ApplicationInsights-dotnet-logging/pull/167)

src/EventSourceListener/EventSourceListeningRequestBase.cs

@@ -17,7 +17,7 @@ public abstract class EventSourceListeningRequestBase
         public string Name { get; set; }
 
         /// <summary>
-        /// Gets or sets a value indicating whether allows wildcards in <see cref="Name" />.
+        /// Gets or sets a value indicating whether the value of the <see cref="Name"/> property should match the name of an EventSource exactly, or should the value be treated as EventSource name prefix.
         /// </summary>
         public bool PrefixMatch { get; set; }
 

src/EventSourceListener/EventSourceTelemetryModule.cs

@@ -29,6 +29,8 @@ namespace Microsoft.ApplicationInsights.EventSourceListener
     /// </summary>
     public class EventSourceTelemetryModule : EventListener, ITelemetryModule
     {
+        private const string AppInsightsDataEventSource = "Microsoft-ApplicationInsights-Data";
+
         private readonly OnEventWrittenHandler onEventWrittenHandler;
         private OnEventWrittenHandler eventWrittenHandlerPicker;
 
@@ -90,7 +92,7 @@ public void Initialize(TelemetryConfiguration configuration)
             if (this.Sources.Count == 0)
             {
                 EventSourceListenerEventSource.Log.NoSourcesConfigured(nameof(EventSourceListener.EventSourceTelemetryModule));
-                return;
+                // Continue--we need to be prepared for handling disabled sources.
             }
 
             try
@@ -105,6 +107,16 @@ public void Initialize(TelemetryConfiguration configuration)
                     }
                 }
 
+                // Special case: because of .NET bug https://github.com/dotnet/coreclr/issues/14434, using Microsoft-ApplicationInsights-Data will result in infinite loop.
+                // So we will disable it by default, unless there is explicit configuration for this EventSource.
+                bool hasExplicitConfigForAiDataSource =
+                    this.Sources.Any(req => req.Name?.StartsWith(AppInsightsDataEventSource, StringComparison.Ordinal) ?? false) ||
+                    this.DisabledSources.Any(req => req.Name?.StartsWith(AppInsightsDataEventSource, StringComparison.Ordinal) ?? false);
+                if (!hasExplicitConfigForAiDataSource)
+                {
+                    this.DisabledSources.Add(new DisableEventSourceRequest { Name = AppInsightsDataEventSource });
+                }
+
                 // Set the initialized flag now to ensure that we do not miss any sources that came online as we are executing the initialization
                 // (OnEventSourceCreated() might have been called on a separate thread). Worst case we will attempt to enable the same source twice
                 // (with same settings), but that is OK, as the semantics of EnableEvents() is really "update what is being tracked", so it is fine

test/EventSourceListener.netcoreapp10.Tests/EventSourceTelemetryModuleTests.cs

@@ -411,6 +411,32 @@ public void DoNotReportTplEvents()
             }
         }
 
+        [TestMethod]
+        [TestCategory("EventSourceListener")]
+        public void DisablesAppInsightsDataByDefault()
+        {
+            using (var module = new EventSourceTelemetryModule())
+            {
+                module.Initialize(GetTestTelemetryConfiguration());
+
+                Assert.AreEqual(1, module.DisabledSources.Count);
+                Assert.AreEqual(new DisableEventSourceRequest { Name = "Microsoft-ApplicationInsights-Data" }, module.DisabledSources[0]);
+            }
+        }
+
+        [TestMethod]
+        [TestCategory("EventSourceListener")]
+        public void DoesNotDisableAppInsightsDataIfExplicitlyEnabled()
+        {
+            using (var module = new EventSourceTelemetryModule())
+            {
+                module.Sources.Add(new EventSourceListeningRequest { Name = "Microsoft-ApplicationInsights-Data" });
+                module.Initialize(GetTestTelemetryConfiguration());
+
+                Assert.AreEqual(0, module.DisabledSources.Count);
+            }
+        }
+
         private Task PerformActivityAsync(int requestId)
         {
             return Task.Run(async () =>