Skip to content

Commit

Permalink
PR Feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
kkeirstead committed Mar 7, 2025
1 parent e5ca091 commit d07998e
Show file tree
Hide file tree
Showing 17 changed files with 150 additions and 157 deletions.
7 changes: 2 additions & 5 deletions documentation/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -2007,14 +2007,11 @@
"additionalProperties": false
},
"MonitorCapability": {
"required": [
"name"
],
"type": "object",
"properties": {
"name": {
"minLength": 1,
"type": "string"
"type": "string",
"nullable": true
},
"enabled": {
"type": "boolean"
Expand Down
12 changes: 12 additions & 0 deletions src/Microsoft.Diagnostics.Monitoring.Options/IMonitorCapability.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Microsoft.Diagnostics.Monitoring.Options
{
public interface IMonitorCapability
{
public string Name { get; }

public bool Enabled { get; }
}
}
Original file line number Diff line number Diff line change
@@ -1,18 +1,22 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.ComponentModel.DataAnnotations;
using System.Text.Json.Serialization;

namespace Microsoft.Diagnostics.Monitoring.Options
{
public class MonitorCapability
public class MonitorCapability : IMonitorCapability
{
[Required]
[JsonPropertyName("name")]
public string Name { get; set; } = string.Empty;
public string Name { get; init; }

[JsonPropertyName("enabled")]
public bool Enabled { get; set; }
public bool Enabled { get; init; }

public MonitorCapability(string name, bool enabled)
{
Name = name;
Enabled = enabled;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ internal static class MonitorCapabilityConstants
{
public const string Exceptions = "exceptions";
public const string ParameterCapturing = "parameters";
public const string CallStacks = "callstacks";
public const string CallStacks = "call_stacks";
public const string Metrics = "metrics";
public const string HttpEgress = "http_egress";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public partial class DiagController : DiagnosticsControllerBase
private readonly ICaptureParametersOperationFactory _captureParametersFactory;
private readonly IGCDumpOperationFactory _gcdumpOperationFactory;
private readonly IStacksOperationFactory _stacksOperationFactory;
private readonly IEnumerable<MonitorCapability> _monitorCapabilities;
private readonly IEnumerable<IMonitorCapability> _monitorCapabilities;
public DiagController(IServiceProvider serviceProvider, ILogger<DiagController> logger)
: base(serviceProvider.GetRequiredService<IDiagnosticServices>(), serviceProvider.GetRequiredService<IEgressOperationStore>(), logger)
{
Expand All @@ -65,7 +65,7 @@ public DiagController(IServiceProvider serviceProvider, ILogger<DiagController>
_captureParametersFactory = serviceProvider.GetRequiredService<ICaptureParametersOperationFactory>();
_gcdumpOperationFactory = serviceProvider.GetRequiredService<IGCDumpOperationFactory>();
_stacksOperationFactory = serviceProvider.GetRequiredService<IStacksOperationFactory>();
_monitorCapabilities = serviceProvider.GetRequiredService<IEnumerable<MonitorCapability>>();
_monitorCapabilities = serviceProvider.GetRequiredService<IEnumerable<IMonitorCapability>>();
}

/// <summary>
Expand Down Expand Up @@ -507,7 +507,7 @@ public ActionResult<DotnetMonitorInfo> GetInfo()
RuntimeVersion = runtimeVersion,
DiagnosticPortMode = diagnosticPortMode,
DiagnosticPortName = diagnosticPortName,
Capabilities = _monitorCapabilities.ToArray()
Capabilities = _monitorCapabilities.Select(c => new MonitorCapability(c.Name, c.Enabled)).ToArray()
};

Logger.WrittenToHttpStream();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,12 @@ public Task InfoEndpointValidationTest(DiagnosticPortConnectionMode mode, bool e
}

Assert.Equal(5, info.Capabilities.Length); // Update if capabilities change
Assert.Contains(info.Capabilities, capability => capability.Name == MonitorCapabilityConstants.Exceptions);
Assert.Contains(info.Capabilities, capability => capability.Name == MonitorCapabilityConstants.ParameterCapturing);
Assert.Contains(info.Capabilities, capability => capability.Name == MonitorCapabilityConstants.Metrics);
Assert.Contains(info.Capabilities, capability => capability.Name == MonitorCapabilityConstants.CallStacks);
Assert.Contains(info.Capabilities, capability => capability.Name == MonitorCapabilityConstants.HttpEgress);
Assert.True(info.Capabilities.First(c => c.Name == MonitorCapabilityConstants.HttpEgress).Enabled);
Assert.True(info.Capabilities.First(c => c.Name == MonitorCapabilityConstants.Metrics).Enabled);
Assert.Equal(enableInProcessFeatures, info.Capabilities.First(c => c.Name == MonitorCapabilityConstants.Exceptions).Enabled);
Assert.Equal(enableInProcessFeatures, info.Capabilities.First(c => c.Name == MonitorCapabilityConstants.ParameterCapturing).Enabled);
Assert.Equal(enableInProcessFeatures, info.Capabilities.First(c => c.Name == MonitorCapabilityConstants.CallStacks).Enabled);

AssertCapability(enableInProcessFeatures, MonitorCapabilityConstants.Exceptions, info.Capabilities);
AssertCapability(enableInProcessFeatures, MonitorCapabilityConstants.ParameterCapturing, info.Capabilities);
AssertCapability(enableInProcessFeatures, MonitorCapabilityConstants.CallStacks, info.Capabilities);
AssertCapability(true, MonitorCapabilityConstants.Metrics, info.Capabilities);
AssertCapability(true, MonitorCapabilityConstants.HttpEgress, info.Capabilities);

await runner.SendCommandAsync(TestAppScenarios.AsyncWait.Commands.Continue);
},
Expand All @@ -93,5 +89,12 @@ public Task InfoEndpointValidationTest(DiagnosticPortConnectionMode mode, bool e
}
});
}

private static void AssertCapability(bool expectEnabled, string capabilityName, IMonitorCapability[] capabilities)
{
IMonitorCapability capability = capabilities.First(c => c.Name == capabilityName);
Assert.NotNull(capability);
Assert.Equal(expectEnabled, capability.Enabled);
}
}
}
21 changes: 21 additions & 0 deletions src/Tools/dotnet-monitor/CallStacksCapability.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// 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.Options;
using Microsoft.Extensions.Options;

namespace Microsoft.Diagnostics.Monitoring
{
public class CallStacksCapability : IMonitorCapability
{
public string Name => MonitorCapabilityConstants.CallStacks;

public bool Enabled { get; init; }

public CallStacksCapability(
IOptions<CallStacksOptions> options)
{
Enabled = options.Value.GetEnabled();
}
}
}

This file was deleted.

28 changes: 0 additions & 28 deletions src/Tools/dotnet-monitor/CapabilitiesPostConfigureOptions.cs

This file was deleted.

21 changes: 21 additions & 0 deletions src/Tools/dotnet-monitor/ExceptionsCapability.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// 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.Options;
using Microsoft.Extensions.Options;

namespace Microsoft.Diagnostics.Monitoring
{
public class ExceptionsCapability : IMonitorCapability
{
public string Name => MonitorCapabilityConstants.Exceptions;

public bool Enabled { get; init; }

public ExceptionsCapability(
IOptions<ExceptionsOptions> options)
{
Enabled = options.Value.GetEnabled();
}
}
}

This file was deleted.

20 changes: 20 additions & 0 deletions src/Tools/dotnet-monitor/HttpEgressCapability.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// 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.Options;

namespace Microsoft.Diagnostics.Monitoring
{
public class HttpEgressCapability : IMonitorCapability
{
public string Name => MonitorCapabilityConstants.HttpEgress;

public bool Enabled { get; init; }

public HttpEgressCapability(
bool isEnabled)
{
Enabled = isEnabled;
}
}
}
22 changes: 22 additions & 0 deletions src/Tools/dotnet-monitor/MetricsCapability.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// 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.Options;
using Microsoft.Diagnostics.Monitoring.WebApi;
using Microsoft.Extensions.Options;

namespace Microsoft.Diagnostics.Monitoring
{
public class MetricsCapability : IMonitorCapability
{
public string Name => MonitorCapabilityConstants.Metrics;

public bool Enabled { get; init; }

public MetricsCapability(
IOptions<MetricsOptions> options)
{
Enabled = options.Value.GetEnabled();
}
}
}
23 changes: 0 additions & 23 deletions src/Tools/dotnet-monitor/MetricsCapabilityPostConfigureOptions.cs

This file was deleted.

21 changes: 21 additions & 0 deletions src/Tools/dotnet-monitor/ParametersCapability.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// 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.Options;
using Microsoft.Extensions.Options;

namespace Microsoft.Diagnostics.Monitoring
{
public class ParametersCapability : IMonitorCapability
{
public string Name => MonitorCapabilityConstants.ParameterCapturing;

public bool Enabled { get; init; }

public ParametersCapability(
IOptions<ParameterCapturingOptions> options)
{
Enabled = options.Value.GetEnabled();
}
}
}

This file was deleted.

21 changes: 5 additions & 16 deletions src/Tools/dotnet-monitor/ServiceCollectionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -243,22 +243,11 @@ public static IServiceCollection ConfigureStorage(this IServiceCollection servic

public static IServiceCollection ConfigureCapabilities(this IServiceCollection services, bool noHttpEgress)
{
ConfigureCapability<MetricsOptions, MetricsCapabilityPostConfigureOptions>(services, MonitorCapabilityConstants.Metrics);
ConfigureCapability<ExceptionsOptions, ExceptionsCapabilityPostConfigureOptions>(services, MonitorCapabilityConstants.Exceptions);
ConfigureCapability<CallStacksOptions, CallStacksCapabilityPostConfigureOptions>(services, MonitorCapabilityConstants.CallStacks);
ConfigureCapability<ParameterCapturingOptions, ParametersCapabilityPostConfigureOptions>(services, MonitorCapabilityConstants.ParameterCapturing);

services.AddSingleton(new MonitorCapability() { Name = MonitorCapabilityConstants.HttpEgress, Enabled = !noHttpEgress });

return services;
}

private static IServiceCollection ConfigureCapability<TOptions, TPostOptions>(this IServiceCollection services, string capabilityName)
where TOptions : class
where TPostOptions : class, IPostConfigureOptions<TOptions>
{
services.AddSingleton(new MonitorCapability() { Name = capabilityName });
services.AddSingleton<IPostConfigureOptions<TOptions>, TPostOptions>();
services.AddSingleton<IMonitorCapability, ExceptionsCapability>();
services.AddSingleton<IMonitorCapability, ParametersCapability>();
services.AddSingleton<IMonitorCapability, CallStacksCapability>();
services.AddSingleton<IMonitorCapability, MetricsCapability>();
services.AddSingleton<IMonitorCapability>(new HttpEgressCapability(!noHttpEgress));

return services;
}
Expand Down

0 comments on commit d07998e

Please sign in to comment.