App insights key lookup occurs only when configuration option is set (#…

…69)

* lookup does not occur in production, refactoring,

* merge refactoring

* removed duplicate registration

src/TesApi.Web/Management/ArmResourceInformationFinder.cs

@@ -0,0 +1,77 @@
+using System;
+using System.Linq;
+using System.Threading.Tasks;
+using Microsoft.Azure.Management.ApplicationInsights.Management;
+using Microsoft.Azure.Management.Batch;
+using Microsoft.Azure.Services.AppAuthentication;
+using Microsoft.Rest;
+
+namespace TesApi.Web.Management
+{
+    /// <summary>
+    /// Provides utility methods to find resource information using the TES service identity
+    /// </summary>
+    public class ArmResourceInformationFinder
+    {
+        /// <summary>
+        /// Looks up the AppInsights instrumentation key in subscriptions the TES services has access to 
+        /// </summary>
+        /// <param name="accountName"></param>
+        /// <returns></returns>
+        public static async Task<string> GetAppInsightsInstrumentationKeyAsync(string accountName)
+        {
+            var azureClient = await AzureManagementClientsFactory.GetAzureManagementClientAsync();
+            var subscriptionIds = (await azureClient.Subscriptions.ListAsync()).Select(s => s.SubscriptionId);
+
+            var credentials = new TokenCredentials(await GetAzureAccessTokenAsync());
+
+            foreach (var subscriptionId in subscriptionIds)
+            {
+                var app = (await new ApplicationInsightsManagementClient(credentials) { SubscriptionId = subscriptionId }.Components.ListAsync())
+                    .FirstOrDefault(a => a.ApplicationId.Equals(accountName, StringComparison.OrdinalIgnoreCase));
+
+                if (app is not null)
+                {
+                    return app.InstrumentationKey;
+                }
+            }
+
+            return null;
+        }
+        //TODO: refactor this to use Azure Identity token provider. 
+        private static Task<string> GetAzureAccessTokenAsync(string resource = "https://management.azure.com/")
+        {
+            return new AzureServiceTokenProvider().GetAccessTokenAsync(resource);
+        }
+
+        /// <summary>
+        /// Attempts to get the batch resource information using the ARM api.
+        /// Returns null if the resource was not found or the account does not have access.
+        /// </summary>
+        /// <param name="batchAccountName">batch account name</param>
+        /// <returns></returns>
+        public static async Task<BatchAccountResourceInformation> TryGetResourceInformationFromAccountNameAsync(string batchAccountName)
+        {
+            //TODO: look if a newer version of the management SDK provides a simpler way to look for this information .
+            var tokenCredentials = new TokenCredentials(await GetAzureAccessTokenAsync());
+            var azureClient = await AzureManagementClientsFactory.GetAzureManagementClientAsync();
+
+            var subscriptionIds = (await azureClient.Subscriptions.ListAsync()).Select(s => s.SubscriptionId);
+
+            foreach (var subId in subscriptionIds)
+            {
+                using var batchClient = new BatchManagementClient(tokenCredentials) { SubscriptionId = subId };
+
+                var batchAccount = (await batchClient.BatchAccount.ListAsync())
+                    .FirstOrDefault(a => a.Name.Equals(batchAccountName, StringComparison.OrdinalIgnoreCase));
+
+                if (batchAccount is not null)
+                {
+                    return BatchAccountResourceInformation.FromBatchResourceId(batchAccount.Id, batchAccount.Location);
+                }
+            }
+
+            return null;
+        }
+    }
+}

src/TesApi.Web/Management/AzureManagementClientsFactory.cs

@@ -2,7 +2,6 @@
 // Licensed under the MIT License.
 
 using System;
-using System.Linq;
 using System.Threading.Tasks;
 using Microsoft.Azure.Management.Batch;
 using Microsoft.Azure.Management.ResourceManager.Fluent;
@@ -71,36 +70,6 @@ public async Task<BatchManagementClient> CreateBatchAccountManagementClient()
         public async Task<FluentAzure.IAuthenticated> CreateAzureManagementClientAsync()
             => await AzureManagementClientsFactory.GetAzureManagementClientAsync();
 
