Skip to content
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.

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

Make resource HealthStatus computed from HealthReports #6368

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
43ac440
Make resource HealthStatus computed from HealthReports
Oct 18, 2024
8b3406a
change HealthStatus back to property, fix test
Oct 18, 2024
c608065
Update HealthStatus when state changes
Oct 18, 2024
d8556f4
Return null HealthStatus when not running, use constant
Oct 18, 2024
8f605c9
add 16 to reserved fields
Oct 18, 2024
d4d6849
add default argument to _healthReports, re-add property
Oct 18, 2024
5955d5f
Say resource is unhealthy if it is running but a health check has not…
Oct 18, 2024
a29cc72
Change Health State -> Health state
Oct 18, 2024
e817af7
Remove redundant condition
Oct 18, 2024
20dd730
clean up
Oct 18, 2024
b864dad
add test for ComputeHealthStatus
Oct 18, 2024
1c0c161
Disambiguate have not received health reports and empty health reports
Oct 18, 2024
0e9aa7e
Revert change, add health reports to test PublishUpdate to reflect ac…
Oct 18, 2024
908ad8d
Set health reports to [] on resources that don't have health checks
Oct 18, 2024
b151230
Merge branch 'main' into dev/adamint/remove-aggregated-healthstatus
Oct 18, 2024
5d2d053
test disabling all rhc tests
Oct 19, 2024
4c5858e
Merge branch 'main' into dev/adamint/remove-aggregated-healthstatus
Oct 19, 2024
f6dd57c
try enabling 4 tests
Oct 19, 2024
3050ff8
enable additional 3 tests
Oct 19, 2024
94ee0cc
re-skip 2 checks
Oct 19, 2024
aeb4618
add initial health snapshots in resource notification service
Oct 19, 2024
b696e47
Fix playground.
mitchdenny Oct 20, 2024
85ac554
set health report on publish
Oct 21, 2024
50c92d9
run CI again
Oct 21, 2024
6f96288
re-add additional test
Oct 21, 2024
4a3f64d
re-enable additional test
Oct 21, 2024
e937b92
re-enable last test
Oct 21, 2024
55b5629
remove the redundant parentheses
Oct 21, 2024
b352068
remove duplicate logic
Oct 21, 2024
6056a29
clean up
Oct 21, 2024
3199d76
remove unnecessary newlines
Oct 21, 2024
b3ecfeb
remove HealthAnnotationsInitialized
Oct 21, 2024
c38a5ff
add comment
Oct 22, 2024
2bb8e07
re-add health status computation in dashboard
Oct 22, 2024
feb1dc7
forgot newline
Oct 22, 2024
5145c38
fix test
Oct 22, 2024
b88279a
Add a healthy check resources in to health check sandbox to showcase …
Oct 22, 2024
5b91f84
Update resource health reports if any have changed after the health c…
Oct 22, 2024
69abad0
move logic into above condition
Oct 22, 2024
f7127eb
extract to local static method
Oct 22, 2024
0129991
use string comparers
Oct 22, 2024
8a02eb5
return true if health check name is not found in existing health reports
Oct 22, 2024
2799a2a
Merge branch 'main' into dev/adamint/remove-aggregated-healthstatus
Oct 22, 2024
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
1 change: 0 additions & 1 deletion src/Aspire.Dashboard/Model/KnownPropertyLookup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ public KnownPropertyLookup(IStringLocalizer<Resources.Resources> loc)
new(KnownProperties.Resource.StartTime, loc[nameof(ResourcesDetailsStartTimeProperty)]),
new(KnownProperties.Resource.StopTime, loc[nameof(ResourcesDetailsStopTimeProperty)]),
new(KnownProperties.Resource.ExitCode, loc[nameof(ResourcesDetailsExitCodeProperty)]),
new(KnownProperties.Resource.HealthState, loc[nameof(ResourcesDetailsHealthStateProperty)])
davidfowl marked this conversation as resolved.
Show resolved Hide resolved
];

_projectProperties =
Expand Down
2 changes: 1 addition & 1 deletion src/Aspire.Dashboard/Model/ResourceStateViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ private static string GetStateText(ResourceViewModel resource, IStringLocalizer<
return resource switch
{
{ State: null or "" } => loc[Columns.UnknownStateLabel],
{ KnownState: KnownResourceState.Running, HealthStatus: not HealthStatus.Healthy } => $"{resource.State.Humanize()} ({(resource.HealthStatus ?? HealthStatus.Unhealthy).Humanize()})",
{ KnownState: KnownResourceState.Running, HealthStatus: not HealthStatus.Healthy, HealthReports: not [] } => $"{resource.State.Humanize()} ({(resource.HealthStatus ?? HealthStatus.Unhealthy).Humanize()})",
_ => resource.State.Humanize()
};
}
Expand Down
16 changes: 15 additions & 1 deletion src/Aspire.Dashboard/Model/ResourceViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,22 @@ public sealed class ResourceViewModel
public required ImmutableArray<VolumeViewModel> Volumes { get; init; }
public required FrozenDictionary<string, ResourcePropertyViewModel> Properties { get; init; }
public required ImmutableArray<CommandViewModel> Commands { get; init; }

