tomkerkhove / promitor

Bringing Azure Monitor metrics where you need them.
https://promitor.io
MIT License
255 stars 93 forks source link

Validation is not enforcing `azureMetricConfiguration` on a metric properly during startup #2137

Open tranmonglong0611 opened 2 years ago

tranmonglong0611 commented 2 years ago

Report

I run Promitor.Agents.Scraper main method local. And I see that when I lack azureMetricConfiguration field when config metrics I will got this kind of error

Expected Behavior

I want to get Report Error that azureMetricConfiguration is required field when config metrics Something like this

image

Actual Behavior

Steps to Reproduce the Problem

  1. Start main method in Promitor.Agents.Scraper.Program.cs

...

Component

Scraper

Version

v1.0.0

Configuration

Configuration:

version: v1
azureMetadata:
  tenantId: 5766f879-f51d-4e43-88e6-a8c609103041
  subscriptionId: 4d47ec28-1799-4c7b-9e34-81300079de6b
  resourceGroupName: ag1-data-platform-core
  cloud: "UsGov"
metricDefaults:
  aggregation:
    interval: 00:00:01
  limit: 10
  labels:
    geo: china
    environment: dev
  scraping:
    # Every minute
    schedule: "0 * * ? * *"
metrics:
  - name: azure_storage_account_capacity_discovery
    description: "The average capacity used in the storage account"
    resourceType: StorageAccount```

### Logs

```shell
System.ArgumentNullException: Value cannot be null. (Parameter 'key')
   at System.Collections.Generic.Dictionary`2.FindValue(TKey key)
   at System.Collections.Generic.Dictionary`2.TryGetValue(TKey key, TValue& value)
   at Microsoft.Extensions.DependencyInjection.SchedulingExtensions.GroupMetricsByScrapingInterval(MetricsDeclaration allMetrics) in /Users/ltran/DataPlatform/promitor/src/Promitor.Agents.Scraper/Scheduling/SchedulingExtensions.cs:line 64
   at Microsoft.Extensions.DependencyInjection.SchedulingExtensions.ScheduleMetricScraping(IServiceCollection services) in /Users/ltran/DataPlatform/promitor/src/Promitor.Agents.Scraper/Scheduling/SchedulingExtensions.cs:line 47
   at Promitor.Agents.Scraper.Startup.ConfigureServices(IServiceCollection services) in /Users/ltran/DataPlatform/promitor/src/Promitor.Agents.Scraper/Startup.cs:line 57
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Span`1& arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at Microsoft.AspNetCore.Hosting.ConfigureServicesBuilder.InvokeCore(Object instance, IServiceCollection services)
   at Microsoft.AspNetCore.Hosting.ConfigureServicesBuilder.<>c__DisplayClass9_0.<Invoke>g__Startup|0(IServiceCollection serviceCollection)
   at Microsoft.AspNetCore.Hosting.ConfigureServicesBuilder.Invoke(Object instance, IServiceCollection services)
   at Microsoft.AspNetCore.Hosting.ConfigureServicesBuilder.<>c__DisplayClass8_0.<Build>b__0(IServiceCollection services)
   at Microsoft.AspNetCore.Hosting.GenericWebHostBuilder.UseStartup(Type startupType, HostBuilderContext context, IServiceCollection services, Object instance)
   at Microsoft.AspNetCore.Hosting.GenericWebHostBuilder.<>c__DisplayClass14_0`1.<UseStartup>b__0(HostBuilderContext context, IServiceCollection services)
   at Microsoft.Extensions.Hosting.HostBuilder.CreateServiceProvider()
   at Microsoft.Extensions.Hosting.HostBuilder.Build()
   at Promitor.Agents.Scraper.Program.Main(String[] args) in /Users/ltran/DataPlatform/promitor/src/Promitor.Agents.Scraper/Program.cs:line 37

Platform

No response

Contact Details

No response

tomkerkhove commented 2 years ago

That's because of the scheduling process which should be blocked.

You are saying that there is no validation today that makes sure the Azure Monitor configuration is there?

tranmonglong0611 commented 2 years ago

I mean the scheduling process is blocked but the stacktrace log is not really meaningful for the user. They should get the log like "azureMetricConfiguration is required field but was not found ..."

tomkerkhove commented 2 years ago

That is exactly the problem though, it should never get that far and the validation should already block that metric.