From 55b8e0300ed3fd702f5433bbdf73b64064707db7 Mon Sep 17 00:00:00 2001 From: Mikel Blanchard Date: Tue, 22 Oct 2024 12:11:05 -0700 Subject: [PATCH] Remove AddSingletonIfNotExists and call TryAddSingleton to fix bugs when keyed services are registered. --- .../ApplicationInsightsExtensions.cs | 22 +--- .../AddApplicationInsightsTelemetryTests.cs | 83 ++++++++----- .../AddSingletonTests.cs | 96 -------------- ...pplicationInsights.AspNetCore.Tests.csproj | 5 +- .../ExtensionsTest.cs | 117 +++++++++++------- ...icationInsights.WorkerService.Tests.csproj | 3 +- 6 files changed, 133 insertions(+), 193 deletions(-) delete mode 100644 NETCORE/test/Microsoft.ApplicationInsights.AspNetCore.Tests/Extensions/ApplicationInsightsExtensionsTests/AddSingletonTests.cs diff --git a/NETCORE/src/Shared/Extensions/ApplicationInsightsExtensions.cs b/NETCORE/src/Shared/Extensions/ApplicationInsightsExtensions.cs index 1c098c3a96..efbce45c49 100644 --- a/NETCORE/src/Shared/Extensions/ApplicationInsightsExtensions.cs +++ b/NETCORE/src/Shared/Extensions/ApplicationInsightsExtensions.cs @@ -280,26 +280,6 @@ internal static void AddTelemetryConfiguration( } } - /// - /// The AddSingleton method will not check if a class has already been added as an ImplementationType. - /// This extension method is to encapsulate those checks. - /// - /// - /// Must check all three properties to avoid duplicates or null ref exceptions. - /// - /// The type of the service to add. - /// The type of the implementation to use. - /// The Microsoft.Extensions.DependencyInjection.IServiceCollection to add the service to. - internal static void AddSingletonIfNotExists(this IServiceCollection services) - where TService : class - where TImplementation : class, TService - { - if (!services.Any(o => o.ImplementationFactory == null && typeof(TImplementation).IsAssignableFrom(o.ImplementationType ?? o.ImplementationInstance.GetType()))) - { - services.AddSingleton(); - } - } - private static bool TryGetValue(this IConfiguration config, string primaryKey, out string value, string backupKey = null) { value = config[primaryKey]; @@ -332,7 +312,7 @@ private static void AddCommonInitializers(IServiceCollection services) private static void AddCommonTelemetryModules(IServiceCollection services) { // Previously users were encouraged to manually add the DiagnosticsTelemetryModule. - services.AddSingletonIfNotExists(); + services.TryAddSingleton(); // These modules add properties to Heartbeat and expect the DiagnosticsTelemetryModule to be configured in DI. services.AddSingleton(); diff --git a/NETCORE/test/Microsoft.ApplicationInsights.AspNetCore.Tests/Extensions/ApplicationInsightsExtensionsTests/AddApplicationInsightsTelemetryTests.cs b/NETCORE/test/Microsoft.ApplicationInsights.AspNetCore.Tests/Extensions/ApplicationInsightsExtensionsTests/AddApplicationInsightsTelemetryTests.cs index 3282ffa5e3..57529f2dbb 100644 --- a/NETCORE/test/Microsoft.ApplicationInsights.AspNetCore.Tests/Extensions/ApplicationInsightsExtensionsTests/AddApplicationInsightsTelemetryTests.cs +++ b/NETCORE/test/Microsoft.ApplicationInsights.AspNetCore.Tests/Extensions/ApplicationInsightsExtensionsTests/AddApplicationInsightsTelemetryTests.cs @@ -37,6 +37,25 @@ namespace Microsoft.Extensions.DependencyInjection.Test public class AddApplicationInsightsTelemetryTests : BaseTestClass { + [Fact] + public static void TelemetryModuleResolvableWhenKeyedServiceRegistered() + { + // Note: This test verifies a regression doesn't get introduced for: + // https://github.com/dotnet/extensions/issues/5222 + + var services = new ServiceCollection(); + + services.AddKeyedSingleton(typeof(ITestService), serviceKey: new(), implementationType: typeof(TestService)); + services.AddKeyedSingleton(typeof(ITestService), serviceKey: new(), implementationInstance: new TestService()); + services.AddKeyedSingleton(typeof(ITestService), serviceKey: new(), implementationFactory: (sp, key) => new TestService()); + + services.AddApplicationInsightsTelemetry(); + + using var sp = services.BuildServiceProvider(); + + var telmetryModule = sp.GetRequiredService(); + } + [Theory] [InlineData(typeof(ITelemetryInitializer), typeof(ApplicationInsights.AspNetCore.TelemetryInitializers.DomainNameRoleInstanceTelemetryInitializer), ServiceLifetime.Singleton)] [InlineData(typeof(ITelemetryInitializer), typeof(AzureAppServiceRoleNameFromHostNameHeaderInitializer), ServiceLifetime.Singleton)] @@ -94,7 +113,7 @@ public static void RegistersTelemetryConfigurationFactoryMethodThatCreatesDefaul } /// - /// Tests that the instrumentation key configuration can be read from a JSON file by the configuration factory. + /// Tests that the instrumentation key configuration can be read from a JSON file by the configuration factory. /// /// /// Calls services.AddApplicationInsightsTelemetry() when the value is true and reads IConfiguration from user application automatically. @@ -119,7 +138,7 @@ public static void RegistersTelemetryConfigurationFactoryMethodThatReadsInstrume } /// - /// Tests that the connection string can be read from a JSON file by the configuration factory. + /// Tests that the connection string can be read from a JSON file by the configuration factory. /// /// /// Calls services.AddApplicationInsightsTelemetry() when the value is true and reads IConfiguration from user application automatically. @@ -141,7 +160,7 @@ public static void RegistersTelemetryConfigurationFactoryMethodThatReadsConnecti } /// - /// Tests that the connection string can be read from a JSON file by the configuration factory. + /// Tests that the connection string can be read from a JSON file by the configuration factory. /// This config has both a connection string and an instrumentation key. It is expected to use the ikey from the connection string. /// [Fact] @@ -159,7 +178,7 @@ public static void RegistersTelemetryConfigurationFactoryMethodThatReadsConnecti /// /// Tests that the Active configuration singleton is updated, but another instance of telemetry configuration is created for dependency injection. - /// ASP.NET Core developers should always use Dependency Injection instead of static singleton approach. + /// ASP.NET Core developers should always use Dependency Injection instead of static singleton approach. /// See Microsoft/ApplicationInsights-dotnet#613 /// [Fact] @@ -221,7 +240,7 @@ public static void RegistersTelemetryConfigurationFactoryMethodThatReadsEndpoint if (useDefaultConfig) { - // Endpoint comes from appSettings + // Endpoint comes from appSettings Assert.Equal("http://hosthere/v2/track/", telemetryConfiguration.TelemetryChannel.EndpointAddress); } else @@ -603,7 +622,7 @@ public static void RegistersTelemetryConfigurationFactoryMethodThatPopulatesItWi #if NETCOREAPP // Developer Note: Expected modules: - // RequestTrackingTelemetryModule, PerformanceCollectorModule, AppServicesHeartbeatTelemetryModule, AzureInstanceMetadataTelemetryModule, + // RequestTrackingTelemetryModule, PerformanceCollectorModule, AppServicesHeartbeatTelemetryModule, AzureInstanceMetadataTelemetryModule, // QuickPulseTelemetryModule, DiagnosticsTelemetryModule, DependencyTrackingTelemetryModule, EventCollectorCollectionModule Assert.Equal(8, modules.Count()); #else @@ -681,13 +700,13 @@ public static void RegistersTelemetryConfigurationFactoryMethodThatPopulatesDepe /// /// User could enable or disable LegacyCorrelationHeadersInjection of DependencyCollectorOptions. - /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. + /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. /// /// /// DefaultConfiguration - calls services.AddApplicationInsightsTelemetry() which reads IConfiguration from user application automatically. /// SuppliedConfiguration - invokes services.AddApplicationInsightsTelemetry(configuration) where IConfiguration object is supplied by caller. - /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. - /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. + /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. + /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. /// /// Sets the value for property EnableLegacyCorrelationHeadersInjection. [Theory] @@ -1018,13 +1037,13 @@ public static void ConfigureRequestTrackingTelemetryDefaultOptions() /// /// User could enable or disable RequestCollectionOptions by setting InjectResponseHeaders, TrackExceptions and EnableW3CDistributedTracing. - /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. + /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. /// /// /// DefaultConfiguration - calls services.AddApplicationInsightsTelemetry() which reads IConfiguration from user application automatically. /// SuppliedConfiguration - invokes services.AddApplicationInsightsTelemetry(configuration) where IConfiguration object is supplied by caller. - /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. - /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. + /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. + /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. /// /// Sets the value for property InjectResponseHeaders, TrackExceptions and EnableW3CDistributedTracing. [Theory] @@ -1121,13 +1140,13 @@ public static void AddsAddaptiveSamplingServiceToTheConfigurationByDefault() /// /// User could enable or disable sampling by setting EnableAdaptiveSampling. - /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. + /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. /// /// /// DefaultConfiguration - calls services.AddApplicationInsightsTelemetry() which reads IConfiguration from user application automatically. /// SuppliedConfiguration - invokes services.AddApplicationInsightsTelemetry(configuration) where IConfiguration object is supplied by caller. - /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. - /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. + /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. + /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. /// /// Sets the value for property EnableAdaptiveSampling. [Theory] @@ -1335,13 +1354,13 @@ public static void AddsAutoCollectedMetricsExtractorProcessorToTheConfigurationB /// /// User could enable or disable auto collected metrics by setting AddAutoCollectedMetricExtractor. - /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. + /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. /// /// /// DefaultConfiguration - calls services.AddApplicationInsightsTelemetry() which reads IConfiguration from user application automatically. /// SuppliedConfiguration - invokes services.AddApplicationInsightsTelemetry(configuration) where IConfiguration object is supplied by caller. - /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. - /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. + /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. + /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. /// /// Sets the value for property AddAutoCollectedMetricExtractor. [Theory] @@ -1386,13 +1405,13 @@ public static void DoesNotAddQuickPulseProcessorToConfigurationIfExplicitlyContr /// /// User could enable or disable AuthenticationTrackingJavaScript by setting EnableAuthenticationTrackingJavaScript. - /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. + /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. /// /// /// DefaultConfiguration - calls services.AddApplicationInsightsTelemetry() which reads IConfiguration from user application automatically. /// SuppliedConfiguration - invokes services.AddApplicationInsightsTelemetry(configuration) where IConfiguration object is supplied by caller. - /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. - /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. + /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. + /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. /// /// Sets the value for property EnableAuthenticationTrackingJavaScript. [Theory] @@ -1523,8 +1542,8 @@ public static void NullLoggerCallbackAlowed() /// /// Creates two copies of ApplicationInsightsServiceOptions. First object is created by calling services.AddApplicationInsightsTelemetry() or services.AddApplicationInsightsTelemetry(config). /// Second object is created directly from configuration file without using any of SDK functionality. - /// Compares ApplicationInsightsServiceOptions object from dependency container and one created directly from configuration. - /// This proves all that SDK read configuration successfully from configuration file. + /// Compares ApplicationInsightsServiceOptions object from dependency container and one created directly from configuration. + /// This proves all that SDK read configuration successfully from configuration file. /// Properties from appSettings.json, appsettings.{env.EnvironmentName}.json and Environmental Variables are read if no IConfiguration is supplied or used in an application. /// /// If this is set, read value from appsettings.json, else from passed file. @@ -1551,7 +1570,7 @@ public static void ReadsSettingsFromDefaultAndSuppliedConfiguration(bool readFro // VALIDATE // Generate config and don't pass to services - // this is directly generated from config file + // this is directly generated from config file // which could be used to validate the data from dependency container if (!readFromAppSettings) @@ -1617,7 +1636,7 @@ public static void ReadsSettingsFromDefaultConfigurationWithEnvOverridingConfig( // This line mimics the default behavior by CreateDefaultBuilder services.AddSingleton(config); - // ACT + // ACT services.AddApplicationInsightsTelemetry(); // VALIDATE @@ -1654,7 +1673,7 @@ public static void VerifiesIkeyProvidedInAddApplicationInsightsAlwaysWinsOverOth // This line mimics the default behavior by CreateDefaultBuilder services.AddSingleton(config); - // ACT + // ACT services.AddApplicationInsightsTelemetry("userkey"); // VALIDATE @@ -1689,7 +1708,7 @@ public static void ReadsFromAppSettingsIfNoSettingsFoundInDefaultConfiguration() { // Host.CreateDefaultBuilder() in .NET Core 3.0 adds appsetting.json and env variable // to configuration and is made available for constructor injection. - // This test validates that SDK does not throw any error if it cannot find + // This test validates that SDK does not throw any error if it cannot find // application insights configuration in default IConfiguration. // ARRANGE var jsonFullPath = Path.Combine(Directory.GetCurrentDirectory(), "content", "sample-appsettings_dontexist.json"); @@ -1698,7 +1717,7 @@ public static void ReadsFromAppSettingsIfNoSettingsFoundInDefaultConfiguration() // This line mimics the default behavior by CreateDefaultBuilder services.AddSingleton(config); - // ACT + // ACT services.AddApplicationInsightsTelemetry(); // VALIDATE @@ -1711,5 +1730,13 @@ public static void ReadsFromAppSettingsIfNoSettingsFoundInDefaultConfiguration() Assert.Equal(appSettingsConfig["ApplicationInsights:InstrumentationKey"], telemetryConfiguration.InstrumentationKey); } + + private sealed class TestService : ITestService + { + } + + private interface ITestService + { + } } } diff --git a/NETCORE/test/Microsoft.ApplicationInsights.AspNetCore.Tests/Extensions/ApplicationInsightsExtensionsTests/AddSingletonTests.cs b/NETCORE/test/Microsoft.ApplicationInsights.AspNetCore.Tests/Extensions/ApplicationInsightsExtensionsTests/AddSingletonTests.cs deleted file mode 100644 index 871788c39d..0000000000 --- a/NETCORE/test/Microsoft.ApplicationInsights.AspNetCore.Tests/Extensions/ApplicationInsightsExtensionsTests/AddSingletonTests.cs +++ /dev/null @@ -1,96 +0,0 @@ -using System; -using System.Collections.Generic; -using System.ComponentModel; -using System.Linq; -using System.Text; - -using Microsoft.ApplicationInsights.Extensibility; -using Microsoft.ApplicationInsights.Extensibility.Implementation.Tracing; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.DependencyInjection.Test; - -using Xunit; - -namespace Microsoft.ApplicationInsights.AspNetCore.Tests.Extensions.ApplicationInsightsExtensionsTests -{ - /// - /// When working with IServiceCossection, it can store three types of Implementations: - /// ImplementationFactory, ImplementationInstance, and ImplementationType. - /// We want to be able to add a Singleton but only if a user hasn't already done so. - /// This class is to test all the various edge cases. - /// - public class AddSingletonTests : BaseTestClass - { - [Fact] - /// - /// When iterating the services collection, if we forget to check the ImplementationFactory this will throw NullRefExceptions. - /// - public static void VerifyAddSingletonIfNotExists_CanDetectImplemnationFactory() - { - var services = GetServiceCollectionWithContextAccessor(); - services.AddTransient(MyServiceFactory.Create); - - services.AddSingletonIfNotExists(); - - //VALIDATE - IServiceProvider serviceProvider = services.BuildServiceProvider(); - var count = serviceProvider.GetServices().OfType().Count(); - Assert.Equal(1, count); - } - - [Fact] - /// - /// AddSingleton is the most common way to register a type. The framework methods will check for this. - /// This test is to confirm that we haven't broken any expected behavior. - /// - public static void VerifyAddSingletonIfNotExists_CanDetectImplemnationType() - { - var services = GetServiceCollectionWithContextAccessor(); - services.AddSingleton(); - - services.AddSingletonIfNotExists(); - - //VALIDATE - IServiceProvider serviceProvider = services.BuildServiceProvider(); - var count = serviceProvider.GetServices().OfType().Count(); - Assert.Equal(1, count); - } - - [Fact] - /// - /// This is the case that's hardest to check for. The framework methods won't check for this. - /// - public static void VerifyAddSingletonIfNotExists_CanDetectImplemnationInstance() - { - var services = GetServiceCollectionWithContextAccessor(); - services.AddSingleton(new MyService()); - - services.AddSingletonIfNotExists(); - - //VALIDATE - IServiceProvider serviceProvider = services.BuildServiceProvider(); - var count = serviceProvider.GetServices().OfType().Count(); - Assert.Equal(1, count); - } - - private interface IService - { - } - - private class MyService : IService - { - } - - private class MyService2 : IService - { - } - - private static class MyServiceFactory - { - public static MyService Create(IServiceProvider serviceProvider) - { - return new MyService(); - } - } - } -} diff --git a/NETCORE/test/Microsoft.ApplicationInsights.AspNetCore.Tests/Microsoft.ApplicationInsights.AspNetCore.Tests.csproj b/NETCORE/test/Microsoft.ApplicationInsights.AspNetCore.Tests/Microsoft.ApplicationInsights.AspNetCore.Tests.csproj index 39bb5f9b7e..047711bcbc 100644 --- a/NETCORE/test/Microsoft.ApplicationInsights.AspNetCore.Tests/Microsoft.ApplicationInsights.AspNetCore.Tests.csproj +++ b/NETCORE/test/Microsoft.ApplicationInsights.AspNetCore.Tests/Microsoft.ApplicationInsights.AspNetCore.Tests.csproj @@ -18,11 +18,12 @@ + - diff --git a/NETCORE/test/Microsoft.ApplicationInsights.WorkerService.Tests/ExtensionsTest.cs b/NETCORE/test/Microsoft.ApplicationInsights.WorkerService.Tests/ExtensionsTest.cs index 6c8c134010..518bb65089 100644 --- a/NETCORE/test/Microsoft.ApplicationInsights.WorkerService.Tests/ExtensionsTest.cs +++ b/NETCORE/test/Microsoft.ApplicationInsights.WorkerService.Tests/ExtensionsTest.cs @@ -123,6 +123,25 @@ private static ServiceCollection CreateServicesAndAddApplicationinsightsWorker(A return services; } + [Fact] + public static void TelemetryModuleResolvableWhenKeyedServiceRegistered() + { + // Note: This test verifies a regression doesn't get introduced for: + // https://github.com/dotnet/extensions/issues/5222 + + var services = new ServiceCollection(); + + services.AddKeyedSingleton(typeof(ITestService), serviceKey: new(), implementationType: typeof(TestService)); + services.AddKeyedSingleton(typeof(ITestService), serviceKey: new(), implementationInstance: new TestService()); + services.AddKeyedSingleton(typeof(ITestService), serviceKey: new(), implementationFactory: (sp, key) => new TestService()); + + services.AddApplicationInsightsTelemetryWorkerService(); + + using var sp = services.BuildServiceProvider(); + + var telmetryModule = sp.GetRequiredService(); + } + [Theory] [InlineData(typeof(ITelemetryInitializer), typeof(ApplicationInsights.WorkerService.TelemetryInitializers.DomainNameRoleInstanceTelemetryInitializer), ServiceLifetime.Singleton)] [InlineData(typeof(ITelemetryInitializer), typeof(AzureWebAppRoleEnvironmentTelemetryInitializer), ServiceLifetime.Singleton)] @@ -168,7 +187,7 @@ public void ReadsSettingsFromSuppliedConfiguration() this.output.WriteLine("json:" + jsonFullPath); var config = new ConfigurationBuilder().AddJsonFile(jsonFullPath).Build(); var services = new ServiceCollection(); - + services.AddApplicationInsightsTelemetryWorkerService(config); IServiceProvider serviceProvider = services.BuildServiceProvider(); var telemetryConfiguration = serviceProvider.GetRequiredService(); @@ -191,9 +210,9 @@ public void ReadsSettingsFromDefaultConfiguration() // This line mimics the default behavior by CreateDefaultBuilder services.AddSingleton(config); - // ACT + // ACT services.AddApplicationInsightsTelemetryWorkerService(); - + // VALIDATE IServiceProvider serviceProvider = services.BuildServiceProvider(); var telemetryConfiguration = serviceProvider.GetRequiredService(); @@ -203,7 +222,7 @@ public void ReadsSettingsFromDefaultConfiguration() } /// - /// Tests that the connection string can be read from a JSON file by the configuration factory. + /// Tests that the connection string can be read from a JSON file by the configuration factory. /// /// /// Calls services.AddApplicationInsightsTelemetryWorkerService() when the value is true and reads IConfiguration from user application automatically. @@ -237,7 +256,7 @@ public void ReadsSettingsFromDefaultConfigurationWithEnvOverridingConfig() // to configuration and is made available for constructor injection. // this test validates that SDK reads settings from this configuration by default // and gives priority to the ENV variables than the one from config. - + // ARRANGE Environment.SetEnvironmentVariable("APPINSIGHTS_INSTRUMENTATIONKEY", TestInstrumentationKeyEnv); Environment.SetEnvironmentVariable("APPINSIGHTS_ENDPOINTADDRESS", TestEndPointEnv); @@ -253,7 +272,7 @@ public void ReadsSettingsFromDefaultConfigurationWithEnvOverridingConfig() // This line mimics the default behavior by CreateDefaultBuilder services.AddSingleton(config); - // ACT + // ACT services.AddApplicationInsightsTelemetryWorkerService(); // VALIDATE @@ -287,7 +306,7 @@ public void VerifiesIkeyProvidedInAddApplicationInsightsAlwaysWinsOverOtherOptio // This line mimics the default behavior by CreateDefaultBuilder services.AddSingleton(config); - // ACT + // ACT services.AddApplicationInsightsTelemetryWorkerService("userkey"); // VALIDATE @@ -306,7 +325,7 @@ public void DoesNoThrowIfNoSettingsFound() { // Host.CreateDefaultBuilder() in .NET Core 3.0 adds appsetting.json and env variable // to configuration and is made available for constructor injection. - // This test validates that SDK does not throw any error if it cannot find + // This test validates that SDK does not throw any error if it cannot find // application insights configuration in default IConfiguration. // ARRANGE var jsonFullPath = Path.Combine(Directory.GetCurrentDirectory(), "content", "sample-appsettings_dontexist.json"); @@ -316,7 +335,7 @@ public void DoesNoThrowIfNoSettingsFound() // This line mimics the default behavior by CreateDefaultBuilder services.AddSingleton(config); - // ACT + // ACT services.AddApplicationInsightsTelemetryWorkerService(); // VALIDATE @@ -330,7 +349,7 @@ public void VerifyAddAIWorkerServiceSetsUpDefaultConfigurationAndModules() { var services = new ServiceCollection(); - // ACT + // ACT services.AddApplicationInsightsTelemetryWorkerService("ikey"); // VALIDATE @@ -345,7 +364,7 @@ public void VerifyAddAIWorkerServiceSetsUpDefaultConfigurationAndModules() // AppID var channel = serviceProvider.GetRequiredService(); - Assert.NotNull(channel); + Assert.NotNull(channel); Assert.True(channel is ServerTelemetryChannel); // TelemetryModules @@ -399,7 +418,7 @@ public static void RegistersTelemetryConfigurationFactoryMethodThatPopulatesEven // ACT IServiceProvider serviceProvider = services.BuildServiceProvider(); - var modules = serviceProvider.GetServices(); + var modules = serviceProvider.GetServices(); var telemetryConfiguration = serviceProvider.GetRequiredService(); var eventCounterModule = modules.OfType().Single(); @@ -414,7 +433,7 @@ public void VerifyAddAIWorkerServiceUsesTelemetryInitializerAddedToDI() var services = new ServiceCollection(); var telemetryInitializer = new FakeTelemetryInitializer(); - // ACT + // ACT services.AddApplicationInsightsTelemetryWorkerService(); services.AddSingleton(telemetryInitializer); @@ -431,7 +450,7 @@ public void VerifyAddAIWorkerServiceUsesTelemetryChannelAddedToDI() var services = new ServiceCollection(); var telChannel = new ServerTelemetryChannel() {StorageFolder = "c:\\mycustom" }; - // ACT + // ACT services.AddApplicationInsightsTelemetryWorkerService("ikey"); services.AddSingleton(telChannel); @@ -449,7 +468,7 @@ public void VerifyAddAIWorkerServiceRespectsAIOptions() { var services = new ServiceCollection(); - // ACT + // ACT var aiOptions = new ApplicationInsightsServiceOptions(); aiOptions.AddAutoCollectedMetricExtractor = false; aiOptions.EnableAdaptiveSampling = false; @@ -478,7 +497,7 @@ public static void SanityCheckRoleInstance() services.AddApplicationInsightsTelemetryWorkerService(); IServiceProvider serviceProvider = services.BuildServiceProvider(); - // Request TC from DI which would be made with the default TelemetryConfiguration which should + // Request TC from DI which would be made with the default TelemetryConfiguration which should // contain the telemetry initializer capable of populate node name and role instance name. var tc = serviceProvider.GetRequiredService(); var mockItem = new EventTelemetry(); @@ -487,19 +506,19 @@ public static void SanityCheckRoleInstance() // This is expected to run all TI and populate the node name and role instance. tc.Initialize(mockItem); - // VERIFY + // VERIFY Assert.Contains(expected, mockItem.Context.Cloud.RoleInstance, StringComparison.CurrentCultureIgnoreCase); } /// /// User could enable or disable PerformanceCounterCollectionModule by setting EnablePerformanceCounterCollectionModule. - /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. + /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. /// /// /// DefaultConfiguration - calls services.AddApplicationInsightsTelemetryWorkerService() which reads IConfiguration from user application automatically. /// SuppliedConfiguration - invokes services.AddApplicationInsightsTelemetryWorkerService(configuration) where IConfiguration object is supplied by caller. - /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. - /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. + /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. + /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. /// /// Sets the value for property EnablePerformanceCounterCollectionModule. [Theory] @@ -520,13 +539,13 @@ public static void UserCanEnableAndDisablePerfCollectorModule(string configType, /// /// User could enable or disable EventCounterCollectionModule by setting EnableEventCounterCollectionModule. - /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. + /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. /// /// /// DefaultConfiguration - calls services.AddApplicationInsightsTelemetryWorkerService() which reads IConfiguration from user application automatically. /// SuppliedConfiguration - invokes services.AddApplicationInsightsTelemetryWorkerService(configuration) where IConfiguration object is supplied by caller. - /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. - /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. + /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. + /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. /// /// Sets the value for property EnableEventCounterCollectionModule. [Theory] @@ -547,13 +566,13 @@ public static void UserCanEnableAndDisableEventCounterCollectorModule(string con /// /// User could enable or disable DependencyTrackingTelemetryModule by setting EnableDependencyTrackingTelemetryModule. - /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. + /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. /// /// /// DefaultConfiguration - calls services.AddApplicationInsightsTelemetryWorkerService() which reads IConfiguration from user application automatically. /// SuppliedConfiguration - invokes services.AddApplicationInsightsTelemetryWorkerService(configuration) where IConfiguration object is supplied by caller. - /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. - /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. + /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. + /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. /// /// Sets the value for property EnableDependencyTrackingTelemetryModule. [Theory] @@ -574,13 +593,13 @@ public static void UserCanEnableAndDisableDependencyCollectorModule(string confi /// /// User could enable or disable QuickPulseCollectorModule by setting EnableQuickPulseMetricStream. - /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. + /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. /// /// /// DefaultConfiguration - calls services.AddApplicationInsightsTelemetryWorkerService() which reads IConfiguration from user application automatically. /// SuppliedConfiguration - invokes services.AddApplicationInsightsTelemetryWorkerService(configuration) where IConfiguration object is supplied by caller. - /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. - /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. + /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. + /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. /// /// Sets the value for property EnableQuickPulseMetricStream. [Theory] @@ -601,13 +620,13 @@ public static void UserCanEnableAndDisableQuickPulseCollectorModule(string confi /// /// User could enable or disable AzureInstanceMetadataModule by setting EnableAzureInstanceMetadataTelemetryModule. - /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. + /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. /// /// /// DefaultConfiguration - calls services.AddApplicationInsightsTelemetryWorkerService() which reads IConfiguration from user application automatically. /// SuppliedConfiguration - invokes services.AddApplicationInsightsTelemetryWorkerService(configuration) where IConfiguration object is supplied by caller. - /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. - /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. + /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. + /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. /// /// Sets the value for property EnableAzureInstanceMetadataTelemetryModule. [Theory] @@ -690,13 +709,13 @@ public static void UserCanEnableAndDisableHeartbeatFeature(string configType, bo /// /// User could enable or disable LegacyCorrelationHeadersInjection of DependencyCollectorOptions. - /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. + /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. /// /// /// DefaultConfiguration - calls services.AddApplicationInsightsTelemetryWorkerService() which reads IConfiguration from user application automatically. /// SuppliedConfiguration - invokes services.AddApplicationInsightsTelemetryWorkerService(configuration) where IConfiguration object is supplied by caller. - /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. - /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. + /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. + /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. /// /// Sets the value for property EnableLegacyCorrelationHeadersInjection. [Theory] @@ -740,13 +759,13 @@ public static void RegistersTelemetryConfigurationFactoryMethodThatPopulatesDepe /// /// User could enable or disable sampling by setting EnableAdaptiveSampling. - /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. + /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. /// /// /// DefaultConfiguration - calls services.AddApplicationInsightsTelemetryWorkerService() which reads IConfiguration from user application automatically. /// SuppliedConfiguration - invokes services.AddApplicationInsightsTelemetryWorkerService(configuration) where IConfiguration object is supplied by caller. - /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. - /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. + /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. + /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. /// /// Sets the value for property EnableAdaptiveSampling. [Theory] @@ -781,13 +800,13 @@ public static void DoesNotAddSamplingToConfigurationIfExplicitlyControlledThroug /// /// User could enable or disable auto collected metrics by setting AddAutoCollectedMetricExtractor. - /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. + /// This configuration can be read from a JSON file by the configuration factory or through code by passing ApplicationInsightsServiceOptions. /// /// /// DefaultConfiguration - calls services.AddApplicationInsightsTelemetryWorkerService() which reads IConfiguration from user application automatically. /// SuppliedConfiguration - invokes services.AddApplicationInsightsTelemetryWorkerService(configuration) where IConfiguration object is supplied by caller. - /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. - /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. + /// Code - Caller creates an instance of ApplicationInsightsServiceOptions and passes it. This option overrides all configuration being used in JSON file. + /// There is a special case where NULL values in these properties - InstrumentationKey, ConnectionString, EndpointAddress and DeveloperMode are overwritten. We check IConfiguration object to see if these properties have values, if values are present then we override it. /// /// Sets the value for property AddAutoCollectedMetricExtractor. [Theory] @@ -822,8 +841,8 @@ public static void DoesNotAddAutoCollectedMetricsExtractorToConfigurationIfExpli /// /// Creates two copies of ApplicationInsightsServiceOptions. First object is created by calling services.AddApplicationInsightsTelemetryWorkerService() or services.AddApplicationInsightsTelemetryWorkerService(config). /// Second object is created directly from configuration file without using any of SDK functionality. - /// Compares ApplicationInsightsServiceOptions object from dependency container and one created directly from configuration. - /// This proves all that SDK read configuration successfully from configuration file. + /// Compares ApplicationInsightsServiceOptions object from dependency container and one created directly from configuration. + /// This proves all that SDK read configuration successfully from configuration file. /// Properties from appSettings.json, appsettings.{env.EnvironmentName}.json and Environmental Variables are read if no IConfiguration is supplied or used in an application. /// /// If this is set, read value from appsettings.json, else from passed file. @@ -850,7 +869,7 @@ public static void ReadsSettingsFromDefaultAndSuppliedConfiguration(bool readFro // VALIDATE // Generate config and don't pass to services - // this is directly generated from config file + // this is directly generated from config file // which could be used to validate the data from dependency container if (!readFromAppSettings) @@ -895,6 +914,14 @@ private static int GetTelemetryProcessorsCountInConfigurationDefaultSink(Tele { return telemetryConfiguration.DefaultTelemetrySink.TelemetryProcessors.Where(processor => processor.GetType() == typeof(T)).Count(); } + + private sealed class TestService : ITestService + { + } + + private interface ITestService + { + } } internal class FakeTelemetryInitializer : ITelemetryInitializer @@ -905,7 +932,7 @@ public FakeTelemetryInitializer() public void Initialize(ITelemetry telemetry) { - + } } } diff --git a/NETCORE/test/Microsoft.ApplicationInsights.WorkerService.Tests/Microsoft.ApplicationInsights.WorkerService.Tests.csproj b/NETCORE/test/Microsoft.ApplicationInsights.WorkerService.Tests/Microsoft.ApplicationInsights.WorkerService.Tests.csproj index 6b1f427966..5b50f81088 100644 --- a/NETCORE/test/Microsoft.ApplicationInsights.WorkerService.Tests/Microsoft.ApplicationInsights.WorkerService.Tests.csproj +++ b/NETCORE/test/Microsoft.ApplicationInsights.WorkerService.Tests/Microsoft.ApplicationInsights.WorkerService.Tests.csproj @@ -21,6 +21,7 @@ + @@ -33,5 +34,5 @@ Always - +