Merge pull request #206 from karolz-ms/develop

Disable Microsoft-ApplicationInsights-Data EventSource by default
microsoft · Aug 13, 2018 · 4dc34ab · 4dc34ab
2 parents 03bf8a9 + fa9b3a5
commit 4dc34ab
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 2 deletions.
diff --git a/CHANGELOG.md b/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)

diff --git a/src/EventSourceListener/EventSourceListeningRequestBase.cs b/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; }
 

diff --git a/src/EventSourceListener/EventSourceTelemetryModule.cs b/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

diff --git a/test/EventSourceListener.netcoreapp10.Tests/EventSourceTelemetryModuleTests.cs b/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 () =>