Skip to content

Use a singular option for all counter intervals #944

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Oct 3, 2021
Merged

Use a singular option for all counter intervals #944

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4984b95
Use a singular option for all counter intervals
wiktork Oct 1, 2021
37dbeba
Fix unit tests
wiktork Oct 1, 2021
497a161
Pr feedback
wiktork Oct 2, 2021
9ac77bc
Pr feedback
wiktork Oct 3, 2021
a5d6bfb
Use float for interval parsing validation
wiktork Oct 3, 2021
b6e837f
CI Unit test fix attempt
wiktork Oct 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 0 additions & 39 deletions documentation/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,19 +385,6 @@
"default": 30
}
},
{
"name": "metricsIntervalSeconds",
"in": "query",
"description": "The reporting interval (in seconds) for event counters.",
"schema": {
"maximum": 2147483647,
"minimum": 1,
"type": "integer",
"description": "The reporting interval (in seconds) for event counters.",
"format": "int32",
"default": 1
}
},
{
"name": "egressProvider",
"in": "query",
Expand Down Expand Up @@ -852,19 +839,6 @@
"default": 30
}
},
{
"name": "metricsIntervalSeconds",
"in": "query",
"description": "The reporting interval (in seconds) for event counters.",
"schema": {
"maximum": 2147483647,
"minimum": 1,
"type": "integer",
"description": "The reporting interval (in seconds) for event counters.",
"format": "int32",
"default": 5
}
},
{
"name": "egressProvider",
"in": "query",
Expand Down Expand Up @@ -943,19 +917,6 @@
"default": 30
}
},
{
"name": "metricsIntervalSeconds",
"in": "query",
"description": "The reporting interval (in seconds) for event counters.",
"schema": {
"maximum": 2147483647,
"minimum": 1,
"type": "integer",
"description": "The reporting interval (in seconds) for event counters.",
"format": "int32",
"default": 5
}
},
{
"name": "egressProvider",
"in": "query",
Expand Down
59 changes: 28 additions & 31 deletions documentation/schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,17 @@
"$ref": "#/definitions/CollectionRuleOptions"
}
},
"GlobalCounter": {
"default": {},
"oneOf": [
{
"type": "null"
},
{
"$ref": "#/definitions/GlobalCounterOptions"
}
]
},
"CorsConfiguration": {
"default": {},
"oneOf": [
Expand Down Expand Up @@ -723,6 +734,23 @@
}
}
},
"GlobalCounterOptions": {
"type": "object",
"additionalProperties": false,
"properties": {
"IntervalSeconds": {
"type": [
"integer",
"null"
],
"description": "Determines the metrics interval for all dotnet-monitor scenarios. This includes prometheus metrics, live metrics, triggers, and traces.",
"format": "int32",
"default": 5,
"maximum": 86400.0,
"minimum": 1.0
}
}
},
"CorsConfigurationOptions": {
"type": "object",
"additionalProperties": false,
Expand Down Expand Up @@ -933,15 +961,6 @@
],
"description": "Endpoints that expose prometheus metrics. Defaults to http://localhost:52325."
},
"UpdateIntervalSeconds": {
"type": [
"integer",
"null"
],
"description": "How often metrics are collected.",
"format": "int32",
"default": 10
},
"MetricCount": {
"type": [
"integer",
Expand Down Expand Up @@ -1168,17 +1187,6 @@
}
]
},
"MetricsIntervalSeconds": {
"type": [
"integer",
"null"
],
"description": "The amount of time (in seconds) between the collection of each sample of the counter. Only applicable when Profile contains Metrics.",
"format": "int32",
"default": 1,
"maximum": 86400.0,
"minimum": 1.0
},
"Providers": {
"type": [
"array",
Expand Down Expand Up @@ -1520,17 +1528,6 @@
"description": "The sliding time window in which the counter must maintain its value as specified by the threshold levels in GreaterThan and LessThan.",
"format": "time-span",
"default": "00:01:00"
},
"Frequency": {
"type": [
"integer",
"null"
],
"description": "The amount of time (in seconds) between the collection of each sample of the counter.",
"format": "int32",
"default": 5,
"maximum": 86400.0,
"minimum": 1.0
}
}
},
Expand Down
31 changes: 31 additions & 0 deletions src/Microsoft.Diagnostics.Monitoring.Options/GlobalCounterOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.ComponentModel.DataAnnotations;
using System.Text;

