Provide better configuration deserialization & validation at startup

[tomkerkhove]

Provide better configuration deserialization & validation at startup.

An example of this came up in #591 where everything seemed ok but in really there was a deserialization issue which was not blocked given we don't have mandatory field validation.

Configuration was:

metrics:
  - name: "generic_postgresql_scraper"
    description: "test"
    resourceType: "Generic"
    resourceUri: "Microsoft.DBforPostgreSQL/servers/promitor"
    azureMetricConfiguration:
      metricName: "cpu_percent"
      aggregation:
        Type: "Average"

Which should resolve in something along the lines of:

[
  {
    "resourceType": "Generic",
    "resourceUri": "Microsoft.DBforPostgreSQL/servers/promitor",
    "azureMetricConfiguration": {
      "metricName": "cpu_percent",
      "aggregation": {
        "type": "None",
        "interval": "00:05:00"
      }
    },
    "description": "test",
    "name": "generic_postgresql_scraper",
    "resourceGroupName": "promitor",
    "scraping": {
      "schedule": "0 * * ? * *"
    }
  }
]

Note that azureMetricConfiguration.aggregation.type is none since the configuration has Type rather than type. We should not ignore this.

[github-actions]

This issue is stale because it has been open with no activity. Remove stale label, add keep label or comment to prevent this issue from being closed in 21 days.

[adamconnelly]

@tomkerkhove I'm interested in working on this. I've posted a PR with an outline of what I'm thinking: #793.

I've also mocked up some tests locally to verify quickly that we should be able to do this without too much difficulty.

If you have some time, any chance you can take a look at the PR and let me know if you think I'm going in the right direction, and if you've got any suggestions?

[tomkerkhove]

I certainly will, thank you very much!

[adamconnelly]

@tomkerkhove sorry about the delayed response - I spent some time over the holidays and got this mostly working. I still need to go through and tidy things up, and maybe tweak the startup code of promitor to make things a bit nicer to read (at the moment I've just hooked things into the main validation code, and it ends up printing a big stack trace out, which is a bit ugly).

Anyhow, since this is another one of these changes that affects a massive chunk of the code-base, I want to try to split it into a few PRs to make it a bit more manageable to review. At the same time, I don't really want to put up PRs for merging back into the main Promitor repo with partially working code, so I was going to suggest getting you to review a few PRs against my own fork, and then once it's complete I can create another PR to merge all my changes into your repo. Does that sound ok?

If so, the first (and hopefully biggest) PR is here: adamconnelly#1. It should give you a decent idea of the approach I've taken.

[tomkerkhove]

Awesome, thanks Adam!

I'll go through that PR asap, thanks.

[adamconnelly]

@tomkerkhove here's the next PR when you have some time: adamconnelly#2.

[adamconnelly]

@tomkerkhove here's a PR converting all the resource deserializers over: adamconnelly#3.

It would be great if you could check that the fields are correct in terms of being required or optional. Especially for the following resources that I'm not really sure about:

• ApiManagementDeserializer
• AppPlanDeserializer
• FunctionAppDeserializer
• WebAppDeserializer

I tried to guess based on the docs on promitor.io, but I might have got some of them wrong.

[adamconnelly]

@tomkerkhove here's a PR hooking the changes up properly so we can see it working end to end: adamconnelly#4.

There's still other things to do like offering suggestions, adding validation of cron expressions, along with a few other changes you asked me to make in my initial PR, but it shows that the concept works.