Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Move Azure Monitor SDK Feature Flag to Runtime Configuration #2507

Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions .github/workflows/templates-build-push-image.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
name: Build and Push (Linux)
hkfgo marked this conversation as resolved.
Show resolved Hide resolved
on:
workflow_call:
workflow_dispatch:
inputs:
image_name:
required: true
Expand All @@ -15,6 +16,8 @@ jobs:
linux:
name: Build & Push (Linux)
runs-on: ubuntu-latest
permissions:
packages: write
steps:
- name: Checkout Code
uses: actions/checkout@v4
Expand All @@ -40,7 +43,7 @@ jobs:
uses: docker/login-action@v3
with:
registry: ghcr.io
username: tomkerkhove
username: hkfgo
password: ${{ secrets.GITHUB_TOKEN }}

- name: Build and push preview image
Expand All @@ -49,5 +52,5 @@ jobs:
build-args: VERSION="${{ env.artifact_full_version }}"
context: ./src/
file: ./src/${{ inputs.project_name }}/Dockerfile.linux
tags: ${{ env.image_commit_uri }},${{ env.image_latest_uri }}
push: true
tags: ghcr.io/hkfgo/${{ env.image_commit_uri }},ghcr.io/hkfgo/${{ env.image_latest_uri }}
push: true
6 changes: 3 additions & 3 deletions src/Promitor.Agents.Scraper/AzureMonitorClientFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions src/Promitor.Agents.Scraper/Docs/Open-Api.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
public class AzureMonitorIntegrationConfiguration
{
public AzureMonitorHistoryConfiguration History { get; set; } = new();
public bool UseAzureMonitorSdk { get; set; } = true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,43 @@
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);
}

Check notice on line 134 in src/Promitor.Tests.Unit/Configuration/RuntimeConfigurationUnitTest.cs

View check run for this annotation

codefactor.io / CodeFactor

src/Promitor.Tests.Unit/Configuration/RuntimeConfigurationUnitTest.cs#L134

Code should not contain multiple blank lines in a row. (SA1507)

[Theory]
[InlineData(true)]
[InlineData(false)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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;
}

Expand Down Expand Up @@ -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}");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Loading