Work-around heartbeat interval initialization bug in AI (#321)

* Work-around heartbeat interval initialization bug in AI

* Use TelemetryConfiguration.CreateDefault() to account for applicationinsights.config file

* Avoid static state. Added comments.
Make DiagnosticsTelemetryModule accessible to allow tweaking heartbeats.

Author: xavierdecoster

src/NuGet.Services.Logging/ApplicationInsights.cs

@@ -5,98 +5,89 @@
 using Microsoft.ApplicationInsights;
 using Microsoft.ApplicationInsights.DataContracts;
 using Microsoft.ApplicationInsights.Extensibility;
-using Microsoft.ApplicationInsights.Extensibility.Implementation;
 using Microsoft.ApplicationInsights.Extensibility.Implementation.Tracing;
 
 namespace NuGet.Services.Logging
 {
+    /// <summary>
+    /// Utility class to initialize an <see cref="ApplicationInsightsConfiguration"/> instance
+    /// using provided instrumentation key, optional heartbeat interval, 
+    /// and, if detected, taking into account an optional ApplicationInsights.config file.
+    /// </summary>
+    /// <remarks>
+    /// Calling <see cref="Initialize(string)"/> or <see cref="Initialize(string, TimeSpan)"/> returns the
+    /// initialized <see cref="ApplicationInsightsConfiguration"/> object; 
+    /// it does not set the obsolete <see cref="TelemetryClient.Active"/> component.
+    /// 
+    /// It is the caller's responsibility to ensure the returned configuration is used 
+    /// when creating new <see cref="TelemetryClient"/> instances.
+    /// </remarks>
     public static class ApplicationInsights
     {
-        public static IHeartbeatPropertyManager HeartbeatManager { get; private set; }
-
-        public static bool Initialized { get; private set; }
-
-        public static void Initialize(string instrumentationKey)
+        /// <summary>
+        /// Initializes an <see cref="ApplicationInsightsConfiguration"/> using the provided
+        /// <paramref name="instrumentationKey"/>, taking into account the <c>ApplicationInsights.config</c> file if present.
+        /// </summary>
+        /// <param name="instrumentationKey">The instrumentation key to use.</param>
+        public static ApplicationInsightsConfiguration Initialize(string instrumentationKey)
         {
-            InitializeTelemetryConfiguration(instrumentationKey, heartbeatInterval: null);
+            return InitializeApplicationInsightsConfiguration(instrumentationKey, heartbeatInterval: null);
         }
 
-        public static void Initialize(string instrumentationKey, TimeSpan heartbeatInterval)
+        /// <summary>
+        /// Initializes an <see cref="ApplicationInsightsConfiguration"/> using the provided
+        /// <paramref name="instrumentationKey"/> and <paramref name="heartbeatInterval"/>, 
+        /// taking into account the <c>ApplicationInsights.config</c> file if present.
+        /// </summary>
+        /// <param name="instrumentationKey">The instrumentation key to use.</param>
+        /// <param name="heartbeatInterval">The heartbeat interval to use.</param>
+        public static ApplicationInsightsConfiguration Initialize(
+            string instrumentationKey,
+            TimeSpan heartbeatInterval)
         {
-            InitializeTelemetryConfiguration(instrumentationKey, heartbeatInterval);
+            return InitializeApplicationInsightsConfiguration(instrumentationKey, heartbeatInterval);
         }
 
-        private static void InitializeTelemetryConfiguration(string instrumentationKey, TimeSpan? heartbeatInterval)
+        private static ApplicationInsightsConfiguration InitializeApplicationInsightsConfiguration(
+            string instrumentationKey,
+            TimeSpan? heartbeatInterval)
         {
+            // Note: TelemetryConfiguration.Active is being deprecated
+            // https://github.com/microsoft/ApplicationInsights-dotnet/issues/1152
+            // We use TelemetryConfiguration.CreateDefault() as opposed to instantiating a new TelemetryConfiguration()
+            // to take into account the ApplicationInsights.config file (if detected).
+            var telemetryConfiguration = TelemetryConfiguration.CreateDefault();
+
             if (!string.IsNullOrWhiteSpace(instrumentationKey))
             {
-                TelemetryConfiguration.Active.InstrumentationKey = instrumentationKey;
-                TelemetryConfiguration.Active.TelemetryInitializers.Add(new TelemetryContextInitializer());
+                telemetryConfiguration.InstrumentationKey = instrumentationKey;
+            }
 
-                // Construct a TelemetryClient to emit traces so we can track and debug AI initialization.
-                var telemetryClient = new TelemetryClient();
+            telemetryConfiguration.TelemetryInitializers.Add(new TelemetryContextInitializer());
 
-                // Configure heartbeat interval if specified.
-                // When not defined or null, the DiagnosticsTelemetryModule will use its internal defaults (heartbeat enabled, interval of 15 minutes).
-                if (heartbeatInterval.HasValue)
-                {
-                    var heartbeatManager = GetHeartbeatPropertyManager(telemetryClient);
-                    if (heartbeatManager != null)
-                    {
-                        heartbeatManager.HeartbeatInterval = heartbeatInterval.Value;
+            // Construct a TelemetryClient to emit traces so we can track and debug AI initialization.
+            var telemetryClient = new TelemetryClient(telemetryConfiguration);
 
-                        telemetryClient.TrackTrace(
-                            $"Telemetry initialized using configured heartbeat interval: {heartbeatInterval.Value}.",
-                            SeverityLevel.Information);
-                    }
-                }
-                else
-                {
-                    telemetryClient.TrackTrace(
-                        "Telemetry initialized using default heartbeat interval.",
+            telemetryClient.TrackTrace(
+                        $"TelemetryConfiguration initialized using instrumentation key: {instrumentationKey ?? "EMPTY"}.",
                         SeverityLevel.Information);
-                }
 
-                Initialized = true;
-            }
-            else
-            {
-                Initialized = false;
-            }
-        }
+            var diagnosticsTelemetryModule = new DiagnosticsTelemetryModule();
 
-        private static IHeartbeatPropertyManager GetHeartbeatPropertyManager(TelemetryClient telemetryClient)
-        {
-            if (HeartbeatManager == null)
+            // Configure heartbeat interval if specified.
+            // When not defined, the DiagnosticsTelemetryModule will use its internal defaults (heartbeat enabled, interval of 15 minutes).
+            var traceMessage = "DiagnosticsTelemetryModule initialized using default heartbeat interval.";
+            if (heartbeatInterval.HasValue)
             {
-                var telemetryModules = TelemetryModules.Instance;
+                diagnosticsTelemetryModule.HeartbeatInterval = heartbeatInterval.Value;
+                traceMessage = $"DiagnosticsTelemetryModule initialized using configured heartbeat interval: {heartbeatInterval.Value}.";
+            }
 
-                try
-                {
-                    foreach (var module in telemetryModules.Modules)
-                    {
-                        if (module is IHeartbeatPropertyManager heartbeatManager)
-                        {
-                            HeartbeatManager = heartbeatManager;
-                        }
-                    }
-                }
-                catch (Exception hearbeatManagerAccessException)
-                {
-                    // An non-critical, unexpected exception occurred trying to access the heartbeat manager.
-                    telemetryClient.TrackTrace(
-                        $"There was an error accessing heartbeat manager. Details: {hearbeatManagerAccessException.ToInvariantString()}",
-                        SeverityLevel.Error);
-                }
+            diagnosticsTelemetryModule.Initialize(telemetryConfiguration);
 
-                if (HeartbeatManager == null)
-                {
-                    // Heartbeat manager unavailable: log warning.
-                    telemetryClient.TrackTrace("Heartbeat manager unavailable", SeverityLevel.Warning);
-                }
-            }
+            telemetryClient.TrackTrace(traceMessage, SeverityLevel.Information);
 
-            return HeartbeatManager;
+            return new ApplicationInsightsConfiguration(telemetryConfiguration, diagnosticsTelemetryModule);
         }
     }
 }

