Skip to content

Fix monitor_healthmodels_entity_get HTTP 400 error #2896

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

Open
jairmyree wants to merge 4 commits into
base: main
Choose a base branch
from
+44 −2
Open

Fix monitor_healthmodels_entity_get HTTP 400 error #2896

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7dc5489
Fix monitor_healthmodels_entity_get HTTP 400 error
jairmyree Jun 16, 2026
f1c7bca
Move HealthModel service URL/api-version tests into EntityGetHealthCo…
Copilot Jun 16, 2026
04b8d0e
Restructure HealthModel tests: unit in EntityGetHealthCommandTests, l…
Copilot Jun 16, 2026
8ed7390
Assert 400 status in Should_Return400_WithInvalidHealthModelInput
Copilot Jun 23, 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
4 changes: 4 additions & 0 deletions servers/Azure.Mcp.Server/changelog-entries/jairmyree-fix-healthmodels-entity-get.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
changes:
- section: "Bugs Fixed"
description: |
Fixed `monitor_healthmodels_entity_get` returning HTTP 400 (Bad Request) when querying entity health. The tool now uses a supported `Microsoft.CloudHealth` control-plane API version (`2025-05-01-preview`), and the data-plane request URL is built correctly by joining the dataplane endpoint with a path separator and URL-encoding the entity name.
4 changes: 2 additions & 2 deletions tools/Azure.Mcp.Tools.Monitor/src/Services/MonitorHealthModelService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace Azure.Mcp.Tools.Monitor.Services;
public class MonitorHealthModelService(ITenantService tenantService, IHttpClientFactory httpClientFactory)
: BaseAzureService(tenantService), IMonitorHealthModelService
{
private const string ApiVersion = "2023-10-01-preview";
private const string ApiVersion = "2025-05-01-preview";
private readonly IHttpClientFactory _httpClientFactory = httpClientFactory ?? throw new ArgumentNullException(nameof(httpClientFactory));
private readonly ITenantService _tenantService = tenantService ?? throw new ArgumentNullException(nameof(tenantService));

Expand Down Expand Up @@ -45,7 +45,7 @@ public async Task<JsonNode> GetEntityHealth(
ValidateRequiredParameters((nameof(entity), entity), (nameof(healthModelName), healthModelName), (nameof(resourceGroupName), resourceGroupName), (nameof(subscription), subscription));

string dataplaneEndpoint = await GetDataplaneEndpointAsync(subscription, resourceGroupName, healthModelName, cancellationToken);
string entityHealthUrl = $"{dataplaneEndpoint}api/entities/{entity}/history";
string entityHealthUrl = $"{dataplaneEndpoint.TrimEnd('/')}/api/entities/{Uri.EscapeDataString(entity)}/history";

string healthResponseString = await GetDataplaneResponseAsync(entityHealthUrl, cancellationToken);
return JsonNode.Parse(healthResponseString) ?? throw new Exception("Failed to parse health response to JSON.");
Expand Down
38 changes: 38 additions & 0 deletions tools/Azure.Mcp.Tools.Monitor/tests/Azure.Mcp.Tools.Monitor.Tests/MonitorCommandTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public sealed class MonitorCommandTests : RecordedCommandTestsBase
private string? _storageAccountName;
private string? _appInsightsName;
private string? _bingWebTestName;
private string? _healthModelName;

public MonitorCommandTests(ITestOutputHelper output, TestProxyFixture fixture, LiveServerFixture liveServerFixture)
: base(output, fixture, liveServerFixture)
Expand Down Expand Up @@ -91,6 +92,7 @@ public override async ValueTask InitializeAsync()
_storageAccountName = $"{Settings.ResourceBaseName}mon";
_appInsightsName = $"{Settings.ResourceBaseName}-ai";
_bingWebTestName = $"{Settings.ResourceBaseName}-bing-test";
_healthModelName = $"{Settings.ResourceBaseName}-health";

if (TestMode == TestMode.Playback)
{
Expand Down Expand Up @@ -519,6 +521,42 @@ public override async ValueTask DisposeAsync()
// }
// }

#region HealthModels Integration Tests

[Fact]
public async Task Should_Get_Entity_Health()
{
var result = await CallToolAsync(
"monitor_healthmodels_entity_get",
new()
{
{ "subscription", Settings.SubscriptionId },
{ "resource-group", Settings.ResourceGroupName },
{ "health-model", _healthModelName },
{ "entity", "root" }
});

Assert.NotNull(result);
}

[Theory]
[InlineData("--invalid-param")]
[InlineData("--subscription invalidSub")]
[InlineData("--subscription sub --resource-group rg")] // Missing required entity/health-model
public async Task Should_Return400_WithInvalidHealthModelInput(string args)
{
var result = await CallToolAsync(
"monitor_healthmodels_entity_get",
new()
{
{ "args", args }
});

Assert.NotEqual(200, result?.GetProperty("status").GetInt32() ?? 500);

@JasonYeMSFT JasonYeMSFT Jun 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If result?.GetProperty("status").GetInt32() returns null, are we supposed to consider this as a pass?

@jairmyree jairmyree Jun 23, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@JasonYeMSFT Equating a null response to a 500 is logically sound as it would indicate an issue connecting to the service, which isn't the situation the test aims to cover. With that being said, I think it makes more sense to have the test actually test for the expected status code rather than it not being a 200.

@jairmyree jairmyree Jun 23, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@copilot the Should_Return400_WithInvalidHealthModelInput test should actually test that a 400 status code is returned. Not that a 200 status code is not returned.

Copilot AI Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Updated Should_Return400_WithInvalidHealthModelInput to assert the status code is exactly 400 (Assert.Equal(400, result?.GetProperty("status").GetInt32())) instead of just not 200.

}

#endregion

#region WebTests Integration Tests

[Fact]
Expand Down
Loading