Report validation error if resources not specified

[adamconnelly]

I've updated the MetricDefinitionDeserializer to report a validation error if neither resources nor resourceCollections is populated.

Fixes #806

A metric definition like the following:

- name: azure_lb_data_path_availability
  description: "The CPU of our containers in a container group."
  resourceType: Generic
  azureMetricConfiguration:
    metricName: VipAvailability
    aggregation:
      type: Average

Will now produce the following validation error:

[15:56:21 ERR] The following problems were found with the metric configuration:
Error 13:5: Either 'resources' or 'resourceCollections' must be specified.

[promitor-bot]

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:pr1107-linux

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-agent-scraper-pr1107-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:pr1107-linux

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

[tomkerkhove]

LGTM, thanks!

Would you mind adding an entry on https://github.com/tomkerkhove/promitor/blob/master/changelog/content/experimental/unreleased.md please?

[adamconnelly]

@tomkerkhove I'm happy to do that - I'm just not sure what the best thing to do is because you've already added the following entry to the changelog:

New validation rule to ensure at least one resource or resource collection is configured to scrape

I'm thinking it might be confusing if we put it in another entry that's pretty similar. I realise your entry is for the internal model validation, but what I've added is for the configuration validation, but maybe the distinction won't be important for users?

I'll go ahead and add changelog entries for my other two PRs anyhow - just need some guidance on this one.

[tomkerkhove]

That's a valid point, no need to take any action then!

Once the code quality check is passing we can merge this one, thanks!

[promitor-bot]

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:pr1107-linux

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-agent-scraper-pr1107-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:pr1107-linux

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

[adamconnelly]

Ok - should be sorted now @tomkerkhove

[promitor-bot]

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:pr1107-linux

Want to verify the new version? Run it locally:

docker run -d -p 8999:80 --name promitor-agent-scraper-pr1107-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:pr1107-linux

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

[tomkerkhove]

Thanks man!