From 35ebbe59a17b43bc7ad32145b1e95d798624a0be Mon Sep 17 00:00:00 2001 From: Stephen Chen Date: Sat, 29 Jul 2023 12:08:22 -0700 Subject: [PATCH] fix: Improve AzureMonitorScraper Error Handling for Time Series Missing 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 --- changelog/content/experimental/unreleased.md | 1 + .../AzureMonitorScraper.cs | 4 ++- .../Exceptions/MissingDimensionException.cs | 32 +++++++++++++++++ src/Promitor.Core/Metrics/MeasuredMetric.cs | 11 ++++-- .../AzureMonitorClient.cs | 4 ++- .../Metrics/MeasuredMetricTests.cs | 35 +++++++++++++++++++ 6 files changed, 82 insertions(+), 5 deletions(-) create mode 100644 src/Promitor.Core/Metrics/Exceptions/MissingDimensionException.cs create mode 100644 src/Promitor.Tests.Unit/Metrics/MeasuredMetricTests.cs diff --git a/changelog/content/experimental/unreleased.md b/changelog/content/experimental/unreleased.md index 41ae7d210..4379f261b 100644 --- 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 index edae805f0..d7cf8b509 100644 --- a/src/Promitor.Core.Scraping/AzureMonitorScraper.cs +++ b/src/Promitor.Core.Scraping/AzureMonitorScraper.cs @@ -58,7 +58,9 @@ protected override async Task 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 new file mode 100644 index 000000000..98256de27 --- /dev/null +++ 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 + { + /// + /// Constructor + /// + /// Name of the dimension + /// Time series element missing the dimension + public MissingDimensionException(string dimensionName, TimeSeriesElement timeSeries) : base($"No value found for dimension '{dimensionName}'") + { + Guard.NotNullOrWhitespace(dimensionName, nameof(dimensionName)); + + DimensionName = dimensionName; + TimeSeries = timeSeries; + } + + /// + /// Name of the dimension + /// + public string DimensionName { get; } + + /// + /// Time series element producing the exception + /// + public TimeSeriesElement TimeSeries { get; } + } +} \ No newline at end of file diff --git a/src/Promitor.Core/Metrics/MeasuredMetric.cs b/src/Promitor.Core/Metrics/MeasuredMetric.cs index 21634f194..635773b2a 100644 --- 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 dim { Guard.NotAny(dimensionNames, nameof(dimensionNames)); Guard.NotNull(timeseries, nameof(timeseries)); - Guard.For(() => timeseries.Metadatavalues.Any() == false); - + var dimensions = new List(); 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 index 5dcd3672d..1e899f306 100644 --- a/src/Promitor.Integrations.AzureMonitor/AzureMonitorClient.cs +++ b/src/Promitor.Integrations.AzureMonitor/AzureMonitorClient.cs @@ -109,7 +109,9 @@ public async Task> 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 new file mode 100644 index 000000000..afd420bfe --- /dev/null +++ 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 { "dimTest"}; + var dimensionValue = "dimTest1"; + var timeSeries = new TimeSeriesElement(new List { 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 { "dimTest"}; + var timeSeries = new TimeSeriesElement(new List()); + MissingDimensionException ex = Assert.Throws(() => MeasuredMetric.CreateForDimensions(1, dimensionName, timeSeries)); + Assert.Equal(ex.DimensionName, dimensionName[0]); + } + } +} \ No newline at end of file