Add a Logger property to IDistributedApplicationResourceEvent
#13732
Conversation
Fixes dotnet#7531. - Add `IServiceProvider Services` to `IDistributedApplicationResourceEvent`. All existing implementations already had this property, so it doesn't really change anything, but provided a default interface method to maintain backwards compatibility. - Add an ILogger property to `IDistributedApplicationResourceEvent`, with a default implementation using `IDistributedApplicationEvent.Services` to get the resource logger - Update xml docs of Service properties to inherit from the parent interface
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 13732Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 13732" |
dotnet-policy-service
bot
added
the
community-contribution
| /// <summary> | ||
| /// The <see cref="IServiceProvider"/> instance. | ||
| /// </summary> | ||
| IServiceProvider Services => throw new NotImplementedException(); |
There was a problem hiding this comment.
This is the one part I'm a bit unsure about. It's really nice about how this lets Logger just light up.
But if you just go an implement the interface, the IDE won't add it automatically for you, and you could easily miss that you need to implement the Services property.
There was a problem hiding this comment.
Pull request overview
This PR adds a convenient Logger property to IDistributedApplicationResourceEvent to simplify resource event logging. Previously, developers had to manually obtain the resource logger using evt.Services.GetRequiredService<ResourceLoggerService>().GetLogger(evt.Resource), which required knowledge of internal service APIs.
Key Changes:
- Added
Servicesproperty to theIDistributedApplicationResourceEventinterface with a default implementation that throwsNotImplementedException(for backwards compatibility) - Added
Loggerproperty to the interface that uses theServicesproperty to automatically resolve and return the resource-specific logger - Updated all concrete event class implementations to use
<inheritdoc />for theirServicesproperty documentation, inheriting from the parent interface
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Hosting/Eventing/IDistributedApplicationEvent.cs | Adds Services and Logger properties to IDistributedApplicationResourceEvent interface with default implementations |
| src/Aspire.Hosting/ApplicationModel/ResourceStoppedEvent.cs | Updates XML docs to use <inheritdoc /> for Services property |
| src/Aspire.Hosting/ApplicationModel/ResourceReadyEvent.cs | Updates XML docs to use <inheritdoc /> for Services property |
| src/Aspire.Hosting/ApplicationModel/ResourceEndpointsAllocatedEvent.cs | Updates XML docs to use <inheritdoc /> for Services property |
| src/Aspire.Hosting/ApplicationModel/ConnectionStringAvailableEvent.cs | Updates XML docs to use <inheritdoc /> for Services property |
| src/Aspire.Hosting/ApplicationModel/BeforeResourceStartedEvent.cs | Updates XML docs to use <inheritdoc /> for Services property |
| src/Aspire.Hosting.DevTunnels/DevTunnelResourceBuilderExtensions.cs | Removes unused DevTunnelResourceStartedEvent private class that implemented the interface |
| /// <summary> | ||
| /// The <see cref="IServiceProvider"/> instance. | ||
| /// </summary> |
There was a problem hiding this comment.
The XML documentation for the Services property should provide more descriptive content beyond just the type. According to the Aspire documentation guidelines, properties should not simply restate the member name. Consider adding a description that explains its purpose, such as "Gets the service provider for the application host that can be used to resolve additional services."
| /// <summary> | ||
| /// An instance of <see cref="ILogger"/> that can be used to log messages for the resource. | ||
| /// </summary> | ||
| ILogger Logger => Services.GetRequiredService<ResourceLoggerService>().GetLogger(Resource); |
There was a problem hiding this comment.
According to Aspire documentation guidelines, the Logger property should include a <remarks> section explaining when and how to use this property. Consider adding remarks to clarify that this logger is specifically scoped to the associated resource and will write to that resource's log stream, which is visible in the Aspire dashboard.
1 participant
Description
IServiceProvider ServicestoIDistributedApplicationResourceEvent. All existing implementations already had this property, so it doesn't really change anything, but provided a default interface method to maintain backwards compatibility.IDistributedApplicationResourceEvent, with a default implementation usingIDistributedApplicationEvent.Servicesto get the resource loggerFixes #7531.
Checklist
<remarks />and<code />elements on your triple slash comments?