/// <summary>The health status of the resource. <see langword="null"/> indicates that health status is expected but not yet available.</summary>
public required HealthStatus? HealthStatus { get; init; }
public HealthStatus? HealthStatus
{
get
{
if (HealthReports.Length == 0 && KnownState == KnownResourceState.Running)
{
// If there are no health reports and the resource is running, assume it's healthy.
return Microsoft.Extensions.Diagnostics.HealthChecks.HealthStatus.Healthy;
}

return HealthReports.MinBy(r => r.HealthStatus)?.HealthStatus;
}
}
adamint marked this conversation as resolved.
Show resolved Hide resolved

public required ImmutableArray<HealthReportViewModel> HealthReports { get; init; }
public KnownResourceState? KnownState { get; init; }

Expand Down
1 change: 0 additions & 1 deletion src/Aspire.Dashboard/ResourceService/Partials.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ public ResourceViewModel ToViewModel(BrowserTimeProvider timeProvider, IKnownPro
KnownState = HasState ? Enum.TryParse(State, out KnownResourceState knownState) ? knownState : null : null,
StateStyle = HasStateStyle ? StateStyle : null,
Commands = GetCommands(),
HealthStatus = HasHealthStatus ? MapHealthStatus(HealthStatus) : null,
adamint marked this conversation as resolved.
Show resolved Hide resolved
HealthReports = HealthReports.Select(ToHealthReportViewModel).OrderBy(vm => vm.Name).ToImmutableArray(),
};
}
Expand Down
14 changes: 11 additions & 3 deletions src/Aspire.Hosting/ApplicationModel/CustomResourceSnapshot.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,19 @@ public sealed record CustomResourceSnapshot
/// </summary>
/// <remarks>
/// <para>
/// This value is derived from <see cref="HealthReports"/>. If a resource is known to have a health check
/// and no reports exist, or if a resource does not have a health check, then this value is <see langword="null"/>.
/// This value is derived from <see cref="HealthReports"/>.
/// </para>
/// </remarks>
public HealthStatus? HealthStatus { get; init; }
public HealthStatus? GetHealthStatus()
{
if (HealthReports.Length == 0 && State == "Running")
{
// If there are no health reports and the resource is running, assume it's healthy.
return HealthStatus.Healthy;
}

return HealthReports.MinBy(r => r.Status)?.Status;
}
adamint marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// The health reports for this resource.
Expand Down
20 changes: 2 additions & 18 deletions src/Aspire.Hosting/ApplicationModel/ResourceNotificationService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ static bool IsContinuableState(CustomResourceSnapshot snapshot) =>
/// </remarks>
public Task<ResourceEvent> WaitForResourceHealthyAsync(string resourceName, CancellationToken cancellationToken = default)
{
return WaitForResourceAsync(resourceName, re => re.Snapshot.HealthStatus == HealthStatus.Healthy, cancellationToken: cancellationToken);
return WaitForResourceAsync(resourceName, re => re.Snapshot.GetHealthStatus() == HealthStatus.Healthy, cancellationToken: cancellationToken);
}

