refactor: Move Azure Monitor SDK Feature Flag to Runtime Configuration (

#2507) * github action image build * github action image build * add run time flag for azure monitor SDK integration * remove SDK flag out of metrics declaration * remove SDK flag out of metrics declaration * revert github action
tomkerkhove · Jun 4, 2024 · 2c1ac6b · 2c1ac6b
1 parent d1925df
commit 2c1ac6b
Show file tree

Hide file tree

Showing 11 changed files with 52 additions and 38 deletions.
diff --git a/src/Promitor.Agents.Scraper/AzureMonitorClientFactory.cs b/src/Promitor.Agents.Scraper/AzureMonitorClientFactory.cs
@@ -31,14 +31,14 @@ public class AzureMonitorClientFactory
         /// <param name="azureMonitorIntegrationConfiguration">Options for Azure Monitor integration</param>
         /// <param name="azureMonitorLoggingConfiguration">Options for Azure Monitor logging</param>
         /// <param name="loggerFactory">Factory to create loggers with</param>
-        /// <param name="useAzureMonitorSdk">Whether to use the new Azure.Monitor.Query package for queries</param>
-        public IAzureMonitorClient CreateIfNotExists(AzureCloud cloud, string tenantId, string subscriptionId, MetricSinkWriter metricSinkWriter, IAzureScrapingSystemMetricsPublisher azureScrapingSystemMetricsPublisher, IMemoryCache resourceMetricDefinitionMemoryCache, IConfiguration configuration, IOptions<AzureMonitorIntegrationConfiguration> azureMonitorIntegrationConfiguration, IOptions<AzureMonitorLoggingConfiguration> azureMonitorLoggingConfiguration, ILoggerFactory loggerFactory, bool useAzureMonitorSdk)
+        public IAzureMonitorClient CreateIfNotExists(AzureCloud cloud, string tenantId, string subscriptionId, MetricSinkWriter metricSinkWriter, IAzureScrapingSystemMetricsPublisher azureScrapingSystemMetricsPublisher, IMemoryCache resourceMetricDefinitionMemoryCache, IConfiguration configuration, IOptions<AzureMonitorIntegrationConfiguration> azureMonitorIntegrationConfiguration, IOptions<AzureMonitorLoggingConfiguration> azureMonitorLoggingConfiguration, ILoggerFactory loggerFactory)
         {   
             if (_azureMonitorClients.TryGetValue(subscriptionId, out var value))
             {
                 return value;
             }
-
+
+            var useAzureMonitorSdk = azureMonitorIntegrationConfiguration.Value.UseAzureMonitorSdk;
             IAzureMonitorClient azureMonitorClient;
             if (useAzureMonitorSdk) {
                 azureMonitorClient = CreateNewAzureMonitorQueryClient(cloud, tenantId, subscriptionId, metricSinkWriter, azureScrapingSystemMetricsPublisher, resourceMetricDefinitionMemoryCache, configuration, azureMonitorIntegrationConfiguration, azureMonitorLoggingConfiguration, loggerFactory);

diff --git a/src/Promitor.Agents.Scraper/Docs/Open-Api.xml b/src/Promitor.Agents.Scraper/Docs/Open-Api.xml
diff --git a/src/Promitor.Agents.Scraper/Scheduling/ResourcesScrapingJob.cs b/src/Promitor.Agents.Scraper/Scheduling/ResourcesScrapingJob.cs
@@ -277,10 +277,10 @@ private async Task ScrapeMetric(ScrapeDefinition<IAzureResourceDefinition> scrap
                     ? scrapeDefinition.Resource.SubscriptionId
                     : _metricsDeclaration.AzureMetadata.SubscriptionId;
                 var azureEnvironent = _metricsDeclaration.AzureMetadata.Cloud.GetAzureEnvironment();
-                Logger.LogInformation("Parsed SDK Config {UseAzureMonitorSdk}", _metricsDeclaration.UseAzureMonitorSdk);
+                Logger.LogInformation("Parsed SDK Config {UseAzureMonitorSdk}", _azureMonitorIntegrationConfiguration.Value.UseAzureMonitorSdk);
                 var azureMonitorClient = _azureMonitorClientFactory.CreateIfNotExists(_metricsDeclaration.AzureMetadata.Cloud, _metricsDeclaration.AzureMetadata.TenantId,
                     resourceSubscriptionId, _metricSinkWriter, _azureScrapingSystemMetricsPublisher, _resourceMetricDefinitionMemoryCache, _configuration,
-                    _azureMonitorIntegrationConfiguration, _azureMonitorLoggingConfiguration, _loggerFactory, _metricsDeclaration.UseAzureMonitorSdk);
+                    _azureMonitorIntegrationConfiguration, _azureMonitorLoggingConfiguration, _loggerFactory);
 
                 var tokenCredential = AzureAuthenticationFactory.GetTokenCredential(azureEnvironent.ManagementEndpoint, _metricsDeclaration.AzureMetadata.TenantId,
                     AzureAuthenticationFactory.GetConfiguredAzureAuthentication(_configuration), new Uri(_metricsDeclaration.AzureMetadata.Cloud.GetAzureEnvironment().AuthenticationEndpoint));

diff --git a/src/Promitor.Core.Scraping/Configuration/Model/MetricsDeclaration.cs b/src/Promitor.Core.Scraping/Configuration/Model/MetricsDeclaration.cs
@@ -5,7 +5,6 @@ namespace Promitor.Core.Scraping.Configuration.Model
 {
     public class MetricsDeclaration
     {
-        public bool UseAzureMonitorSdk { get; set; } = true;
         public AzureMetadata AzureMetadata { get; set; }
         public MetricDefaults MetricDefaults { get; set; } = new MetricDefaults();
         public List<MetricDefinition> Metrics { get; set; } = new List<MetricDefinition>();

diff --git a/src/Promitor.Core.Scraping/Configuration/Serialization/v1/Core/V1Deserializer.cs b/src/Promitor.Core.Scraping/Configuration/Serialization/v1/Core/V1Deserializer.cs
@@ -20,8 +20,6 @@ public V1Deserializer(IDeserializer<AzureMetadataV1> azureMetadataDeserializer,
             Map(definition => definition.Version)
                 .IsRequired()
                 .MapUsing(GetVersion);
-            Map(definition => definition.UseAzureMonitorSdk)
-                .WithDefault(true);
             Map(definition => definition.AzureMetadata)
                 .IsRequired()
                 .MapUsingDeserializer(azureMetadataDeserializer);

diff --git a/src/Promitor.Core.Scraping/Configuration/Serialization/v1/Model/MetricsDeclarationV1.cs b/src/Promitor.Core.Scraping/Configuration/Serialization/v1/Model/MetricsDeclarationV1.cs
@@ -9,7 +9,6 @@ namespace Promitor.Core.Scraping.Configuration.Serialization.v1.Model
     public class MetricsDeclarationV1
     {
         public string Version { get; set; } = SpecVersion.v1.ToString();
-        public bool UseAzureMonitorSdk { get; set; } = true;
         public AzureMetadataV1 AzureMetadata { get; set; }
         public MetricDefaultsV1 MetricDefaults { get; set; }
         public IReadOnlyCollection<MetricDefinitionV1> Metrics { get; set; }

diff --git a/src/Promitor.Integrations.AzureMonitor/Configuration/AzureMonitorIntegrationConfiguration.cs b/src/Promitor.Integrations.AzureMonitor/Configuration/AzureMonitorIntegrationConfiguration.cs
@@ -3,5 +3,6 @@
     public class AzureMonitorIntegrationConfiguration
     {
         public AzureMonitorHistoryConfiguration History { get; set; } = new();
+        public bool UseAzureMonitorSdk { get; set; } = true;
     }
 }
diff --git a/src/Promitor.Integrations.AzureMonitor/LegacyAzureMonitorClient.cs b/src/Promitor.Integrations.AzureMonitor/LegacyAzureMonitorClient.cs
@@ -61,7 +61,7 @@ public LegacyAzureMonitorClient(AzureEnvironment azureCloud, string tenantId, st
             _resourceMetricDefinitionMemoryCache = resourceMetricDefinitionMemoryCache;
             _azureMonitorIntegrationConfiguration = azureMonitorIntegrationConfiguration;
             _logger = loggerFactory.CreateLogger<LegacyAzureMonitorClient>();
-            _logger.LogInformation("Using legacy scraper");
+            _logger.LogWarning("Will use deprecated Azure Management Libraries for Metric Scraping");
             _authenticatedAzureSubscription = CreateLegacyAzureClient(azureCloud, tenantId, subscriptionId, azureAuthenticationInfo, loggerFactory, metricSinkWriter, azureScrapingSystemMetricsPublisher, azureMonitorLoggingConfiguration);
         }
 

diff --git a/src/Promitor.Tests.Unit/Configuration/RuntimeConfigurationUnitTest.cs b/src/Promitor.Tests.Unit/Configuration/RuntimeConfigurationUnitTest.cs
@@ -96,6 +96,43 @@ public async Task RuntimeConfiguration_HasStartingFromInHoursConfiguredForAzureM
             Assert.Equal(expectedStartingFromInHours, runtimeConfiguration.AzureMonitor.Integration.History.StartingFromInHours);
         }
 
+        [Fact]
+        public async Task RuntimeConfiguration_HasNoNewSdkFlagForAzureMonitorIntegration_DefaultsToTrue()
+        {
+            // Arrange
+            var configuration = await RuntimeConfigurationGenerator.WithServerConfiguration()
+                .WithAzureMonitorIntegration(useAzureMonitorSdk: null)
+                .GenerateAsync();
+
+            // Act
+            var runtimeConfiguration = configuration.Get<ScraperRuntimeConfiguration>();
+
+            // Assert
+            Assert.NotNull(runtimeConfiguration);
+            Assert.NotNull(runtimeConfiguration.AzureMonitor);
+            Assert.NotNull(runtimeConfiguration.AzureMonitor.Integration);
+            Assert.True(runtimeConfiguration.AzureMonitor.Integration.UseAzureMonitorSdk);
+        }
+
+        [Fact]
+        public async Task RuntimeConfiguration_OverrideNewSdkFlagForAzureMonitorIntegration_BecomesFalse()
+        {
+            // Arrange
+            var configuration = await RuntimeConfigurationGenerator.WithServerConfiguration()
+                .WithAzureMonitorIntegration(useAzureMonitorSdk: false)
+                .GenerateAsync();
+
+            // Act
+            var runtimeConfiguration = configuration.Get<ScraperRuntimeConfiguration>();
+
+            // Assert
+            Assert.NotNull(runtimeConfiguration);
+            Assert.NotNull(runtimeConfiguration.AzureMonitor);
+            Assert.NotNull(runtimeConfiguration.AzureMonitor.Integration);
+            Assert.False(runtimeConfiguration.AzureMonitor.Integration.UseAzureMonitorSdk);
+        }
+
+
         [Theory]
         [InlineData(true)]
         [InlineData(false)]

diff --git a/src/Promitor.Tests.Unit/Generators/Config/RuntimeConfigurationGenerator.cs b/src/Promitor.Tests.Unit/Generators/Config/RuntimeConfigurationGenerator.cs
@@ -235,7 +235,7 @@ public RuntimeConfigurationGenerator WithAzureMonitorLogging(bool isEnabled = tr
             return this;
         }
 
-        public RuntimeConfigurationGenerator WithAzureMonitorIntegration(int? startingFromInHours = 100)
+        public RuntimeConfigurationGenerator WithAzureMonitorIntegration(int? startingFromInHours = 100, bool? useAzureMonitorSdk = true)
         {
             _runtimeConfiguration.AzureMonitor ??= new AzureMonitorConfiguration();
             _runtimeConfiguration.AzureMonitor.Integration ??= new AzureMonitorIntegrationConfiguration();
@@ -247,6 +247,11 @@ public RuntimeConfigurationGenerator WithAzureMonitorIntegration(int? startingFr
                 _runtimeConfiguration.AzureMonitor.Integration.History.StartingFromInHours = startingFromInHours.Value;
             }
 
+            if (useAzureMonitorSdk != null)
+            {
+                _runtimeConfiguration.AzureMonitor.Integration.UseAzureMonitorSdk = useAzureMonitorSdk.Value;
+            }
+
             return this;
         }
 
@@ -340,6 +345,7 @@ public async Task<IConfiguration> GenerateAsync()
                 if (_runtimeConfiguration?.AzureMonitor.Integration?.History != null)
                 {
                     configurationBuilder.AppendLine("  integration:");
+                    configurationBuilder.AppendLine($"    useAzureMonitorSdk: {_runtimeConfiguration?.AzureMonitor.Integration.UseAzureMonitorSdk}");
                     configurationBuilder.AppendLine("    history:");
                     configurationBuilder.AppendLine($"      startingFromInHours: {_runtimeConfiguration?.AzureMonitor.Integration.History.StartingFromInHours}");
                 }

diff --git a/src/Promitor.Tests.Unit/Serialization/v1/Core/V1DeserializerTests.cs b/src/Promitor.Tests.Unit/Serialization/v1/Core/V1DeserializerTests.cs
@@ -187,30 +187,5 @@ public void Deserialize_Metric_SetsMetricsNull()
             // Assert
             Assert.Null(declaration.Metrics);
         }
-
-          [Fact]
-        public void Deserialize_NoSDKFlag_DefaultsToTrue()
-        {
-            // Arrange
-            var node = YamlUtils.CreateYamlNode(
-@"azureMetadata:
-    tenantId: '123'");
-            var builder = _deserializer.Deserialize(node, _errorReporter.Object);
-
-            Assert.True(builder.UseAzureMonitorSdk);
-        }
-
-         [Fact]
-        public void Deserialize_SdkSpecified_SetsCorrectFlag()
-        {
-            // Arrange
-            var yamlNode = YamlUtils.CreateYamlNode("useAzureMonitorSdk: false");
-
-            // Act
-            var builder = _deserializer.Deserialize(yamlNode, _errorReporter.Object);
-
-            // Assert
-            Assert.False(builder.UseAzureMonitorSdk);
-        }
     }
 }