src/NuGet.Services.Logging/ApplicationInsightsConfiguration.cs

@@ -0,0 +1,36 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using Microsoft.ApplicationInsights.Extensibility;
+using Microsoft.ApplicationInsights.Extensibility.Implementation.Tracing;
+
+namespace NuGet.Services.Logging
+{
+    public sealed class ApplicationInsightsConfiguration
+    {
+        internal ApplicationInsightsConfiguration(
+            TelemetryConfiguration telemetryConfiguration,
+            DiagnosticsTelemetryModule diagnosticsTelemetryModule)
+        {
+            TelemetryConfiguration = telemetryConfiguration ?? throw new ArgumentNullException(nameof(telemetryConfiguration));
+            DiagnosticsTelemetryModule = diagnosticsTelemetryModule ?? throw new ArgumentNullException(nameof(diagnosticsTelemetryModule));
+        }
+
+        /// <summary>
+        /// Contains the initialized <see cref="Microsoft.ApplicationInsights.Extensibility.TelemetryConfiguration"/>.
+        /// Used to initialize new <see cref="Microsoft.ApplicationInsights.TelemetryClient"/> instances.
+        /// Allows tweaking telemetry initializers.
+        /// </summary>
+        /// <remarks>
+        /// Needs to be disposed when gracefully shutting down the application.
+        /// </remarks>
+        public TelemetryConfiguration TelemetryConfiguration { get; }
+
+        /// <summary>
+        /// Contains the initialized <see cref="Microsoft.ApplicationInsights.Extensibility.Implementation.Tracing.DiagnosticsTelemetryModule"/>.
+        /// Allows tweaking Application Insights heartbeat telemetry.
+        /// </summary>
+        public DiagnosticsTelemetryModule DiagnosticsTelemetryModule { get; }
+    }
+}

