Prevent duplicate logs with aspire --watch#14601
Prevent duplicate logs with aspire --watch#14601sebastienros wants to merge 2 commits intorelease/13.2from
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14601Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14601" |
sebastienros
force-pushed
the
sebros/watchspam
branch
from
5a996de to
107e1fe
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #14405 where aspire resources --watch was spamming duplicate output when resource states remained unchanged. The fix introduces deduplication logic that compares the formatted output string for each resource and suppresses consecutive identical outputs.
Changes:
- Adds a
lastDisplayedOutputdictionary to track the last output string displayed for each resource - Refactors
DisplayResourceUpdatefrom an instance method to a staticFormatResourceUpdatemethod that returns a string instead of directly outputting it - Implements string comparison-based deduplication logic that works for both JSON and human-readable output formats
|
Is this output usable you think or should we improve it as well? Maybe docker events is a reasonable thing to compare against |
| var allResources = new Dictionary<string, ResourceSnapshot>(StringComparer.OrdinalIgnoreCase); | ||
|
|
||
| // Track last displayed output per resource to suppress duplicates | ||
| var lastDisplayedOutput = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase); |
There was a problem hiding this comment.
Out of curiosity, why OrdinalIgnoreCase? If the whole point is to de-dupe identical strings, shouldn't this just be Ordinal? Same goes for the actual deduping happening below in line 219
There was a problem hiding this comment.
One more minor thing ( no need to address here but worth thinking about), but looks like both this dictionary and the one above currently grow unbounded. I know you can only have a limited number of resources in an aspire app, but curious if you think it would be relevant to handle this case to avoid potential memory pressure.
There was a problem hiding this comment.
I initially compared with == but was I preferred to be explicit about the comparison, and then also decided to use OrdinalIgnoreCase because this is the default comparer used for resource names. Actually, if the file is available I should use the existing constant.
There was a problem hiding this comment.
I don't think we should care about memory pressure. It would only be the case if new dynamic resources kept being created, which would probably be an issue in another component.
|
Why does the backchannel API send duplicates? Wouldn't it be better to filter them at the source rather than all consumers of the API haven't to track and filter duplicates. |
|
I don't know that they are dupes, I think there are properties changing that are not displayed and as a result, we end up displaying what looks like a dupe. |
🎬 CLI E2E Test RecordingsThe following terminal recordings are available for commit
📹 Recordings uploaded automatically from CI run #22318614147 |
Fixes #14405
New output: