Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jul 16, 2025

Problem

A regression was introduced in PR #10411 where database calls to uninstrumented peers no longer match their corresponding database resources in the Aspire Dashboard. This caused database connections to appear unmatched (with red error icons) instead of being properly linked to their database resources.

The issue occurred because the peer matching logic was changed from "first one wins" to "return false if multiple resources match the same address." While this was intended to handle ambiguous cases, it caused legitimate database connections to fail matching when multiple resources shared similar addresses.

Examples

Before (working):
Working database matching

After PR #10411 (broken):
Broken database matching

The red database icon shows the call to localhost:52074 no longer matches the postgres database resource.

Solution

This PR reverts the TryMatchAgainstResources method back to the original "first one wins" logic as requested by @davidfowl for the 9.4 release:

  1. Restored immediate return on first match: The method now returns true immediately when a matching resource is found, instead of checking for multiple matches
  2. Removed problematic tests: Eliminated the 5 test methods that validated the "multiple matches return false" behavior that caused the regression
  3. Updated existing tests: Modified tests to expect the restored "first one wins" behavior

Code Changes

// Before (problematic logic)
if (DoesAddressMatch(resourceAddress, peerAddress))
{
    if (foundResource is null)
    {
        foundResource = resource;
    }
    else if (!string.Equals(foundResource.Name, resource.Name, StringComparisons.ResourceName))
    {
        // Multiple different resources match - return false immediately
        name = null;
        resourceMatch = null;
        return false;
    }
    // ...
}

// After (restored logic)
if (DoesAddressMatch(resourceAddress, peerAddress))
{
    name = ResourceViewModel.GetResourceName(resource, resources);
    resourceMatch = resource;
    return true; // First one wins
}

Impact

  • Database calls to uninstrumented peers (like localhost:52074) now correctly match database resources
  • Replica matching issues mentioned in the issue comments are also resolved
  • The Dashboard properly displays database connections with the correct resource icons

This follows @davidfowl's guidance to revert PR #10411 changes for 9.4, allowing the team to design a more sophisticated approach for handling ambiguous matches in future releases.

Fixes #10452.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

…iling tests

Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com>
Copilot AI changed the title [WIP] Regression when matching to database as uninstrumented peer Revert peer matching logic to fix database resource resolution regression Jul 16, 2025
Copilot AI requested a review from davidfowl July 16, 2025 13:44
Copilot finished work on behalf of davidfowl July 16, 2025 13:44
@davidfowl davidfowl marked this pull request as ready for review July 16, 2025 14:45
Copilot AI review requested due to automatic review settings July 16, 2025 14:45
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR reverts peer matching logic to fix a regression where database calls to uninstrumented peers no longer matched their corresponding database resources in the Aspire Dashboard. The issue was introduced in PR #10411 which changed the logic from "first one wins" to "return false if multiple resources match the same address."

Key changes:

  • Restored immediate return on first match in TryMatchAgainstResources method
  • Removed 5 test methods that validated the problematic "multiple matches return false" behavior
  • Updated existing tests to expect the restored "first one wins" behavior

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/Aspire.Dashboard/Model/ResourceOutgoingPeerResolver.cs Reverted the TryMatchAgainstResources method to return immediately on first match instead of checking for multiple matches
tests/Aspire.Dashboard.Tests/ResourceOutgoingPeerResolverTests.cs Removed 5 problematic test methods and updated existing tests to validate the restored "first one wins" logic

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@davidfowl davidfowl added this to the 9.4 milestone Jul 16, 2025
@davidfowl davidfowl merged commit a5f282e into main Jul 17, 2025
544 of 546 checks passed
@davidfowl davidfowl deleted the copilot/fix-10452 branch July 17, 2025 00:17
@davidfowl
Copy link
Member

/backport release/9.4

@davidfowl
Copy link
Member

/backport to release/9.4

@github-actions
Copy link
Contributor

Started backporting to release/9.4: https://github.com/dotnet/aspire/actions/runs/16333078441

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression when matching to database as uninstrumented peer

3 participants