src/NuGet.Services.Logging/NuGet.Services.Logging.csproj

@@ -49,6 +49,7 @@
   </ItemGroup>
   <ItemGroup>
     <Compile Include="ApplicationInsights.cs" />
+    <Compile Include="ApplicationInsightsConfiguration.cs" />
     <Compile Include="DurationMetric.cs" />
     <Compile Include="ExceptionTelemetryProcessor.cs" />
     <Compile Include="LoggingSetup.cs" />

tests/NuGet.Services.Logging.Tests/ApplicationInsightsTests.cs

@@ -0,0 +1,40 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using Xunit;
+
+namespace NuGet.Services.Logging.Tests
+{
+    public class ApplicationInsightsTests
+    {
+        private const string InstrumentationKey = "abcdef12-3456-7890-abcd-ef123456789";
+
+        [Fact]
+        public void InitializeReturnsApplicationInsightsConfiguration()
+        {
+            var applicationInsightsConfiguration = ApplicationInsights.Initialize(InstrumentationKey);
+
+            Assert.NotNull(applicationInsightsConfiguration);
+            Assert.Equal(InstrumentationKey, applicationInsightsConfiguration.TelemetryConfiguration.InstrumentationKey);
+        }
+
+        [Fact]
+        public void InitializeRegistersTelemetryContextInitializer()
+        {
+            var applicationInsightsConfiguration = ApplicationInsights.Initialize(InstrumentationKey);
+            Assert.Contains(applicationInsightsConfiguration.TelemetryConfiguration.TelemetryInitializers, ti => ti is TelemetryContextInitializer);
+        }
+
+        [Fact]
+        public void InitializeSetsHeartbeatIntervalAndDiagnosticsTelemetryModule()
+        {
+            var heartbeatInterval = TimeSpan.FromMinutes(1);
+
+            var applicationInsightsConfiguration = ApplicationInsights.Initialize(InstrumentationKey, heartbeatInterval);
+
+            Assert.NotNull(applicationInsightsConfiguration.DiagnosticsTelemetryModule);
+            Assert.Equal(heartbeatInterval, applicationInsightsConfiguration.DiagnosticsTelemetryModule.HeartbeatInterval);
+        }
+    }
+}

tests/NuGet.Services.Logging.Tests/NuGet.Services.Logging.Tests.csproj

@@ -53,6 +53,7 @@
     </PackageReference>
   </ItemGroup>
   <ItemGroup>
+    <Compile Include="ApplicationInsightsTests.cs" />
     <Compile Include="ExceptionTelemetryProcessorTests.cs" />
     <Compile Include="LoggingSetupTests.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />