Add an option to set DCP log file name suffix based on test name #9088
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
area-app-model
There was a problem hiding this comment.
Pull Request Overview
This pull request adds an option to allow DCP log files to incorporate the test name into their suffix, making it easier to correlate logs with specific tests.
- Test program now sets the "LogFileNameSuffix" configuration based on the test name.
- DcpOptions is updated with a new nullable property and corresponding documentation.
- DcpHost now uses the new option to add an environment variable if the suffix is provided.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/testproject/TestProject.AppHost/TestProgram.cs | Adds a new configuration entry for log file suffix |
| src/Aspire.Hosting/Dcp/DcpOptions.cs | Introduces the LogFileNameSuffix property and documentation |
| src/Aspire.Hosting/Dcp/DcpHost.cs | Applies the new option to set an environment variable for DCP logging |
Comments suppressed due to low confidence (1)
tests/testproject/TestProject.AppHost/TestProgram.cs:57
- [nitpick] Consider adding unit or integration tests to verify that the log file name suffix is correctly propagated and applied, ensuring the expected log file naming format is maintained.
builder.Configuration["DcpPublisher:LogFileNameSuffix"] = testName;
davidfowl
reviewed
May 3, 2025
| @@ -54,6 +54,7 @@ private TestProgram( | |||
| builder.Configuration["DcpPublisher:ResourceNameSuffix"] = $"{Random.Shared.Next():x}"; | |||
| builder.Configuration["DcpPublisher:RandomizePorts"] = randomizePorts.ToString(CultureInfo.InvariantCulture); | |||
| builder.Configuration["DcpPublisher:WaitForResourceCleanup"] = "true"; | |||
| builder.Configuration["DcpPublisher:LogFileNameSuffix"] = testName; | |||
There was a problem hiding this comment.
Do we have an issue for doing this in more places?
There was a problem hiding this comment.
I do not think we do. Do you want me to open one?
There was a problem hiding this comment.
Yep. This is a very specific test.
There was a problem hiding this comment.
Created #9106 to track that work
davidfowl
approved these changes
May 5, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adds a DCP option that allows DCP log names to reflect the test that launched the specific DCP instance (and leverages it for
DistributedApplicationTests). With this change DCP log names follow the following formatfor example
dcp-12345678-proxyless-endpoint-works.log(forproxyless-endpoint-workstest), making it very easy to correlate between test under investigation and corresponding DCP log file.The environment variable that this change is using is not effective yet with the DCP version that is integrated into Aspire, but it will become effective with the next DCP insertion.
Checklist
<remarks />and<code />elements on your triple slash comments?doc-ideatemplatebreaking-changetemplatediagnostictemplate