-        /// <summary>
-        /// Attempts to get the batch resource information using the ARM api.
-        /// Returns null if the resource was not found or the account does not have access.
-        /// </summary>
-        /// <param name="batchAccountName">batch account name</param>
-        /// <returns></returns>
-        public static async Task<BatchAccountResourceInformation> TryGetResourceInformationFromAccountNameAsync(string batchAccountName)
-        {
-            //TODO: look if a newer version of the management SDK provides a simpler way to look for this information .
-            var tokenCredentials = new TokenCredentials(await GetAzureAccessTokenAsync());
-            var azureClient = await GetAzureManagementClientAsync();
-
-            var subscriptionIds = (await azureClient.Subscriptions.ListAsync()).Select(s => s.SubscriptionId);
-
-            foreach (var subId in subscriptionIds)
-            {
-                using var batchClient = new BatchManagementClient(tokenCredentials) { SubscriptionId = subId };
-
-                var batchAccount = (await batchClient.BatchAccount.ListAsync())
-                    .FirstOrDefault(a => a.Name.Equals(batchAccountName, StringComparison.OrdinalIgnoreCase));
-
-                if (batchAccount is not null)
-                {
-                    return BatchAccountResourceInformation.FromBatchResourceId(batchAccount.Id, batchAccount.Location);
-                }
-            }
-
-            return null;
-        }
-
         /// <summary>
         /// Creates a new instance of Azure Management client
         /// </summary>

src/TesApi.Web/Management/Configuration/BatchAccountOptions.cs

@@ -11,7 +11,7 @@ public class BatchAccountOptions
         /// <summary>
         /// Configuration section.
         /// </summary>
-        public const string BatchAccount = "BatchAccount";
+        public const string SectionName = "BatchAccount";
         /// <summary>
         /// Default Azure offer durable id. 
         /// </summary>

src/TesApi.Web/Management/Configuration/ContainerRegistryOptions.cs

@@ -11,7 +11,7 @@ public class ContainerRegistryOptions
     /// <summary>
     /// Configuration section.
     /// </summary>
-    public const string ContainerRegistrySection = "ContainerRegistry";
+    public const string SectionName = "ContainerRegistry";
     /// <summary>
     /// Enables/disables auto-discovery features
     /// </summary>

src/TesApi.Web/Management/Configuration/RetryPolicyOptions.cs

@@ -11,7 +11,7 @@ public class RetryPolicyOptions
         /// <summary>
         /// Retry policy configuration section
         /// </summary>
-        public const string RetryPolicy = "RetryPolicy";
+        public const string SectionName = "RetryPolicy";
 
         private const int DefaultRetryCount = 3;
         private const int DefaultExponentialBackOffExponent = 2;

src/TesApi.Web/Management/Configuration/TerraOptions.cs

@@ -11,7 +11,7 @@ public class TerraOptions
     /// <summary>
     /// Terra configuration section
     /// </summary>
-    public const string Terra = "Terra";
+    public const string SectionName = "Terra";
     private const int DefaultSasTokenExpirationInSeconds = 60 * 24 * 3; // 3 days
 
     /// <summary>

src/TesApi.Web/Startup.cs

