Provide possible suggestions for unknown fields (#1106)

* Provide possible suggestions for unknown fields

I've updated the deserialization to provide a list of suggestions for unknown field names. This uses the Fastenshtein library to calculate the edit distance between the field name entered by a user and the possible fields.

I've chosen to return fields if they have an edit distance less than 3. This is fairly arbitrary and just based on some simple testing. I wanted to provide suggestions, but not provide ones that were completely different to the name entered.

Also:

- Updated the V1Deserializer to use the `Map()` syntax for mapping rather than doing everything manually. The main reason for this was so that the fields are added as suggestions, although it also removes some unnecessary code.
- Altered MetricsDeclarationProvider to not set the defaults if validating the metrics fails. The reason I've done this is that if the metric defaults couldn't be deserialized, we end up with a NullReferenceException.

Fixes #1105

* fixup! Provide possible suggestions for unknown fields

changelog/content/experimental/unreleased.md

@@ -7,4 +7,5 @@ version:
 
 - {{% tag added %}} New validation rule to ensure at least one resource or resource collection is configured to scrape
 - {{% tag removed %}} Support for Swagger 2.0 ([deprecation notice](https://changelog.promitor.io/#swagger-2-0))
-- {{% tag removed %}} Support for Swagger UI 2.0 ([deprecation notice](https://changelog.promitor.io/#swagger-ui-2-0))
+- {{% tag removed %}} Support for Swagger UI 2.0 ([deprecation notice](https://changelog.promitor.io/#swagger-ui-2-0))
+- {{% tag added %}} Provide suggestions when unknown fields are found in the metrics config. [#1105](https://github.com/tomkerkhove/promitor/issues/1105).

src/Promitor.Core.Scraping/Configuration/Providers/MetricsDeclarationProvider.cs

@@ -40,7 +40,7 @@ public virtual MetricsDeclaration Get(bool applyDefaults = false, IErrorReporter
 
             var config = _configurationSerializer.Deserialize(rawMetricsDeclaration, errorReporter);
 
-            if (applyDefaults)
+            if (!errorReporter.HasErrors && applyDefaults)
             {
                 foreach (var metric in config.Metrics)
                 {

src/Promitor.Core.Scraping/Configuration/Serialization/DeserializationContext.cs

@@ -1,5 +1,6 @@
 using System.Collections.Generic;
 using System.Linq;
+using Fastenshtein;
 
 namespace Promitor.Core.Scraping.Configuration.Serialization
 {
@@ -8,6 +9,13 @@ namespace Promitor.Core.Scraping.Configuration.Serialization
     /// </summary>
     public class DeserializationContext<TObject>
     {
+        /// <summary>
+        /// The max edit distance for us to consider a field as a suggestion
+        /// between a field name entered by a user, and the field names that
+        /// have been configured for the object.
+        /// </summary>
+        private const int MaxSuggestionDistance = 3;
+
         private readonly ISet<string> _ignoredFields;
         private readonly Dictionary<string, FieldDeserializationContext<TObject>> _fields;
 
@@ -57,5 +65,21 @@ public bool TryGetField(string fieldName, out FieldDeserializationContext<TObjec
         {
             return _fields.TryGetValue(fieldName, out field);
         }
+
+        /// <summary>
+        /// Returns a list of fields that are close enough to <paramref ref="fieldName" />
+        /// for us to suggest them as alternatives.
+        /// </summary>
+        /// <param name="fieldName">
+        /// The name of a field that doesn't exist that we want to find suggestions for.
+        /// </param>
+        public IReadOnlyCollection<string> GetSuggestions(string fieldName)
+        {
+            return _fields
+                .Select(field => field.Key)
+                .Union(_ignoredFields)
+                .Where(suggestion => Levenshtein.Distance(suggestion, fieldName) < MaxSuggestionDistance)
+                .ToList();
+        }
     }
 }

src/Promitor.Core.Scraping/Configuration/Serialization/Deserializer.cs

@@ -45,7 +45,8 @@ public virtual TObject Deserialize(YamlMappingNode node, IErrorReporter errorRep
                 }
                 else
                 {
-                    errorReporter.ReportWarning(child.Key, $"Unknown field '{child.Key}'.");
+                    var warningMessage = GetUnknownFieldWarningMessage(deserializationContext, child.Key.ToString());
+                    errorReporter.ReportWarning(child.Key, warningMessage);
                 }
             }
 
@@ -158,5 +159,28 @@ private static object GetFieldValue(
 
             return null;
         }
+
+        /// <summary>
+        /// Gets the warning message to use when an invalid field name is found
+        /// in the configuration.
+        /// </summary>
+        /// <param name="deserializationContext">The deserialization context.</param>
+        /// <param name="fieldName">The unknown field.</param>
+        private static string GetUnknownFieldWarningMessage(DeserializationContext<TObject> deserializationContext, string fieldName)
+        {
+            var message = $"Unknown field '{fieldName}'.";
+            var suggestions = deserializationContext
+                .GetSuggestions(fieldName)
+                .Select(suggestion => $"'{suggestion}'")
+                .ToList();
+
+            if (suggestions.Any())
+            {
+                var formattedSuggestions = string.Join(", ", suggestions);
+                message += $" Did you mean {formattedSuggestions}?";
+            }
+
+            return message;
+        }
     }
 }

src/Promitor.Core.Scraping/Configuration/Serialization/v1/Core/V1Deserializer.cs

@@ -8,79 +8,43 @@ namespace Promitor.Core.Scraping.Configuration.Serialization.v1.Core
 {
     public class V1Deserializer : Deserializer<MetricsDeclarationV1>
     {
-        private readonly IDeserializer<AzureMetadataV1> _azureMetadataDeserializer;
-        private readonly IDeserializer<MetricDefaultsV1> _defaultsDeserializer;
         private readonly IDeserializer<MetricDefinitionV1> _metricsDeserializer;
 
         public V1Deserializer(IDeserializer<AzureMetadataV1> azureMetadataDeserializer,
             IDeserializer<MetricDefaultsV1> defaultsDeserializer,
             IDeserializer<MetricDefinitionV1> metricsDeserializer,
             ILogger<V1Deserializer> logger) : base(logger)
         {
-            _azureMetadataDeserializer = azureMetadataDeserializer;
-            _defaultsDeserializer = defaultsDeserializer;
             _metricsDeserializer = metricsDeserializer;
-        }
-
-        public override MetricsDeclarationV1 Deserialize(YamlMappingNode rootNode, IErrorReporter errorReporter)
-        {
-            ValidateVersion(rootNode);
-
-            var azureMetadata = DeserializeAzureMetadata(rootNode, errorReporter);
-            var metricDefaults = DeserializeMetricDefaults(rootNode, errorReporter);
-            var metrics = DeserializeMetrics(rootNode, errorReporter);
-
-            return new MetricsDeclarationV1
-            {
-                Version = SpecVersion.v1.ToString(),
-                AzureMetadata = azureMetadata,
-                MetricDefaults = metricDefaults,
-                Metrics = metrics
-            };
-        }
-
-        private static void ValidateVersion(YamlMappingNode rootNode)
-        {
-            var versionFound = rootNode.Children.TryGetValue("version", out var versionNode);
-            if (!versionFound)
-            {
-                throw new System.Exception("No 'version' element was found in the metrics config");
-            }
 
-            if (versionNode.ToString() != SpecVersion.v1.ToString())
-            {
-                throw new System.Exception($"A 'version' element with a value of '{SpecVersion.v1}' was expected but the value '{versionNode}' was found");
-            }
+            Map(definition => definition.Version)
+                .IsRequired()
+                .MapUsing(GetVersion);
+            Map(definition => definition.AzureMetadata)
+                .IsRequired()
+                .MapUsingDeserializer(azureMetadataDeserializer);
+            Map(definition => definition.MetricDefaults)
+                .IsRequired()
+                .MapUsingDeserializer(defaultsDeserializer);
+            Map(definition => definition.Metrics)
+                .IsRequired()
+                .MapUsing(DeserializeMetrics);
         }
 
-        private AzureMetadataV1 DeserializeAzureMetadata(YamlMappingNode rootNode, IErrorReporter errorReporter)
+        private static object GetVersion(string value, KeyValuePair<YamlNode, YamlNode> node, IErrorReporter errorReporter)
         {
-            if (rootNode.Children.TryGetValue("azureMetadata", out var azureMetadataNode))
+            if (value != SpecVersion.v1.ToString())
             {
-                return _azureMetadataDeserializer.Deserialize((YamlMappingNode)azureMetadataNode, errorReporter);
+                errorReporter.ReportError(node.Value, $"A 'version' element with a value of '{SpecVersion.v1}' was expected but the value '{value}' was found");
             }
 
-            return null;
+            return SpecVersion.v1.ToString();
         }
 
-        private MetricDefaultsV1 DeserializeMetricDefaults(YamlMappingNode rootNode, IErrorReporter errorReporter)
+        private IReadOnlyCollection<MetricDefinitionV1> DeserializeMetrics(
+            string value, KeyValuePair<YamlNode, YamlNode> nodePair, IErrorReporter errorReporter)
         {
-            if (rootNode.Children.TryGetValue("metricDefaults", out var defaultsNode))
-            {
-                return _defaultsDeserializer.Deserialize((YamlMappingNode)defaultsNode, errorReporter);
-            }
-
-            return null;
-        }
-
-        private IReadOnlyCollection<MetricDefinitionV1> DeserializeMetrics(YamlMappingNode rootNode, IErrorReporter errorReporter)
-        {
-            if (rootNode.Children.TryGetValue("metrics", out var metricsNode))
-            {
-                return _metricsDeserializer.Deserialize((YamlSequenceNode)metricsNode, errorReporter);
-            }
-
-            return null;
+            return _metricsDeserializer.Deserialize((YamlSequenceNode)nodePair.Value, errorReporter);
         }
     }
 }

src/Promitor.Core.Scraping/Promitor.Core.Scraping.csproj

@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <TargetFramework>netcoreapp3.1</TargetFramework>
@@ -16,6 +16,7 @@
 
   <ItemGroup>
     <PackageReference Include="AutoMapper" Version="9.0.0" />
+    <PackageReference Include="Fastenshtein" Version="1.0.0.5" />
     <PackageReference Include="Guard.Net" Version="1.2.0" />
     <PackageReference Include="Microsoft.ApplicationInsights" Version="2.14.0" />
     <PackageReference Include="Microsoft.Azure.Management.Fluent" Version="1.34.0" />

src/Promitor.Tests.Unit/Promitor.Tests.Unit.csproj

@@ -27,6 +27,7 @@
   <ItemGroup>
     <PackageReference Include="AutoMapper" Version="9.0.0" />
     <PackageReference Include="Bogus" Version="29.0.2" />
+    <PackageReference Include="Fastenshtein" Version="1.0.0.5" />
     <PackageReference Include="JetBrains.DotMemoryUnit" Version="3.1.20200127.214830" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.6.1" />
     <PackageReference Include="Moq" Version="4.14.3" />

src/Promitor.Tests.Unit/Serialization/DeserializationContextTests.cs

@@ -0,0 +1,102 @@
+using System.Collections.Generic;
+using System.Reflection;
+using Promitor.Core.Scraping.Configuration.Serialization;
+using Xunit;
+
+namespace Promitor.Tests.Unit.Serialization
+{
+    public class DeserializationContextTests
+    {
+        [Fact]
+        public void GetSuggestions_NoFieldsConfigured_Empty()
+        {
+            // Arrange
+            var context = new DeserializationContext<Person>(
+                new HashSet<string>(), new List<FieldDeserializationInfo>());
+
+            // Act
+            var suggestions = context.GetSuggestions("job");
+
+            // Assert
+            Assert.Empty(suggestions);
+        }
+
+        [Fact]
+        public void GetSuggestions_CloseEnoughSuggestion_ReturnsSuggestion()
+        {
+            // Arrange
+            var fields = new List<FieldDeserializationInfo>
+            {
+                CreateField(typeof(Person).GetProperty(nameof(Person.Name))),
+                CreateField(typeof(Person).GetProperty(nameof(Person.Age))),
+                CreateField(typeof(Person).GetProperty(nameof(Person.Country))),
+                CreateField(typeof(Person).GetProperty(nameof(Person.County)))
+            };
+            var context = new DeserializationContext<Person>(new HashSet<string>(), fields);
+
+            // Act
+            var suggestions = context.GetSuggestions("nmae");
+
+            // Assert
+            Assert.Collection(suggestions, suggestion => Assert.Equal("name", suggestion));
+        }
+
+        [Fact]
+        public void GetSuggestions_CloseEnoughSuggestion_ReturnsAllSuggestions()
+        {
+            // Arrange
+            var fields = new List<FieldDeserializationInfo>
+            {
+                CreateField(typeof(Person).GetProperty(nameof(Person.Name))),
+                CreateField(typeof(Person).GetProperty(nameof(Person.Age))),
+                CreateField(typeof(Person).GetProperty(nameof(Person.Country))),
+                CreateField(typeof(Person).GetProperty(nameof(Person.County)))
+            };
+            var context = new DeserializationContext<Person>(new HashSet<string>(), fields);
+
+            // Act
+            var suggestions = context.GetSuggestions("count");
+
+            // Assert
+            Assert.Collection(suggestions,
+                suggestion => Assert.Equal("country", suggestion),
+                suggestion => Assert.Equal("county", suggestion));
+        }
+
+        [Fact]
+        public void GetSuggestions_IgnoredFields_IncludesIgnoredFieldsInSuggestions()
+        {
+            // Arrange
+            var fields = new List<FieldDeserializationInfo>
+            {
+                CreateField(typeof(Person).GetProperty(nameof(Person.Name))),
+                CreateField(typeof(Person).GetProperty(nameof(Person.Age))),
+                CreateField(typeof(Person).GetProperty(nameof(Person.Country))),
+                CreateField(typeof(Person).GetProperty(nameof(Person.County)))
+            };
+            var ignoredFields = new HashSet<string> { "named" };
+            var context = new DeserializationContext<Person>(ignoredFields, fields);
+
+            // Act
+            var suggestions = context.GetSuggestions("nam");
+
+            // Assert
+            Assert.Collection(suggestions,
+                suggestion => Assert.Equal("name", suggestion),
+                suggestion => Assert.Equal("named", suggestion));
+        }
+
+        private FieldDeserializationInfo CreateField(PropertyInfo propertyInfo)
+        {
+            return new FieldDeserializationInfo(propertyInfo, false, null, null, null);
+        }
+
+        public class Person
+        {
+            public string Name { get; set; }
+            public int Age { get; set; }
+            public string Country { get; set; }
+            public string County { get; set; }
+        }
+    }
+}

src/Promitor.Tests.Unit/Serialization/DeserializerTests/ValidationTests.cs

@@ -1,4 +1,4 @@
-using System;
+using System;
 using System.Linq;
 using Microsoft.Extensions.Logging.Abstractions;
 using Moq;
@@ -153,11 +153,42 @@ public void Deserialize_CalledMultipleTimes_DoesNotReusePreviousState()
                 r => r.ReportError(It.IsAny<YamlNode>(), It.Is<string>(s => s.Contains("name"))));
         }
 
+        [Fact]
+        public void Deserialize_UnknownField_ReturnsSuggestion()
+        {
+            // Arrange
+            var node = YamlUtils.CreateYamlNode(@"nmae: Promitor");
+
+            // Act
+            _deserializer.Deserialize(node, _errorReporter.Object);
+
+            // Assert
+            var nameTagNode = node.Children.Single(c => c.Key.ToString() == "nmae").Key;
+
+            _errorReporter.Verify(r => r.ReportWarning(nameTagNode, "Unknown field 'nmae'. Did you mean 'name'?"));
+        }
+
+        [Fact]
+        public void Deserialize_UnknownField_ReturnsMultipleSuggestions()
+        {
+            // Arrange
+            var node = YamlUtils.CreateYamlNode(@"dat: Monday");
+
+            // Act
+            _deserializer.Deserialize(node, _errorReporter.Object);
+
+            // Assert
+            var nameTagNode = node.Children.Single(c => c.Key.ToString() == "dat").Key;
+
+            _errorReporter.Verify(r => r.ReportWarning(nameTagNode, "Unknown field 'dat'. Did you mean 'day', 'date'?"));
+        }
+
         private class TestConfigObject
         {
             public string Name { get; set; }
             public int Age { get; set; }
             public DayOfWeek Day { get; set; }
+            public string Date { get; set; }
             public TimeSpan Interval { get; set; }
         }
 
@@ -168,6 +199,7 @@ public TestDeserializer() : base(NullLogger.Instance)
                 Map(t => t.Name).IsRequired();
                 Map(t => t.Age);
                 Map(t => t.Day);
+                Map(t => t.Date);
                 Map(t => t.Interval);
                 IgnoreField("customField");
             }