Provide possible suggestions for unknown fields #1106

adamconnelly · 2020-06-20T17:09:29Z

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

If we have the following config:

version: v1
azureMetadata:
  tenantId: <tenant-id>
  subscriptionId: <subscription-id>
  resourceGroupName: promitor
metricDeflauts:
  aggregation:
    interval: 00:05:00
  scraping:
    # Every minute
    schedule: "0 * * ? * *"
metrics:
  - nam: azure_lb_data_path_availability
    desription: "The CPU of our containers in a container group."
    resourcetype: Generic
    Scraping:
      schedule: '0 * * ? * *'
    azureMetricConfiguration:
      metricName: VipAvailability
      aggregation:
        type: Average
    resource:
    - resourceUri: "Microsoft.Network/loadBalancers/promitor-lb"

It produces the following error messages:

[17:47:06 ERR] The following problems were found with the metric configuration:
Warning 6:1: Unknown field 'metricDeflauts'. Did you mean 'metricDefaults'?
Warning 13:5: Unknown field 'nam'. Did you mean 'name'?
Error 13:5: 'name' is a required field but was not found.
Error 13:5: 'description' is a required field but was not found.
Error 13:5: 'resourceType' is a required field but was not found.
Warning 14:5: Unknown field 'desription'. Did you mean 'description'?
Warning 15:5: Unknown field 'resourcetype'. Did you mean 'resourceType'?
Warning 16:5: Unknown field 'Scraping'. Did you mean 'scraping'?
Warning 22:5: Unknown field 'resource'. Did you mean 'resources'?
[17:47:06 WRN] Validation step 3/6 failed. Error(s): Errors were found while deserializing the metric configuration.

promitor-bot · 2020-06-20T17:12:05Z

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr1106-linux

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-agent-scraper-pr1106-linux \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env-file C:/Promitor/az-mon-auth.creds \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml  \
                         --volume C:/Promitor/runtime-config.yaml:/config/runtime.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr1106-linux

You can find a CI version of our Helm chart on hub.helm.sh

promitor-bot · 2020-06-20T17:20:48Z

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr1106-linux

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-agent-scraper-pr1106-linux \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env-file C:/Promitor/az-mon-auth.creds \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml  \
                         --volume C:/Promitor/runtime-config.yaml:/config/runtime.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr1106-linux

You can find a CI version of our Helm chart on hub.helm.sh

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 tomkerkhove#1105

promitor-bot · 2020-06-20T21:56:48Z

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr1106-linux

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-agent-scraper-pr1106-linux \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env-file C:/Promitor/az-mon-auth.creds \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml  \
                         --volume C:/Promitor/runtime-config.yaml:/config/runtime.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr1106-linux

You can find a CI version of our Helm chart on hub.helm.sh

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

tomkerkhove · 2020-06-22T05:40:19Z

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,


Looks a lot better, thanks 👌

Yeah - I think we can simplify it even more by adding support for deserializing YamlSequenceNode objects, but I didn't want to change too much at once (or have to figure out how to do that 😄).

tomkerkhove · 2020-06-22T05:50:35Z

src/Promitor.Tests.Unit/Serialization/v1/Core/V1DeserializerTests.cs

@@ -34,16 +34,18 @@ public V1DeserializerTests()
        }

        [Fact]
-        public void Deserialize_NoVersionSpecified_ThrowsException()


What happens if these are not specified and you start up the app? Does it still block it from proceeding?

Yeah - in all cases it blocks the app from starting properly. The version is actually a bit of an odd one - it won't even get as far as the deserializer if the version is missing because we don't know what version of the config to deserialize!

Anyhow, here's examples of the output in each case.

No version: https://gist.github.com/adamconnelly/99ec2264cf8e796a54f40d0bd6b3df22

Metrics missing: https://gist.github.com/adamconnelly/1c402535d4eca8f07fc3d9e7662af6b6

Defaults missing: https://gist.github.com/adamconnelly/c8016c675e81190cd3d81e8b9479a292

Azure metadata missing: https://gist.github.com/adamconnelly/d14d422bc8ee3cbf9b38cda3482308be

Awesome, thanks!

promitor-bot · 2020-06-22T16:54:46Z

Docker image for this PR was built and is available on Docker Hub.

You can pull it locally via the CLI:

docker pull tomkerkhove/promitor-agent-scraper-ci:pr1106-linux

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-agent-scraper-pr1106-linux \
                         --env PROMITOR_AUTH_APPID='<azure-ad-app-id>' \
                         --env-file C:/Promitor/az-mon-auth.creds \
                         --volume C:/Promitor/metrics-declaration.yaml:/config/metrics-declaration.yaml  \
                         --volume C:/Promitor/runtime-config.yaml:/config/runtime.yaml \
                         tomkerkhove/promitor-agent-scraper-ci:pr1106-linux

You can find a CI version of our Helm chart on hub.helm.sh

adamconnelly requested a review from tomkerkhove as a code owner June 20, 2020 17:09

adamconnelly force-pushed the field-suggestions branch from 5c94fbc to 788bf77 Compare June 20, 2020 17:18

adamconnelly force-pushed the field-suggestions branch from 788bf77 to b877cf8 Compare June 20, 2020 21:54

tomkerkhove reviewed Jun 22, 2020

View reviewed changes

trafico-bot bot added the 🔍 Ready for Review Pull Request is not reviewed yet label Jun 22, 2020

fixup! Provide possible suggestions for unknown fields

7e799e6

adamconnelly mentioned this pull request Jun 22, 2020

Ensure that schedule is a valid Cron expression #1104

Merged

tomkerkhove approved these changes Jun 23, 2020

View reviewed changes

trafico-bot bot added ✅ Approved Pull Request has been approved and can be merged and removed 🔍 Ready for Review Pull Request is not reviewed yet labels Jun 23, 2020

tomkerkhove merged commit 5d6291e into tomkerkhove:master Jun 23, 2020

trafico-bot bot added ✨ Merged Pull Request has been merged successfully and removed ✅ Approved Pull Request has been approved and can be merged labels Jun 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Provide possible suggestions for unknown fields #1106

Provide possible suggestions for unknown fields #1106

adamconnelly commented Jun 20, 2020

promitor-bot commented Jun 20, 2020

promitor-bot commented Jun 20, 2020

promitor-bot commented Jun 20, 2020

tomkerkhove Jun 22, 2020

adamconnelly Jun 22, 2020

tomkerkhove Jun 22, 2020

adamconnelly Jun 22, 2020

tomkerkhove Jun 23, 2020

promitor-bot commented Jun 22, 2020

Provide possible suggestions for unknown fields #1106

Provide possible suggestions for unknown fields #1106

Conversation

adamconnelly commented Jun 20, 2020

promitor-bot commented Jun 20, 2020

promitor-bot commented Jun 20, 2020

promitor-bot commented Jun 20, 2020

tomkerkhove Jun 22, 2020

Choose a reason for hiding this comment

adamconnelly Jun 22, 2020

Choose a reason for hiding this comment

tomkerkhove Jun 22, 2020

Choose a reason for hiding this comment

adamconnelly Jun 22, 2020

Choose a reason for hiding this comment

tomkerkhove Jun 23, 2020

Choose a reason for hiding this comment

promitor-bot commented Jun 22, 2020