@@ -61,20 +61,19 @@ public Startup(IConfiguration configuration, ILogger<Startup> logger, IWebHostEn
         /// </summary>
         /// <param name="services">The Microsoft.Extensions.DependencyInjection.IServiceCollection to add the services to.</param>
         public void ConfigureServices(IServiceCollection services)
-            => services
-                .Configure<BatchAccountOptions>(Configuration.GetSection(BatchAccountOptions.BatchAccount))
+        {
+            services
+                .Configure<BatchAccountOptions>(Configuration.GetSection(BatchAccountOptions.SectionName))
                 .Configure<CosmosDbOptions>(Configuration.GetSection(CosmosDbOptions.CosmosDbAccount))
-                .Configure<RetryPolicyOptions>(Configuration.GetSection(RetryPolicyOptions.RetryPolicy))
-                .Configure<TerraOptions>(Configuration.GetSection(TerraOptions.Terra))
-                .Configure<ContainerRegistryOptions>(Configuration.GetSection(ContainerRegistryOptions.ContainerRegistrySection))
+                .Configure<RetryPolicyOptions>(Configuration.GetSection(RetryPolicyOptions.SectionName))
+                .Configure<TerraOptions>(Configuration.GetSection(TerraOptions.SectionName))
+                .Configure<ContainerRegistryOptions>(Configuration.GetSection(ContainerRegistryOptions.SectionName))
                 .AddSingleton<IAppCache, CachingService>()
-                .AddSingleton(CreateBatchPoolManagerFromConfiguration)
-
-                .AddSingleton<AzureProxy, AzureProxy>()
-
+                .AddSingleton<AzureProxy>()
                 .AddSingleton(CreateCosmosDbRepositoryFromConfiguration)
                 .AddSingleton<IBatchPoolFactory, BatchPoolFactory>()
                 .AddTransient<BatchPool>()
+                .AddSingleton(CreateBatchPoolManagerFromConfiguration)
 
                 .AddControllers()
                 .AddNewtonsoftJson(opts =>
@@ -97,9 +96,8 @@ public void ConfigureServices(IServiceCollection services)
                 .AddSingleton<IBatchSkuInformationProvider, PriceApiBatchSkuInformationProvider>()
                 .AddSingleton(CreateBatchAccountResourceInformation)
                 .AddSingleton(CreateBatchQuotaProviderFromConfiguration)
-                .AddSingleton<AzureManagementClientsFactory, AzureManagementClientsFactory>()
-                //.AddSingleton<ArmBatchQuotaProvider, ArmBatchQuotaProvider>() //added so config utils gets the arm implementation, to be removed once config utils is refactored.
-                .AddSingleton<ConfigurationUtils, ConfigurationUtils>()
+                .AddSingleton<AzureManagementClientsFactory>()
+                .AddSingleton<ConfigurationUtils>()
                 .AddSingleton<TokenCredential>(s => new DefaultAzureCredential())
 
                 .AddSwaggerGen(c =>
@@ -132,29 +130,33 @@ public void ConfigureServices(IServiceCollection services)
                 .AddHostedService<Scheduler>()
                 .AddHostedService<DeleteCompletedBatchJobsHostedService>()
                 .AddHostedService<DeleteOrphanedBatchJobsHostedService>()
-                .AddHostedService<DeleteOrphanedAutoPoolsHostedService>()
-                //.AddHostedService<RefreshVMSizesAndPricesHostedService>()
+                .AddHostedService<DeleteOrphanedAutoPoolsHostedService>();
+            //.AddHostedService<RefreshVMSizesAndPricesHostedService>()
 
 
-                //Configure AppInsights Azure Service when in PRODUCTION environment
-                .IfThenElse(hostingEnvironment.IsProduction(),
-                    s =>
-                    {
-                        var applicationInsightsAccountName = Configuration["ApplicationInsightsAccountName"];
-                        var instrumentationKey = AzureProxy.GetAppInsightsInstrumentationKeyAsync(applicationInsightsAccountName).Result;
+            AddAppInsightsWithDefaultOrLookingUpTheConnectionString(services);
+        }
 
-                        if (instrumentationKey is not null)
-                        {
-                            var connectionString = $"InstrumentationKey={instrumentationKey}";
-                            return s.AddApplicationInsightsTelemetry(options =>
-                            {
-                                options.ConnectionString = connectionString;
-                            });
-                        }
-
-                        return s;
-                    },
-                s => s.AddApplicationInsightsTelemetry());
+        private void AddAppInsightsWithDefaultOrLookingUpTheConnectionString(IServiceCollection services)
+        {
+            var accountName = Configuration["ApplicationInsightsAccountName"];
+
+            if (string.IsNullOrEmpty(accountName))
+            {
+                //use default settings that will use the app insights configuration
+                services.AddApplicationInsightsTelemetry();
+                return;
+            }
+
+            services.AddApplicationInsightsTelemetry(s =>
+            {
+                var instrumentationKey = ArmResourceInformationFinder
+                    .GetAppInsightsInstrumentationKeyAsync(accountName)
+                    .Result;
+
+                s.ConnectionString = $"InstrumentationKey={instrumentationKey}";
+            });
+        }
 
         private IBatchQuotaProvider CreateBatchQuotaProviderFromConfiguration(IServiceProvider services)
         {
@@ -249,7 +251,7 @@ private BatchAccountResourceInformation CreateBatchAccountResourceInformation(IS
             if (string.IsNullOrWhiteSpace(options.Value.AppKey))
             {
                 //we are assuming Arm with MI/RBAC if no key is provided. Try to get info from the batch account.
-                var task = AzureManagementClientsFactory.TryGetResourceInformationFromAccountNameAsync(options.Value.AccountName);
+                var task = ArmResourceInformationFinder.TryGetResourceInformationFromAccountNameAsync(options.Value.AccountName);
                 task.Wait();
 
                 if (task.Result == null)