Skip to content

DI Refactor: Azure.Mcp.Tools.ApplicationInsights #1899

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

Draft
Copilot wants to merge 6 commits into
base: main
Choose a base branch
from
+21 −14
Draft

DI Refactor: Azure.Mcp.Tools.ApplicationInsights #1899

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
67d90b5
Initial plan
Copilot Mar 2, 2026
0a97c25
DI refactor: inject IApplicationInsightsService into RecommendationLi…
Copilot Mar 2, 2026
529b4b9
Add shared _commandDefinition field to RecommendationListCommandTests…
Copilot Mar 2, 2026
a1970c9
Fix test naming to match DI refactor pattern (Step 5)
Copilot Mar 3, 2026
34ae7f6
Add CHANGELOG entry for ApplicationInsights DI refactor (Step 5)
Copilot Mar 3, 2026
5e50fe2
Add PR number to ApplicationInsights DI refactor CHANGELOG entry
Copilot Mar 3, 2026
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
3 changes: 3 additions & 0 deletions servers/Azure.Mcp.Server/changelog-entries/copilot-di-refactor-applicationinsights.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
changes:
- section: "Other Changes"
description: "Refactored `ApplicationInsights` tools to use constructor dependency injection."
6 changes: 3 additions & 3 deletions ...re.Mcp.Tools.ApplicationInsights/src/Commands/Recommendation/RecommendationListCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@

Copy link
Member

@conniey conniey Mar 3, 2026

Choose a reason for hiding this comment

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

@copilot Double-check that Step 5 was done correctly.

Copy link
Contributor Author

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

The test was missing a few parts of the Step 5 pattern. Fixed in a1970c9:

  • Renamed _serviceMock → _applicationInsightsService (consistent with service name convention)
  • Moved logger from local variable to private readonly ILogger<RecommendationListCommand> _logger class field
  • Changed constructor call to use implicit new(_logger, _applicationInsightsService) syntax

The test now matches the same pattern as AccountListCommandTests (AppConfig).

Copy link
Member

@conniey conniey Mar 3, 2026

Choose a reason for hiding this comment

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

@copilot Step 5 includes executing New-ChangeLogEntry.ps1 to generate CHANGELOG entry. This is not done.

Copy link
Contributor Author

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

Done in 34ae7f6. Generated servers/Azure.Mcp.Server/changelog-entries/copilot-di-refactor-applicationinsights.yaml via New-ChangelogEntry.ps1 under "Other Changes".

Copy link
Member

@conniey conniey Mar 3, 2026

Choose a reason for hiding this comment

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

@copilot it is missing the PR# in the generated changelog. try again.

Copy link
Contributor Author

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

Added pr: 1899 to the changelog entry in 5e50fe2.

namespace Azure.Mcp.Tools.ApplicationInsights.Commands.Recommendation;

public sealed class RecommendationListCommand(ILogger<RecommendationListCommand> logger) : SubscriptionCommand<RecommendationListOptions>()
public sealed class RecommendationListCommand(ILogger<RecommendationListCommand> logger, IApplicationInsightsService applicationInsightsService) : SubscriptionCommand<RecommendationListOptions>()
{
private const string CommandTitle = "List Application Insights Recommendations";
private readonly ILogger<RecommendationListCommand> _logger = logger;
private readonly IApplicationInsightsService _applicationInsightsService = applicationInsightsService;

public override string Id => "8d259f21-43b3-4962-bec8-de616b8b5f0d";

Expand Down Expand Up @@ -58,8 +59,7 @@ public override async Task<CommandResponse> ExecuteAsync(CommandContext context,
var options = BindOptions(parseResult);
try
{
var service = context.GetService<IApplicationInsightsService>();
var insights = await service.GetProfilerInsightsAsync(
var insights = await _applicationInsightsService.GetProfilerInsightsAsync(
options.Subscription!,
options.ResourceGroup,
options.Tenant,
Expand Down
25 changes: 14 additions & 11 deletions ...hts/tests/Azure.Mcp.Tools.ApplicationInsights.UnitTests/RecommendationListCommandTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.CommandLine;
using System.Text.Json;
using System.Text.Json.Nodes;
using Azure.Mcp.Core.Options;
Expand All @@ -14,18 +15,20 @@ namespace Azure.Mcp.Tools.ApplicationInsights.UnitTests;
public class RecommendationListCommandTests
{
private readonly IServiceProvider _serviceProvider;
private readonly IApplicationInsightsService _serviceMock;
private readonly IApplicationInsightsService _applicationInsightsService;
private readonly ILogger<RecommendationListCommand> _logger;
private readonly RecommendationListCommand _command;
private readonly CommandContext _context;
private readonly Command _commandDefinition;

public RecommendationListCommandTests()
{
var sc = new ServiceCollection();
_serviceMock = Substitute.For<IApplicationInsightsService>();
sc.AddSingleton(_serviceMock);
var logger = Substitute.For<ILogger<RecommendationListCommand>>();
_serviceProvider = sc.BuildServiceProvider();
_command = new RecommendationListCommand(logger);
_applicationInsightsService = Substitute.For<IApplicationInsightsService>();
_logger = Substitute.For<ILogger<RecommendationListCommand>>();
_command = new(_logger, _applicationInsightsService);
_commandDefinition = _command.GetCommand();
_serviceProvider = new ServiceCollection()
.BuildServiceProvider();
_context = new(_serviceProvider);
}

Expand All @@ -37,14 +40,14 @@ public async Task ExecuteAsync_WhenServiceReturnsInsights_SetsResults()
JsonNode.Parse("{ \"id\": \"rec1\", \"type\": \"cpu\" }")!,
JsonNode.Parse("{ \"id\": \"rec2\", \"type\": \"memory\" }")!
};
_serviceMock.GetProfilerInsightsAsync(
_applicationInsightsService.GetProfilerInsightsAsync(
Arg.Any<string>(),
Arg.Any<string?>(),
Arg.Any<string?>(),
Arg.Any<RetryPolicyOptions?>(),
Arg.Any<CancellationToken>())
.Returns(Task.FromResult<IEnumerable<JsonNode>>(insights!));
var args = _command.GetCommand().Parse(["--subscription", "sub1"]);
var args = _commandDefinition.Parse(["--subscription", "sub1"]);
await _command.ExecuteAsync(_context, args, TestContext.Current.CancellationToken);
Assert.NotNull(_context.Response.Results);
var json = JsonSerializer.Serialize(_context.Response.Results);
Expand All @@ -57,14 +60,14 @@ public async Task ExecuteAsync_WhenServiceReturnsInsights_SetsResults()
[Fact]
public async Task ExecuteAsync_WhenServiceReturnsNoInsights_NoResults()
{
_serviceMock.GetProfilerInsightsAsync(
_applicationInsightsService.GetProfilerInsightsAsync(
Arg.Any<string>(),
Arg.Any<string?>(),
Arg.Any<string?>(),
Arg.Any<RetryPolicyOptions?>(),
Arg.Any<CancellationToken>())
.Returns(Task.FromResult<IEnumerable<JsonNode>>(Array.Empty<JsonNode>()));
var args = _command.GetCommand().Parse(["--subscription", "sub1"]);
var args = _commandDefinition.Parse(["--subscription", "sub1"]);
await _command.ExecuteAsync(_context, args, TestContext.Current.CancellationToken);
Assert.Null(_context.Response.Results);
}
Expand Down
Loading