From 92217cd12cbaf926c40eb7f9fcf2ab6228279142 Mon Sep 17 00:00:00 2001 From: Wiktor Kopec Date: Fri, 10 Feb 2023 17:19:17 -0800 Subject: [PATCH] Allow per provider interval specification (#3591) * Allow per provider interval specification * Update src/Microsoft.Diagnostics.Monitoring.WebApi/Metrics/MetricsSettingsFactory.cs Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Update src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/MetricsSettingsTests.cs Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Pr feedback * pr feedback * pr feedback --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../configuration/metrics-configuration.md | 44 ++++++ documentation/schema.json | 25 ++++ .../GlobalCounterOptions.cs | 40 +++++- .../OptionsDisplayStrings.Designer.cs | 18 +++ .../OptionsDisplayStrings.resx | 6 + .../Controllers/DiagController.cs | 2 +- .../Metrics/MetricsSettingsFactory.cs | 14 ++ .../Strings.Designer.cs | 2 +- .../Strings.resx | 2 +- .../Utilities/TraceUtilities.cs | 11 +- .../Validation/CounterValidator.cs | 4 +- .../Options/OptionsExtensions.cs | 10 ++ .../CollectionRuleOptionsTests.cs | 79 +++++++++++ .../MetricsSettingsTests.cs | 126 ++++++++++++++++++ .../Actions/CollectTraceAction.cs | 4 +- .../Actions/CollectTraceOptions.Validate.cs | 16 ++- .../AspNetRequestDurationTriggerFactory.cs | 3 +- .../Triggers/EventCounterTriggerFactory.cs | 2 +- .../ServiceCollectionExtensions.cs | 4 +- 19 files changed, 395 insertions(+), 17 deletions(-) create mode 100644 src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/MetricsSettingsTests.cs diff --git a/documentation/configuration/metrics-configuration.md b/documentation/configuration/metrics-configuration.md index 3b3cb42cd5a..396a71bb830 100644 --- a/documentation/configuration/metrics-configuration.md +++ b/documentation/configuration/metrics-configuration.md @@ -8,6 +8,8 @@ Due to limitations in event counters, `dotnet monitor` supports only **one** refresh interval when collecting metrics. This interval is used for Prometheus metrics, livemetrics, triggers, traces, and trigger actions that collect traces. The default interval is 5 seconds, but can be changed in configuration. +[7.1+] For EventCounter providers, is possible to specify a different interval for each provider. See [Per provider intervals](#per-provider-intervals-71). +
JSON @@ -37,6 +39,48 @@ Prometheus metrics, livemetrics, triggers, traces, and trigger actions that coll ```
+## Per provider intervals (7.1+) + +It is possible to override the global interval on a per provider basis. Note this forces all scenarios (triggers, live metrics, prometheus metrics, traces) that use a particular provider to use that interval. Metrics that are `System.Diagnostics.Metrics` based always use global interval. + +
+ JSON + + ```json + { + "GlobalCounter": { + "IntervalSeconds": 5, + "Providers": { + "System.Runtime": { + "IntervalSeconds": 10 + } + } + } + } + ``` +
+ +
+ Kubernetes ConfigMap + + ```yaml + GlobalCounter__IntervalSeconds: "5" + GlobalCounter__Providers__System.Runtime__IntervalSeconds: "10" + ``` +
+ +
+ Kubernetes Environment Variables + + ```yaml + - name: DotnetMonitor_GlobalCounter__IntervalSeconds + value: "5" + - name: DotnetMonitor_GlobalCounter__Providers__System.Runtime__IntervalSeconds + value: "10" + + ``` +
+ ## Metrics Urls In addition to the ordinary diagnostics urls that `dotnet monitor` binds to, it also binds to metric urls that only expose the `/metrics` endpoint. Unlike the other endpoints, the metrics urls do not require authentication. Unless you enable collection of custom providers that may contain sensitive business logic, it is generally considered safe to expose metrics endpoints. diff --git a/documentation/schema.json b/documentation/schema.json index 0f3a2756fba..8c0b1c4acd3 100644 --- a/documentation/schema.json +++ b/documentation/schema.json @@ -899,6 +899,31 @@ "default": 1000, "maximum": 2147483647.0, "minimum": 1.0 + }, + "Providers": { + "type": [ + "null", + "object" + ], + "description": "Dictionary of provider names and their global configuration.", + "additionalProperties": { + "$ref": "#/definitions/GlobalProviderOptions" + } + } + } + }, + "GlobalProviderOptions": { + "type": "object", + "additionalProperties": false, + "properties": { + "IntervalSeconds": { + "type": [ + "null", + "number" + ], + "format": "float", + "maximum": 86400.0, + "minimum": 1.0 } } }, diff --git a/src/Microsoft.Diagnostics.Monitoring.Options/GlobalCounterOptions.cs b/src/Microsoft.Diagnostics.Monitoring.Options/GlobalCounterOptions.cs index 54e601769c1..ccb83017e09 100644 --- a/src/Microsoft.Diagnostics.Monitoring.Options/GlobalCounterOptions.cs +++ b/src/Microsoft.Diagnostics.Monitoring.Options/GlobalCounterOptions.cs @@ -2,12 +2,15 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; using System.ComponentModel; using System.ComponentModel.DataAnnotations; +using System.Globalization; +using System.Linq; namespace Microsoft.Diagnostics.Monitoring.WebApi { - public class GlobalCounterOptions + public partial class GlobalCounterOptions { public const float IntervalMinSeconds = 1; public const float IntervalMaxSeconds = 60 * 60 * 24; // One day @@ -32,6 +35,38 @@ public class GlobalCounterOptions [DefaultValue(GlobalCounterOptionsDefaults.MaxTimeSeries)] [Range(1, int.MaxValue)] public int? MaxTimeSeries { get; set; } + + [Display( + ResourceType = typeof(OptionsDisplayStrings), + Description = nameof(OptionsDisplayStrings.DisplayAttributeDescription_GlobalCounterOptions_Providers))] + public System.Collections.Generic.IDictionary Providers { get; set; } = new Dictionary(StringComparer.OrdinalIgnoreCase); + } + + public class GlobalProviderOptions + { + [Range(GlobalCounterOptions.IntervalMinSeconds, GlobalCounterOptions.IntervalMaxSeconds)] + public float? IntervalSeconds { get; set; } + } + + partial class GlobalCounterOptions : IValidatableObject + { + public IEnumerable Validate(ValidationContext validationContext) + { + var results = new List(); + var providerResults = new List(); + foreach ((string provider, GlobalProviderOptions options) in Providers) + { + providerResults.Clear(); + if (!Validator.TryValidateObject(options, new ValidationContext(options), providerResults, true)) + { + // We prefix the validation error with the provider. + results.AddRange(providerResults.Select(r => new ValidationResult( + string.Format(CultureInfo.CurrentCulture, OptionsDisplayStrings.ErrorMessage_NestedProviderValidationError, provider, r.ErrorMessage)))); + } + } + + return results; + } } internal static class GlobalCounterOptionsExtensions @@ -44,5 +79,8 @@ public static int GetMaxHistograms(this GlobalCounterOptions options) => public static int GetMaxTimeSeries(this GlobalCounterOptions options) => options.MaxTimeSeries.GetValueOrDefault(GlobalCounterOptionsDefaults.MaxTimeSeries); + + public static float GetProviderSpecificInterval(this GlobalCounterOptions options, string providerName) => + options.Providers.TryGetValue(providerName, out GlobalProviderOptions providerOptions) ? providerOptions.IntervalSeconds ?? options.GetIntervalSeconds() : options.GetIntervalSeconds(); } } diff --git a/src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.Designer.cs b/src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.Designer.cs index e0416dddc5f..544e74f4bcf 100644 --- a/src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.Designer.cs +++ b/src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.Designer.cs @@ -1040,6 +1040,15 @@ public static string DisplayAttributeDescription_GlobalCounterOptions_IntervalSe } } + /// + /// Looks up a localized string similar to Dictionary of provider names and their global configuration.. + /// + public static string DisplayAttributeDescription_GlobalCounterOptions_Providers { + get { + return ResourceManager.GetString("DisplayAttributeDescription_GlobalCounterOptions_Providers", resourceCulture); + } + } + /// /// Looks up a localized string similar to Allows features that require diagnostic components to be loaded into target processes to be enabled. These features may have minimal performance impact on target processes.. /// @@ -1589,6 +1598,15 @@ public static string ErrorMessage_FilterValueMissing { } } + /// + /// Looks up a localized string similar to Provider '{0}' validation error: '{1}'. + /// + public static string ErrorMessage_NestedProviderValidationError { + get { + return ResourceManager.GetString("ErrorMessage_NestedProviderValidationError", resourceCulture); + } + } + /// /// Looks up a localized string similar to An egress provider must be specified if there is no default egress provider.. /// diff --git a/src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.resx b/src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.resx index d162e18422f..d34628caba4 100644 --- a/src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.resx +++ b/src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.resx @@ -811,4 +811,10 @@ The type of metrics this provider consumes The description provided for the MetricType parameter on MetricProvider. + + Dictionary of provider names and their global configuration. + + + Provider '{0}' validation error: '{1}' + \ No newline at end of file diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs index 5b17611c6a6..1276751232d 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.cs @@ -337,7 +337,7 @@ public Task CaptureTrace( { TimeSpan duration = Utilities.ConvertSecondsToTimeSpan(durationSeconds); - var aggregateConfiguration = TraceUtilities.GetTraceConfiguration(profile, _counterOptions.CurrentValue.GetIntervalSeconds()); + var aggregateConfiguration = TraceUtilities.GetTraceConfiguration(profile, _counterOptions.CurrentValue); return StartTrace(processInfo, aggregateConfiguration, duration, egressProvider, tags); }, processKey, Utilities.ArtifactType_Trace); diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Metrics/MetricsSettingsFactory.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Metrics/MetricsSettingsFactory.cs index 4385a11487d..0bc17e7e9a1 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Metrics/MetricsSettingsFactory.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Metrics/MetricsSettingsFactory.cs @@ -4,6 +4,7 @@ using Microsoft.Diagnostics.Monitoring.EventPipe; using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Threading; @@ -20,6 +21,7 @@ public static MetricsPipelineSettings CreateSettings(GlobalCounterOptions counte return CreateSettings(includeDefaults, durationSeconds, counterOptions.GetIntervalSeconds(), + counterOptions.Providers, counterOptions.GetMaxHistograms(), counterOptions.GetMaxTimeSeries(), () => new List(0)); @@ -29,6 +31,7 @@ public static MetricsPipelineSettings CreateSettings(GlobalCounterOptions counte { return CreateSettings(options.IncludeDefaultProviders.GetValueOrDefault(MetricsOptionsDefaults.IncludeDefaultProviders), Timeout.Infinite, counterOptions.GetIntervalSeconds(), + counterOptions.Providers, counterOptions.GetMaxHistograms(), counterOptions.GetMaxTimeSeries(), () => ConvertCounterGroups(options.Providers)); @@ -40,6 +43,7 @@ public static MetricsPipelineSettings CreateSettings(GlobalCounterOptions counte return CreateSettings(configuration.IncludeDefaultProviders, durationSeconds, counterOptions.GetIntervalSeconds(), + counterOptions.Providers, counterOptions.GetMaxHistograms(), counterOptions.GetMaxTimeSeries(), () => ConvertCounterGroups(configuration.Providers)); @@ -48,6 +52,7 @@ public static MetricsPipelineSettings CreateSettings(GlobalCounterOptions counte private static MetricsPipelineSettings CreateSettings(bool includeDefaults, int durationSeconds, float counterInterval, + IDictionary intervalMap, int maxHistograms, int maxTimeSeries, Func> createCounterGroups) @@ -61,6 +66,15 @@ private static MetricsPipelineSettings CreateSettings(bool includeDefaults, eventPipeCounterGroups.Add(new EventPipeCounterGroup { ProviderName = MonitoringSourceConfiguration.GrpcAspNetCoreServer, Type = CounterGroupType.EventCounter }); } + foreach (EventPipeCounterGroup counterGroup in eventPipeCounterGroups) + { + if (intervalMap.TryGetValue(counterGroup.ProviderName, out GlobalProviderOptions providerInterval)) + { + Debug.Assert(counterGroup.IntervalSeconds == null, "Unexpected value for provider interval"); + counterGroup.IntervalSeconds = providerInterval.IntervalSeconds; + } + } + return new MetricsPipelineSettings { CounterGroups = eventPipeCounterGroups.ToArray(), diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.Designer.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.Designer.cs index 1aeb9e1bf36..6a73c585fe8 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.Designer.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.Designer.cs @@ -106,7 +106,7 @@ internal static string ErrorMessage_InvalidMetricCount { } /// - /// Looks up a localized string similar to Custom trace metric provider '{0}' must use the global counter interval '{1}'. + /// Looks up a localized string similar to Custom trace metric provider '{0}' must use the expected counter interval '{1}'.. /// internal static string ErrorMessage_InvalidMetricInterval { get { diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.resx b/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.resx index aef63084a5d..c855827f271 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.resx +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Strings.resx @@ -136,7 +136,7 @@ Gets a string similar to "Invalid metric count.". - Custom trace metric provider '{0}' must use the global counter interval '{1}' + Custom trace metric provider '{0}' must use the expected counter interval '{1}'. Metrics was not enabled. diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities/TraceUtilities.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities/TraceUtilities.cs index 33ecebe9f2f..ee4a949fe13 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities/TraceUtilities.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Utilities/TraceUtilities.cs @@ -13,7 +13,7 @@ namespace Microsoft.Diagnostics.Monitoring.WebApi { internal static class TraceUtilities { - public static MonitoringSourceConfiguration GetTraceConfiguration(Models.TraceProfile profile, float metricsIntervalSeconds) + public static MonitoringSourceConfiguration GetTraceConfiguration(Models.TraceProfile profile, GlobalCounterOptions options) { var configurations = new List(); if (profile.HasFlag(Models.TraceProfile.Cpu)) @@ -34,7 +34,14 @@ public static MonitoringSourceConfiguration GetTraceConfiguration(Models.TracePr } if (profile.HasFlag(Models.TraceProfile.Metrics)) { - configurations.Add(new MetricSourceConfiguration(metricsIntervalSeconds, Enumerable.Empty())); + IEnumerable defaultProviders = MonitoringSourceConfiguration.DefaultMetricProviders.Select(provider => new MetricEventPipeProvider + { + Provider = provider, + IntervalSeconds = options.GetProviderSpecificInterval(provider), + Type = MetricType.EventCounter + }); + + configurations.Add(new MetricSourceConfiguration(options.GetIntervalSeconds(), defaultProviders)); } return new AggregateSourceConfiguration(configurations.ToArray()); diff --git a/src/Microsoft.Diagnostics.Monitoring.WebApi/Validation/CounterValidator.cs b/src/Microsoft.Diagnostics.Monitoring.WebApi/Validation/CounterValidator.cs index a1f6ad99d65..bae063dd3c3 100644 --- a/src/Microsoft.Diagnostics.Monitoring.WebApi/Validation/CounterValidator.cs +++ b/src/Microsoft.Diagnostics.Monitoring.WebApi/Validation/CounterValidator.cs @@ -17,12 +17,12 @@ public static bool ValidateProvider(GlobalCounterOptions counterOptions, if (provider.Arguments?.TryGetValue("EventCounterIntervalSec", out string intervalValue) == true) { if (float.TryParse(intervalValue, out float intervalSeconds) && - intervalSeconds != counterOptions.GetIntervalSeconds()) + intervalSeconds != counterOptions.GetProviderSpecificInterval(provider.Name)) { errorMessage = string.Format(CultureInfo.CurrentCulture, Strings.ErrorMessage_InvalidMetricInterval, provider.Name, - counterOptions.GetIntervalSeconds()); + counterOptions.GetProviderSpecificInterval(provider.Name)); return false; } } diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Options/OptionsExtensions.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Options/OptionsExtensions.cs index b40c9a80433..d953295bcd1 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Options/OptionsExtensions.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.FunctionalTests/Options/OptionsExtensions.cs @@ -13,6 +13,7 @@ using System.Security.Cryptography; using System.Text.Json; using System.Text.Json.Serialization; +using Xunit; namespace Microsoft.Diagnostics.Monitoring.TestCommon.Options { @@ -46,6 +47,15 @@ public static RootOptions AddGlobalCounter(this RootOptions options, int interva return options; } + public static RootOptions AddProviderInterval(this RootOptions options, string name, int intervalSeconds) + { + Assert.NotNull(options.GlobalCounter); + + options.GlobalCounter.Providers.Add(name, new GlobalProviderOptions { IntervalSeconds = (float)intervalSeconds }); + + return options; + } + public static CollectionRuleOptions CreateCollectionRule(this RootOptions rootOptions, string name) { CollectionRuleOptions options = new(); diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectionRuleOptionsTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectionRuleOptionsTests.cs index 5b2d4ba92f7..55f88a787ef 100644 --- a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectionRuleOptionsTests.cs +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/CollectionRuleOptionsTests.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.Diagnostics.Monitoring.EventPipe; using Microsoft.Diagnostics.Monitoring.TestCommon; using Microsoft.Diagnostics.Monitoring.TestCommon.Options; using Microsoft.Diagnostics.Monitoring.WebApi.Models; @@ -19,6 +20,7 @@ using System.Diagnostics.Tracing; using System.Globalization; using System.Linq; +using System.Reflection; using System.Threading.Tasks; using Xunit; using Xunit.Abstractions; @@ -1055,6 +1057,65 @@ public Task CollectionRuleOptions_CollectTraceAction_PropertyValidation() }); } + [Fact] + public Task CollectionRuleOptions_CollectTraceAction_ValidateProviderIntervals() + { + const string ExpectedEgressProvider = "TmpEgressProvider"; + const int ExpectedInterval = 7; + + return ValidateFailure( + rootOptions => + { + rootOptions.AddGlobalCounter(5); + rootOptions.AddProviderInterval(MonitoringSourceConfiguration.SystemRuntimeEventSourceName, ExpectedInterval); + + rootOptions.AddFileSystemEgress(ExpectedEgressProvider, "/tmp"); + + rootOptions.CreateCollectionRule(DefaultRuleName) + .SetStartupTrigger() + .AddCollectTraceAction(new EventPipeProvider[] { new EventPipeProvider + { + Name = MonitoringSourceConfiguration.SystemRuntimeEventSourceName, + Arguments = new Dictionary{ { "EventCounterIntervalSec", "5" } }, + }}, + ExpectedEgressProvider, null); + }, + ex => + { + string failure = Assert.Single(ex.Failures); + VerifyProviderIntervalMessage(failure, MonitoringSourceConfiguration.SystemRuntimeEventSourceName, ExpectedInterval); + }); + } + + [Fact] + public Task CollectionRuleOptions_CollectTraceAction_InvalidProviderInterval() + { + const string ExpectedEgressProvider = "TmpEgressProvider"; + + return ValidateFailure( + rootOptions => + { + rootOptions.AddGlobalCounter(5); + rootOptions.AddProviderInterval(MonitoringSourceConfiguration.SystemRuntimeEventSourceName, -2); + + rootOptions.AddFileSystemEgress(ExpectedEgressProvider, "/tmp"); + + rootOptions.CreateCollectionRule(DefaultRuleName) + .SetStartupTrigger() + .AddCollectTraceAction(new EventPipeProvider[] { new EventPipeProvider + { + Name = MonitoringSourceConfiguration.SystemRuntimeEventSourceName, + Arguments = new Dictionary{ { "EventCounterIntervalSec", "5" } }, + }}, + ExpectedEgressProvider, null); + }, + ex => + { + string failure = Assert.Single(ex.Failures); + VerifyNestedGlobalInterval(failure, MonitoringSourceConfiguration.SystemRuntimeEventSourceName); + }); + } + [Fact] public Task CollectionRuleOptions_CollectTraceAction_NoProfileOrProviders() { @@ -1869,5 +1930,23 @@ private static void VerifyMissingStoppingEventProviderMessage(string[] failures, Assert.Equal(message, failures[index]); } + + private static void VerifyProviderIntervalMessage(string failure, string provider, int expectedInterval) + { + string message = string.Format(CultureInfo.CurrentCulture, WebApi.Strings.ErrorMessage_InvalidMetricInterval, provider, expectedInterval); + + Assert.Equal(message, failure); + } + + private static void VerifyNestedGlobalInterval(string failure, string provider) + { + string rangeValidationMessage = typeof(WebApi.GlobalProviderOptions) + .GetProperty(nameof(WebApi.GlobalProviderOptions.IntervalSeconds)) + .GetCustomAttribute() + .FormatErrorMessage(nameof(WebApi.GlobalProviderOptions.IntervalSeconds)); + + string message = string.Format(CultureInfo.CurrentCulture, WebApi.OptionsDisplayStrings.ErrorMessage_NestedProviderValidationError, provider, rangeValidationMessage); + Assert.Equal(message, failure); + } } } diff --git a/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/MetricsSettingsTests.cs b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/MetricsSettingsTests.cs new file mode 100644 index 00000000000..f6a1a9a5a7b --- /dev/null +++ b/src/Tests/Microsoft.Diagnostics.Monitoring.Tool.UnitTests/MetricsSettingsTests.cs @@ -0,0 +1,126 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Diagnostics.Monitoring.EventPipe; +using Microsoft.Diagnostics.Monitoring.TestCommon; +using Microsoft.Diagnostics.Monitoring.TestCommon.Options; +using Microsoft.Diagnostics.Monitoring.WebApi; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Options; +using System.Collections.Generic; +using System.Linq; +using Xunit; +using Xunit.Abstractions; + +namespace Microsoft.Diagnostics.Monitoring.Tool.UnitTests +{ + public class MetricsSettingsTests + { + private readonly ITestOutputHelper _outputHelper; + + public MetricsSettingsTests(ITestOutputHelper outputHelper) + { + _outputHelper = outputHelper; + } + + [Fact] + public void ValidateDefaultMetricSettings() + { + const int ExpectedGlobalInterval = 5; + int customInterval = ExpectedGlobalInterval + 1; + int[] expectedIntervals = MonitoringSourceConfiguration.DefaultMetricProviders.Select((_, index) => index + ExpectedGlobalInterval + 1).ToArray(); + + using IHost host = TestHostHelper.CreateHost(_outputHelper, (rootOptions) => + { + rootOptions.AddGlobalCounter(ExpectedGlobalInterval); + foreach (string provider in MonitoringSourceConfiguration.DefaultMetricProviders) + { + rootOptions.AddProviderInterval(provider, customInterval++); + } + }, + servicesCallback: null, + loggingCallback: null, + overrideSource: null); + + var options = host.Services.GetRequiredService>(); + + var settings = MetricsSettingsFactory.CreateSettings(options.CurrentValue, includeDefaults: true, durationSeconds: 30); + + Assert.Equal(ExpectedGlobalInterval, settings.CounterIntervalSeconds); + + customInterval = ExpectedGlobalInterval + 1; + foreach (string provider in MonitoringSourceConfiguration.DefaultMetricProviders) + { + Assert.Equal(customInterval++, GetInterval(settings, provider)); + } + } + + [Fact] + public void ValidateApiMetricsSettings() + { + const int ExpectedGlobalInterval = 5; + const int CustomInterval = 6; + const string CustomProvider1 = nameof(CustomProvider1); + const string CustomProvider2 = nameof(CustomProvider2); + + using IHost host = TestHostHelper.CreateHost(_outputHelper, (rootOptions) => + { + rootOptions.AddGlobalCounter(ExpectedGlobalInterval) + .AddProviderInterval(CustomProvider1, CustomInterval); + }, + servicesCallback: null, + loggingCallback: null, + overrideSource: null); + + var options = host.Services.GetRequiredService>(); + + var settings = MetricsSettingsFactory.CreateSettings(options.CurrentValue, 30, new WebApi.Models.EventMetricsConfiguration + { + IncludeDefaultProviders = false, + Providers = new[] { new WebApi.Models.EventMetricsProvider { ProviderName = CustomProvider1 }, new WebApi.Models.EventMetricsProvider { ProviderName = CustomProvider2 } } + }); + + Assert.Equal(ExpectedGlobalInterval, settings.CounterIntervalSeconds); + Assert.Equal(CustomInterval, GetInterval(settings, CustomProvider1)); + Assert.Null(GetInterval(settings, CustomProvider2)); + } + + [Fact] + public void ValidateMetricStoreSettings() + { + const int ExpectedGlobalInterval = 5; + const int CustomInterval = 6; + const string CustomProvider1 = nameof(CustomProvider1); + const string CustomProvider2 = nameof(CustomProvider2); + + using IHost host = TestHostHelper.CreateHost(_outputHelper, (rootOptions) => + { + rootOptions.AddGlobalCounter(ExpectedGlobalInterval) + .AddProviderInterval(CustomProvider1, CustomInterval); + }, + servicesCallback: null, + loggingCallback: null, + overrideSource: null); + + var options = host.Services.GetRequiredService>(); + + var settings = MetricsSettingsFactory.CreateSettings(options.CurrentValue, new MetricsOptions + { + IncludeDefaultProviders = false, + Providers = new List { new MetricProvider { ProviderName = CustomProvider1 }, new MetricProvider { ProviderName = CustomProvider2 } } + }); + + Assert.Equal(ExpectedGlobalInterval, settings.CounterIntervalSeconds); + Assert.Equal(CustomInterval, GetInterval(settings, CustomProvider1)); + Assert.Null(GetInterval(settings, CustomProvider2)); + } + + private static float? GetInterval(MetricsPipelineSettings settings, string provider) + { + EventPipeCounterGroup counterGroup = settings.CounterGroups.FirstOrDefault(g => g.ProviderName == provider); + Assert.NotNull(counterGroup); + return counterGroup.IntervalSeconds; + } + } +} diff --git a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs index cfe51e83b74..e0f7f83c097 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Actions/CollectTraceAction.cs @@ -70,9 +70,7 @@ protected override async Task ExecuteCoreAsync( if (Options.Profile.HasValue) { TraceProfile profile = Options.Profile.Value; - float metricsIntervalSeconds = _counterOptions.CurrentValue.GetIntervalSeconds(); - - configuration = TraceUtilities.GetTraceConfiguration(profile, metricsIntervalSeconds); + configuration = TraceUtilities.GetTraceConfiguration(profile, _counterOptions.CurrentValue); } else { diff --git a/src/Tools/dotnet-monitor/CollectionRules/Options/Actions/CollectTraceOptions.Validate.cs b/src/Tools/dotnet-monitor/CollectionRules/Options/Actions/CollectTraceOptions.Validate.cs index 182973fd250..6ab69e32461 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Options/Actions/CollectTraceOptions.Validate.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Options/Actions/CollectTraceOptions.Validate.cs @@ -55,6 +55,18 @@ IEnumerable IValidatableObject.Validate(ValidationContext vali } else if (hasProviders) { + GlobalCounterOptions counterOptions = null; + + try + { + // Nested validations have to be handled by catching the exception and converting it to a ValidationResult. + counterOptions = validationContext.GetRequiredService>().CurrentValue; + } + catch (OptionsValidationException e) + { + results.AddRange(e.Failures.Select((string failure) => new ValidationResult(e.Message))); + } + // Validate that each provider is valid. int index = 0; foreach (EventPipeProvider provider in Providers) @@ -64,9 +76,7 @@ IEnumerable IValidatableObject.Validate(ValidationContext vali Validator.TryValidateObject(provider, providerContext, results, validateAllProperties: true); - IOptionsMonitor counterOptions = validationContext.GetRequiredService>(); - if (!CounterValidator.ValidateProvider(counterOptions.CurrentValue, - provider, out string errorMessage)) + if (counterOptions != null && !CounterValidator.ValidateProvider(counterOptions, provider, out string errorMessage)) { results.Add(new ValidationResult(errorMessage, new[] { nameof(EventPipeProvider.Arguments) })); } diff --git a/src/Tools/dotnet-monitor/CollectionRules/Triggers/AspNetRequestDurationTriggerFactory.cs b/src/Tools/dotnet-monitor/CollectionRules/Triggers/AspNetRequestDurationTriggerFactory.cs index 6091b366e95..616862cf5ed 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Triggers/AspNetRequestDurationTriggerFactory.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Triggers/AspNetRequestDurationTriggerFactory.cs @@ -44,7 +44,8 @@ public ICollectionRuleTrigger Create(IEndpointInfo endpointInfo, Action callback SlidingWindowDuration = options.SlidingWindowDuration ?? TimeSpan.Parse(AspNetRequestDurationOptionsDefaults.SlidingWindowDuration, CultureInfo.InvariantCulture), }; - var aspnetTriggerSourceConfiguration = new AspNetTriggerSourceConfiguration(_counterOptions.CurrentValue.GetIntervalSeconds()); + //HACK we get the provider specific interval for the configuration + var aspnetTriggerSourceConfiguration = new AspNetTriggerSourceConfiguration(_counterOptions.CurrentValue.GetProviderSpecificInterval(MonitoringSourceConfiguration.MicrosoftAspNetCoreHostingEventSourceName)); return EventPipeTriggerFactory.Create(endpointInfo, aspnetTriggerSourceConfiguration, _traceEventTriggerFactory, settings, callback); } diff --git a/src/Tools/dotnet-monitor/CollectionRules/Triggers/EventCounterTriggerFactory.cs b/src/Tools/dotnet-monitor/CollectionRules/Triggers/EventCounterTriggerFactory.cs index 690bed92cce..309bfa90933 100644 --- a/src/Tools/dotnet-monitor/CollectionRules/Triggers/EventCounterTriggerFactory.cs +++ b/src/Tools/dotnet-monitor/CollectionRules/Triggers/EventCounterTriggerFactory.cs @@ -40,7 +40,7 @@ public ICollectionRuleTrigger Create(IEndpointInfo endpointInfo, Action callback EventCounterTriggerSettings settings = new() { ProviderName = options.ProviderName, - CounterIntervalSeconds = _counterOptions.CurrentValue.GetIntervalSeconds(), + CounterIntervalSeconds = _counterOptions.CurrentValue.GetProviderSpecificInterval(options.ProviderName), CounterName = options.CounterName, GreaterThan = options.GreaterThan, LessThan = options.LessThan, diff --git a/src/Tools/dotnet-monitor/ServiceCollectionExtensions.cs b/src/Tools/dotnet-monitor/ServiceCollectionExtensions.cs index f3ce951dd42..d062a71d8ea 100644 --- a/src/Tools/dotnet-monitor/ServiceCollectionExtensions.cs +++ b/src/Tools/dotnet-monitor/ServiceCollectionExtensions.cs @@ -39,7 +39,9 @@ public static IServiceCollection ConfigureCors(this IServiceCollection services, public static IServiceCollection ConfigureGlobalCounter(this IServiceCollection services, IConfiguration configuration) { - return ConfigureOptions(services, configuration, ConfigurationKeys.GlobalCounter); + return ConfigureOptions(services, configuration, ConfigurationKeys.GlobalCounter) + .AddSingleton, DataAnnotationValidateOptions>(); + } public static IServiceCollection ConfigureCollectionRuleDefaults(this IServiceCollection services, IConfiguration configuration)