fix: Improve AzureMonitorScraper Error Handling for Time Series Missi…

…ng Requested Dimension Value (#2345) * improve error handling for when a time series is returned without dimension value * small fixes address comments * add to changelog * use targeted exception time for missing dimension + add basic tests * formatting fix * address a few small comments from Tom * address a few small comments from Tom * code style fix * fix R sharper style warnings * change measuredmetric test to support multiple dimensions * change measuredmetric test to support multiple dimensions * fix: Remove redundant using statement * Code quality * Code quality --------- Co-authored-by: Tom Kerkhove <[email protected]>
tomkerkhove · Jul 29, 2023 · 35ebbe5 · 35ebbe5
1 parent 45c56c8
commit 35ebbe5
Show file tree

Hide file tree

Showing 6 changed files with 82 additions and 5 deletions.
diff --git a/changelog/content/experimental/unreleased.md b/changelog/content/experimental/unreleased.md
@@ -6,6 +6,7 @@ version:
 
 #### Scraper
 
+- {{% tag fixed %}} Improve handling of time series with missing dimensions that are requested ([#2331](https://github.com/tomkerkhove/promitor/issues/2331))
 - {{% tag added %}} Provide support for all label scenarios in StatsD & OpenTelemetry metric sink. This includes
 dimensions, customer & default labels.
 - {{% tag added %}} Provide support for scraping multiple metrics dimensions.

diff --git a/src/Promitor.Core.Scraping/AzureMonitorScraper.cs b/src/Promitor.Core.Scraping/AzureMonitorScraper.cs
@@ -58,7 +58,9 @@ protected override async Task<ScrapeResult> ScrapeResourceAsync(string subscript
             {
                 Logger.LogWarning("No metric information found for metric {MetricName} with dimensions {MetricDimensions}. Details: {Details}", metricsNotFoundException.Name, metricsNotFoundException.Dimensions, metricsNotFoundException.Details);
 
-                var measuredMetric = dimensionNames.Any() ? MeasuredMetric.CreateForDimensions(dimensionNames) : MeasuredMetric.CreateWithoutDimensions(null);
+                var measuredMetric = dimensionNames.Any() 
+                            ? MeasuredMetric.CreateForDimensions(dimensionNames) 
+                            : MeasuredMetric.CreateWithoutDimensions(null);
                 measuredMetrics.Add(measuredMetric);
             }
 

diff --git a/src/Promitor.Core/Metrics/Exceptions/MissingDimensionException.cs b/src/Promitor.Core/Metrics/Exceptions/MissingDimensionException.cs
@@ -0,0 +1,32 @@
+using System;
+using GuardNet;
+using Microsoft.Azure.Management.Monitor.Fluent.Models;
+
+namespace Promitor.Core.Metrics.Exceptions
+{
+    public class MissingDimensionException : Exception
+    {
+        /// <summary>
+        ///     Constructor
+        /// </summary>
+        /// <param name="dimensionName">Name of the dimension</param>
+        /// <param name="timeSeries">Time series element missing the dimension</param>
+        public MissingDimensionException(string dimensionName, TimeSeriesElement timeSeries) : base($"No value found for dimension '{dimensionName}'")
+        {
+            Guard.NotNullOrWhitespace(dimensionName, nameof(dimensionName));
+
+            DimensionName = dimensionName;
+            TimeSeries = timeSeries; 
+        }
+
+        /// <summary>
+        ///     Name of the dimension
+        /// </summary>
+        public string DimensionName { get; }
+
+        /// <summary>
+        ///     Time series element producing the exception  
+        /// </summary>
+        public TimeSeriesElement TimeSeries { get; }
+    }
+}
diff --git a/src/Promitor.Core/Metrics/MeasuredMetric.cs b/src/Promitor.Core/Metrics/MeasuredMetric.cs
@@ -3,6 +3,7 @@
 using System.Linq;
 using GuardNet;
 using Microsoft.Azure.Management.Monitor.Fluent.Models;
+using Promitor.Core.Metrics.Exceptions;
 
 namespace Promitor.Core.Metrics
 {
@@ -57,12 +58,16 @@ public static MeasuredMetric CreateForDimensions(double? value, List<string> dim
         {
             Guard.NotAny(dimensionNames, nameof(dimensionNames));
             Guard.NotNull(timeseries, nameof(timeseries));
-            Guard.For<ArgumentException>(() => timeseries.Metadatavalues.Any() == false);
-
+
             var dimensions = new List<MeasuredMetricDimension>();
             foreach (var dimensionName in dimensionNames)
             {
-                var dimensionValue = timeseries.Metadatavalues.Single(metadataValue => metadataValue.Name?.Value.Equals(dimensionName, StringComparison.InvariantCultureIgnoreCase) == true).Value;
+                var dimensionMetadataValue = timeseries.Metadatavalues.Where(metadataValue => metadataValue.Name?.Value.Equals(dimensionName, StringComparison.InvariantCultureIgnoreCase) == true).ToList();            
+                if(!dimensionMetadataValue.Any())
+                {
+                    throw new MissingDimensionException(dimensionName, timeseries);
+                }
+                var dimensionValue = dimensionMetadataValue.Single().Value;
                 dimensions.Add(new MeasuredMetricDimension(dimensionName, dimensionValue));
             }
 

diff --git a/src/Promitor.Integrations.AzureMonitor/AzureMonitorClient.cs b/src/Promitor.Integrations.AzureMonitor/AzureMonitorClient.cs
@@ -109,7 +109,9 @@ public async Task<List<MeasuredMetric>> QueryMetricAsync(string metricName, List
                 // Get the metric value according to the requested aggregation type
                 var requestedMetricAggregate = InterpretMetricValue(aggregationType, mostRecentMetricValue);
 
-                var measuredMetric = metricDimensions.Any() ? MeasuredMetric.CreateForDimensions(requestedMetricAggregate, metricDimensions, timeseries) : MeasuredMetric.CreateWithoutDimensions(requestedMetricAggregate);
+                var measuredMetric = metricDimensions.Any() 
+                            ? MeasuredMetric.CreateForDimensions(requestedMetricAggregate, metricDimensions, timeseries) 
+                            : MeasuredMetric.CreateWithoutDimensions(requestedMetricAggregate);
                 measuredMetrics.Add(measuredMetric);
             }
 

diff --git a/src/Promitor.Tests.Unit/Metrics/MeasuredMetricTests.cs b/src/Promitor.Tests.Unit/Metrics/MeasuredMetricTests.cs
@@ -0,0 +1,35 @@
+using System.Collections.Generic;
+using System.ComponentModel;
+using Promitor.Core.Metrics;
+using Microsoft.Azure.Management.Monitor.Fluent.Models;
+using Xunit;
+using Promitor.Core.Metrics.Exceptions;
+
+namespace Promitor.Tests.Unit.Metrics
+{
+    [Category("Unit")]
+    public class MeasuredMetricTest : UnitTest
+    {
+        [Fact]
+        public void Create_MeasuredMetric_With_Single_Dimension_HappyPath_Succeeds()
+        {
+            var dimensionNames = new List<string> { "dimTest"};
+            var dimensionValue = "dimTest1";
+            var timeSeries = new TimeSeriesElement(new List<MetadataValue> { new(name: new LocalizableString(dimensionNames[0]), value: dimensionValue)});
+            var measuredMetric = MeasuredMetric.CreateForDimensions(1, dimensionNames, timeSeries);
+            Assert.Single(measuredMetric.Dimensions);
+            Assert.Equal(dimensionNames[0], measuredMetric.Dimensions[0].Name);
+            Assert.Equal(dimensionValue, measuredMetric.Dimensions[0].Value);
+            Assert.Equal(1, measuredMetric.Value);
+        }
+
+        [Fact]
+        public void Create_MeasuredMetric_Missing_Dimension_Throws_Targeted_Exception()
+        {
+            var dimensionName = new List<string> { "dimTest"};
+            var timeSeries = new TimeSeriesElement(new List<MetadataValue>());
+            MissingDimensionException ex = Assert.Throws<MissingDimensionException>(() => MeasuredMetric.CreateForDimensions(1, dimensionName, timeSeries));
+            Assert.Equal(ex.DimensionName, dimensionName[0]);
+        }
+    }
+}