private async Task WaitUntilCompletionAsync(IResource resource, IResource dependency, int exitCode, CancellationToken cancellationToken)
Expand Down Expand Up @@ -352,7 +352,6 @@ public Task PublishUpdateAsync(IResource resource, string resourceId, Func<Custo
var newState = stateFactory(previousState);

newState = UpdateCommands(resource, newState);
newState = UpdateHealthStatus(resource, newState);

notificationState.LastSnapshot = newState;

Expand All @@ -377,11 +376,10 @@ public Task PublishUpdateAsync(IResource resource, string resourceId, Func<Custo
{
_logger.LogTrace("Resource {Resource}/{ResourceId} update published: " +
"ResourceType = {ResourceType}, CreationTimeStamp = {CreationTimeStamp:s}, State = {{ Text = {StateText}, Style = {StateStyle} }}, " +
"HealthStatus = {HealthStatus} " +
"ExitCode = {ExitCode}, EnvironmentVariables = {{ {EnvironmentVariables} }}, Urls = {{ {Urls} }}, " +
"Properties = {{ {Properties} }}",
resource.Name, resourceId,
newState.ResourceType, newState.CreationTimeStamp, newState.State?.Text, newState.State?.Style, newState.HealthStatus,
newState.ResourceType, newState.CreationTimeStamp, newState.State?.Text, newState.State?.Style,
newState.ExitCode, string.Join(", ", newState.EnvironmentVariables.Select(e => $"{e.Name} = {e.Value}")), string.Join(", ", newState.Urls.Select(u => $"{u.Name} = {u.Url}")),
string.Join(", ", newState.Properties.Select(p => $"{p.Name} = {p.Value}")));
}
Expand All @@ -390,20 +388,6 @@ public Task PublishUpdateAsync(IResource resource, string resourceId, Func<Custo
return Task.CompletedTask;
}

/// <summary>
/// Update resource snapshot health status if the resource is running with no health checks.
/// </summary>
private static CustomResourceSnapshot UpdateHealthStatus(IResource resource, CustomResourceSnapshot previousState)
{
// A resource is also healthy if it has no health check annotations and is in the running state.
if (previousState.HealthStatus is not HealthStatus.Healthy && !resource.TryGetAnnotationsIncludingAncestorsOfType<HealthCheckAnnotation>(out _) && previousState.State?.Text == KnownResourceStates.Running)
{
return previousState with { HealthStatus = HealthStatus.Healthy };
}

return previousState;
}

/// <summary>
/// Use command annotations to update resource snapshot.
/// </summary>
Expand Down
1 change: 0 additions & 1 deletion src/Aspire.Hosting/Dashboard/DashboardServiceData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ static GenericResourceSnapshot CreateResourceSnapshot(IResource resource, string
ExitCode = snapshot.ExitCode,
State = snapshot.State?.Text,
StateStyle = snapshot.State?.Style,
HealthStatus = snapshot.HealthStatus,
HealthReports = GetOrCreateHealthReports(),
Commands = snapshot.Commands
};
Expand Down
3 changes: 0 additions & 3 deletions src/Aspire.Hosting/Dashboard/ResourceSnapshot.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using Aspire.Dashboard.Model;
using Aspire.Hosting.ApplicationModel;
using Google.Protobuf.WellKnownTypes;
using Microsoft.Extensions.Diagnostics.HealthChecks;

namespace Aspire.Hosting.Dashboard;

Expand All @@ -26,7 +25,6 @@ internal abstract class ResourceSnapshot
public required ImmutableArray<EnvironmentVariableSnapshot> Environment { get; init; }
public required ImmutableArray<VolumeSnapshot> Volumes { get; init; }
public required ImmutableArray<UrlSnapshot> Urls { get; init; }
public required HealthStatus? HealthStatus { get; init; }
public required ImmutableArray<HealthReportSnapshot> HealthReports { get; init; }
public required ImmutableArray<ResourceCommandSnapshot> Commands { get; init; }

Expand All @@ -45,7 +43,6 @@ internal abstract class ResourceSnapshot
yield return (KnownProperties.Resource.CreateTime, CreationTimeStamp is null ? Value.ForNull() : Value.ForString(CreationTimeStamp.Value.ToString("O")), IsSensitive: false);
yield return (KnownProperties.Resource.StartTime, StartTimeStamp is null ? Value.ForNull() : Value.ForString(StartTimeStamp.Value.ToString("O")), IsSensitive: false);
yield return (KnownProperties.Resource.StopTime, StopTimeStamp is null ? Value.ForNull() : Value.ForString(StopTimeStamp.Value.ToString("O")), IsSensitive: false);
yield return (KnownProperties.Resource.HealthState, HealthStatus is null ? Value.ForNull() : Value.ForString(HealthStatus.ToString()), IsSensitive: false);

foreach (var property in GetProperties())
{
Expand Down
5 changes: 0 additions & 5 deletions src/Aspire.Hosting/Dashboard/proto/Partials.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,6 @@ public static Resource FromSnapshot(ResourceSnapshot snapshot)
resource.Commands.Add(new ResourceCommand { CommandType = command.Type, DisplayName = command.DisplayName, DisplayDescription = command.DisplayDescription ?? string.Empty, Parameter = ResourceSnapshot.ConvertToValue(command.Parameter), ConfirmationMessage = command.ConfirmationMessage ?? string.Empty, IconName = command.IconName ?? string.Empty, IconVariant = MapIconVariant(command.IconVariant), IsHighlighted = command.IsHighlighted, State = MapCommandState(command.State) });
}

if (snapshot.HealthStatus is not null)
{
resource.HealthStatus = MapHealthStatus(snapshot.HealthStatus.Value);
}

foreach (var report in snapshot.HealthReports)
{
var healthReport = new HealthReport { Key = report.Name, Description = report.Description ?? "", Exception = report.ExceptionText ?? "" };
Expand Down
2 changes: 0 additions & 2 deletions src/Aspire.Hosting/Dashboard/proto/resource_service.proto
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,6 @@ message Resource {
// The set of volumes mounted to the resource. Only applies to containers.
repeated Volume volumes = 15;

// The aggregate health state of the resource. Generally reflects data from health_reports, but may differ.
optional HealthStatus health_status = 16;
adamint marked this conversation as resolved.
Show resolved Hide resolved
// Reports from health checks, about this resource.
repeated HealthReport health_reports = 17;

Expand Down
8 changes: 2 additions & 6 deletions src/Aspire.Hosting/Health/ResourceHealthCheckService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ await eventing.PublishAsync(
cancellationToken).ConfigureAwait(false);
}

if (_latestEvents[resource.Name] is { } latestEvent && latestEvent.Snapshot.HealthStatus == report.Status)
if (_latestEvents[resource.Name] is { } latestEvent && latestEvent.Snapshot.GetHealthStatus() == report.Status)
{
await SlowDownMonitoringAsync(latestEvent, cancellationToken).ConfigureAwait(false);
continue;
Expand All @@ -93,12 +93,8 @@ await resourceNotificationService.PublishUpdateAsync(resource, s =>
{
var healthReports = MergeHealthReports(s.HealthReports, report);

// Matches the logic in ASP.NET Core's private HealthReport.CalculateAggregateStatus
var healthStatus = healthReports.MinBy(r => r.Status)?.Status ?? s.HealthStatus;

return s with
{
HealthStatus = healthStatus,
HealthReports = healthReports
adamint marked this conversation as resolved.
Show resolved Hide resolved
};
}).ConfigureAwait(false);
Expand All @@ -122,7 +118,7 @@ async Task SlowDownMonitoringAsync(ResourceEvent lastEvent, CancellationToken ca

// If we've waited for 30 seconds, or we received an updated event, or the health status is no longer
// healthy then we stop slowing down the monitoring loop.
while (DateTime.Now < releaseAfter && _latestEvents[lastEvent.Resource.Name] == lastEvent && lastEvent.Snapshot.HealthStatus == HealthStatus.Healthy)
while (DateTime.Now < releaseAfter && _latestEvents[lastEvent.Resource.Name] == lastEvent && lastEvent.Snapshot.GetHealthStatus() == HealthStatus.Healthy)
{
await Task.Delay(1000, cancellationToken).ConfigureAwait(false);
}
Expand Down
3 changes: 1 addition & 2 deletions src/Aspire.Hosting/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#nullable enable
Aspire.Hosting.ApplicationModel.ContainerLifetime.Session = 0 -> Aspire.Hosting.ApplicationModel.ContainerLifetime
Aspire.Hosting.ApplicationModel.CustomResourceSnapshot.GetHealthStatus() -> Microsoft.Extensions.Diagnostics.HealthChecks.HealthStatus?
Aspire.Hosting.ApplicationModel.EndpointNameAttribute
Aspire.Hosting.ApplicationModel.EndpointNameAttribute.EndpointNameAttribute() -> void
Aspire.Hosting.ApplicationModel.HealthReportSnapshot
Expand Down Expand Up @@ -46,8 +47,6 @@ Aspire.Hosting.ApplicationModel.ContainerNameAnnotation.Name.get -> string!
Aspire.Hosting.ApplicationModel.ContainerNameAnnotation.Name.set -> void
Aspire.Hosting.ApplicationModel.CustomResourceSnapshot.Commands.get -> System.Collections.Immutable.ImmutableArray<Aspire.Hosting.ApplicationModel.ResourceCommandSnapshot!>
Aspire.Hosting.ApplicationModel.CustomResourceSnapshot.Commands.init -> void
Aspire.Hosting.ApplicationModel.CustomResourceSnapshot.HealthStatus.get -> Microsoft.Extensions.Diagnostics.HealthChecks.HealthStatus?
Aspire.Hosting.ApplicationModel.CustomResourceSnapshot.HealthStatus.init -> void
Aspire.Hosting.ApplicationModel.CustomResourceSnapshot.HealthReports.get -> System.Collections.Immutable.ImmutableArray<Aspire.Hosting.ApplicationModel.HealthReportSnapshot!>
Aspire.Hosting.ApplicationModel.CustomResourceSnapshot.HealthReports.init -> void
Aspire.Hosting.ApplicationModel.CustomResourceSnapshot.StartTimeStamp.get -> System.DateTime?
Expand Down
1 change: 0 additions & 1 deletion src/Shared/Model/KnownProperties.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ public static class Resource
public const string StartTime = "resource.startTime";
public const string StopTime = "resource.stopTime";
public const string Source = "resource.source";
public const string HealthState = "resource.healthState";
public const string ConnectionString = "resource.connectionString";
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ await resourceNotificationService.PublishUpdateAsync(testResource, s =>
});

var resource = Assert.Single((await writer.ReadNextAsync()).Changes.Value).Upsert;
Assert.False(resource.HasHealthStatus);

Assert.Collection(resource.HealthReports,
r =>
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ private static GenericResourceSnapshot CreateResourceSnapshot(string name)
Urls = [],
Volumes = [],
Environment = [],
HealthStatus = null,
HealthReports = [],
Commands = []
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,15 @@ await rns.PublishUpdateAsync(resource.Resource, s => s with
});

var startingEvent = await rns.WaitForResourceAsync("resource", e => e.Snapshot.State?.Text == KnownResourceStates.Starting);
Assert.Null(startingEvent.Snapshot.HealthStatus);
Assert.Null(startingEvent.Snapshot.GetHealthStatus());

await rns.PublishUpdateAsync(resource.Resource, s => s with
{
State = new ResourceStateSnapshot(KnownResourceStates.Running, null)
});

var runningEvent = await rns.WaitForResourceAsync("resource", e => e.Snapshot.State?.Text == KnownResourceStates.Running);
Assert.Equal(HealthStatus.Healthy, runningEvent.Snapshot.HealthStatus);
Assert.Equal(HealthStatus.Healthy, runningEvent.Snapshot.GetHealthStatus());

await app.StopAsync();
}
Expand All @@ -62,18 +62,18 @@ await rns.PublishUpdateAsync(resource.Resource, s => s with
});

var startingEvent = await rns.WaitForResourceAsync("resource", e => e.Snapshot.State?.Text == KnownResourceStates.Starting);
Assert.Null(startingEvent.Snapshot.HealthStatus);
Assert.Null(startingEvent.Snapshot.GetHealthStatus());

await rns.PublishUpdateAsync(resource.Resource, s => s with
{
State = new ResourceStateSnapshot(KnownResourceStates.Running, null)
});

var runningEvent = await rns.WaitForResourceAsync("resource", e => e.Snapshot.State?.Text == KnownResourceStates.Running);
Assert.Null(runningEvent.Snapshot.HealthStatus);
Assert.Null(runningEvent.Snapshot.GetHealthStatus());

var hasHealthReportsEvent = await rns.WaitForResourceAsync("resource", e => e.Snapshot.HealthReports.Length > 0);
Assert.Equal(HealthStatus.Healthy, hasHealthReportsEvent.Snapshot.HealthStatus);
Assert.Equal(HealthStatus.Healthy, hasHealthReportsEvent.Snapshot.GetHealthStatus());

await app.StopAsync();
}
Expand Down
4 changes: 2 additions & 2 deletions tests/Aspire.Hosting.Tests/ResourceNotificationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,15 @@ async Task<List<ResourceEvent>> GetValuesAsync(CancellationToken cancellationTok
Assert.Equal("myResource", c.ResourceId);
Assert.Equal("CustomResource", c.Snapshot.ResourceType);
Assert.Equal("value", c.Snapshot.Properties.Single(p => p.Name == "A").Value);
Assert.Null(c.Snapshot.HealthStatus);
Assert.Null(c.Snapshot.GetHealthStatus());
},
c =>
{
Assert.Equal(resource, c.Resource);
Assert.Equal("myResource", c.ResourceId);
Assert.Equal("CustomResource", c.Snapshot.ResourceType);
Assert.Equal("value", c.Snapshot.Properties.Single(p => p.Name == "B").Value);
Assert.Null(c.Snapshot.HealthStatus);
Assert.Null(c.Snapshot.GetHealthStatus());
});
}

Expand Down
1 change: 0 additions & 1 deletion tests/Shared/DashboardModel/ModelTestHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ public static ResourceViewModel CreateResource(
State = state?.ToString(),
KnownState = state,
StateStyle = stateStyle,
HealthStatus = healthStatus,
HealthReports = [],
Commands = []
};
Expand Down
Loading