namespace Microsoft.Diagnostics.Monitoring.WebApi
{
public class GlobalCounterOptions
{
public const int IntervalMinSeconds = 1;
public const int IntervalMaxSeconds = 60 * 60 * 24; // One day

[Display(
ResourceType = typeof(OptionsDisplayStrings),
Description = nameof(OptionsDisplayStrings.DisplayAttributeDescription_GlobalCounterOptions_IntervalSeconds))]
[Range(IntervalMinSeconds, IntervalMaxSeconds)]
[DefaultValue(GlobalCounterOptionsDefaults.IntervalSeconds)]
public int? IntervalSeconds { get; set; }
}

internal static class GlobalCounterOptionsExtensions
{
public static int GetIntervalSeconds(this GlobalCounterOptions options) =>
Copy link
Contributor

@jander-msft jander-msft Oct 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this better than our current scattering usage of GetValueOrDefault everywhere. Separately, I think we should do this with all of the nullable options properties.

options.IntervalSeconds.GetValueOrDefault(GlobalCounterOptionsDefaults.IntervalSeconds);
}
}
15 changes: 15 additions & 0 deletions src/Microsoft.Diagnostics.Monitoring.Options/GlobalCounterOptionsDefaults.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Text;

namespace Microsoft.Diagnostics.Monitoring.WebApi
{
internal static class GlobalCounterOptionsDefaults
{
public const int IntervalSeconds = 5;
}
}
6 changes: 0 additions & 6 deletions src/Microsoft.Diagnostics.Monitoring.Options/MetricsOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@ public class MetricsOptions
Description = nameof(OptionsDisplayStrings.DisplayAttributeDescription_MetricsOptions_Endpoints))]
public string Endpoints { get; set; }

[Display(
ResourceType = typeof(OptionsDisplayStrings),
Description = nameof(OptionsDisplayStrings.DisplayAttributeDescription_MetricsOptions_UpdateIntervalSeconds))]
[DefaultValue(MetricsOptionsDefaults.UpdateIntervalSeconds)]
public int? UpdateIntervalSeconds { get; set; }

[Display(
ResourceType = typeof(OptionsDisplayStrings),
Description = nameof(OptionsDisplayStrings.DisplayAttributeDescription_MetricsOptions_MetricCount))]
Expand Down
2 changes: 0 additions & 2 deletions src/Microsoft.Diagnostics.Monitoring.Options/MetricsOptionsDefaults.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ internal class MetricsOptionsDefaults
{
public const bool Enabled = true;

public const int UpdateIntervalSeconds = 10;

public const int MetricCount = 3;

public const bool IncludeDefaultProviders = true;
Expand Down
36 changes: 9 additions & 27 deletions src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.Designer.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 3 additions & 12 deletions src/Microsoft.Diagnostics.Monitoring.Options/OptionsDisplayStrings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,10 +289,6 @@
<value>The name of the egress provider to which the trace is egressed.</value>
<comment>The description provided for the Egress parameter on CollectTraceOptions.</comment>
</data>
<data name="DisplayAttributeDescription_CollectTraceOptions_MetricsIntervalSeconds" xml:space="preserve">
<value>The amount of time (in seconds) between the collection of each sample of the counter. Only applicable when Profile contains Metrics.</value>
<comment>The description provided for the MetricsIntervalSeconds parameter on CollectTraceOptions.</comment>
</data>
<data name="DisplayAttributeDescription_CollectTraceOptions_Profile" xml:space="preserve">
<value>Use a predefined set of event providers and settings to capture in the trace. More than one profile may be specified at the same time. Either Profile or Providers must be specified, but not both.</value>
<comment>The description provided for the Profile parameter on CollectTraceOptions.</comment>
Expand Down Expand Up @@ -385,10 +381,6 @@
<value>The name of the counter to monitor.</value>
<comment>The description provided for the CounterName parameter on EventCounterOptions.</comment>
</data>
<data name="DisplayAttributeDescription_EventCounterOptions_Frequency" xml:space="preserve">
<value>The amount of time (in seconds) between the collection of each sample of the counter.</value>
<comment>The description provided for the Frequency parameter on EventCounterOptions.</comment>
</data>
<data name="DisplayAttributeDescription_EventCounterOptions_GreaterThan" xml:space="preserve">
<value>The threshold level the counter must maintain (or higher) for the specified duration. Either GreaterThan or LessThan (or both) must be specified.</value>
<comment>The description provided for the GreaterThan parameter on EventCounterOptions.</comment>
Expand Down Expand Up @@ -481,10 +473,6 @@
<value>Providers for custom metrics.</value>
<comment>The description provided for the Providers parameter on MetricsOptions.</comment>
</data>
<data name="DisplayAttributeDescription_MetricsOptions_UpdateIntervalSeconds" xml:space="preserve">
<value>How often metrics are collected.</value>
<comment>The description provided for the UpdateIntervalSeconds parameter on MetricsOptions.</comment>
</data>
<data name="DisplayAttributeDescription_MonitorApiKeyOptions_PublicKey" xml:space="preserve">
<value>The public key used to sign the JWT (JSON Web Token) used for authentication. This field is a JSON Web Key serialized as JSON encoded with base64Url encoding. The JWK must have a kty field of RSA or EC and should not have the private key information.</value>
<comment>The description provided for the PublicKey parameter on MonitorApiKeyOptions.</comment>
Expand Down Expand Up @@ -556,4 +544,7 @@
<value>Wait for the current action to complete before starting the next action.</value>
<comment>The description provided for the WaitForCompletion parameter on CollectionRuleActionOptions.</comment>
</data>
<data name="DisplayAttributeDescription_GlobalCounterOptions_IntervalSeconds" xml:space="preserve">
<value>Determines the metrics interval for all dotnet-monitor scenarios. This includes prometheus metrics, live metrics, triggers, and traces.</value>
</data>
</root>
12 changes: 3 additions & 9 deletions src/Microsoft.Diagnostics.Monitoring.WebApi/Controllers/DiagController.Metrics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ partial class DiagController
/// <param name="uid">The Runtime instance cookie used to identify the target process.</param>
/// <param name="name">Process name used to identify the target process.</param>
/// <param name="durationSeconds">The duration of the metrics session (in seconds).</param>
/// <param name="metricsIntervalSeconds">The reporting interval (in seconds) for event counters.</param>
/// <param name="egressProvider">The egress provider to which the metrics are saved.</param>
[HttpGet("livemetrics", Name = nameof(CaptureMetrics))]
[ProducesWithProblemDetails(ContentTypes.ApplicationJsonSequence)]
Expand All @@ -45,8 +44,6 @@ public Task<ActionResult> CaptureMetrics(
string name = null,
[FromQuery][Range(-1, int.MaxValue)]
int durationSeconds = 30,
[FromQuery][Range(1, int.MaxValue)]
int metricsIntervalSeconds = 5,
[FromQuery]
string egressProvider = null)
{
Expand All @@ -60,9 +57,9 @@ public Task<ActionResult> CaptureMetrics(
{
var client = new DiagnosticsClient(processInfo.EndpointInfo.Endpoint);
EventPipeCounterPipelineSettings settings = EventCounterSettingsFactory.CreateSettings(
_counterOptions.CurrentValue,
includeDefaults: true,
durationSeconds: durationSeconds,
refreshInterval: metricsIntervalSeconds);
durationSeconds: durationSeconds);

await using EventCounterPipeline eventCounterPipeline = new EventCounterPipeline(client,
settings,
Expand All @@ -89,7 +86,6 @@ public Task<ActionResult> CaptureMetrics(
/// <param name="uid">The Runtime instance cookie used to identify the target process.</param>
/// <param name="name">Process name used to identify the target process.</param>
/// <param name="durationSeconds">The duration of the metrics session (in seconds).</param>
/// <param name="metricsIntervalSeconds">The reporting interval (in seconds) for event counters.</param>
/// <param name="egressProvider">The egress provider to which the metrics are saved.</param>
[HttpPost("livemetrics", Name = nameof(CaptureMetricsCustom))]
[ProducesWithProblemDetails(ContentTypes.ApplicationJsonSequence)]
Expand All @@ -108,8 +104,6 @@ public Task<ActionResult> CaptureMetricsCustom(
string name = null,
[FromQuery][Range(-1, int.MaxValue)]
int durationSeconds = 30,
[FromQuery][Range(1, int.MaxValue)]
int metricsIntervalSeconds = 5,
[FromQuery]
string egressProvider = null)
{
Expand All @@ -123,8 +117,8 @@ public Task<ActionResult> CaptureMetricsCustom(
{
var client = new DiagnosticsClient(processInfo.EndpointInfo.Endpoint);
EventPipeCounterPipelineSettings settings = EventCounterSettingsFactory.CreateSettings(
_counterOptions.CurrentValue,
durationSeconds,
metricsIntervalSeconds,
configuration);

await using EventCounterPipeline eventCounterPipeline = new EventCounterPipeline(client,
Expand Down
Loading