Make URLs available on snapshot of unstarted and waiting resources

[JamesNK]

Description

URLs weren't populated for unstarted and waiting resources. This means they weren't properly linked to in traces.

Fix is to make sure ResourceUrlAnnotation values are populated before the snapshot is published.

Checklist

• Is this feature complete?
  • Yes. Ready to ship.
  • No. Follow-up changes expected.
• Are you including unit tests for the changes and scenario tests if relevant?
  • Yes
  • No
• Did you add public API?
  • Yes
    • If yes, did you have an API Review for it?
      • Yes
      • No
    • Did you add <remarks /> and <code /> elements on your triple slash comments?
      • Yes
      • No
  • No
• Does the change make any security assumptions or guarantees?
  • Yes
    • If yes, have you done a threat model and had a security review?
      • Yes
      • No
  • No
• Does the change require an update in our Aspire docs?
  • Yes
    • Link to aspire-docs issue (consider using one of the following templates):
      • New (or update) doc-idea template
      • New breaking-change template
      • New diagnostic template
  • No

[davidfowl]

Confused by this. Does that mean urls will be clickable before the resource starts?

[JamesNK]

Confused by this. Does that mean urls will be clickable before the resource starts?

No, because they're inactive. And the UI only displays active URLs.

But traces UI in the dashboard can use them to match requests to a resource. For example, if the catalogservice resource is waiting for the database resource to start, but a request is made that attempts to call it, the traces UI will know show catalogservice as the destination instead of localhost:5392 because it can use the inactive URL to make that connection.

[JamesNK]

This is basically an improvement to #7906. It now works for resources that have never been started, e.g. resource has explicit start annotation, or is in a waiting state.

[JamesNK]

Actually, my change in #7906 originally supported unstarted/waiting resources. But it was regressed in the change to support custom URLs: #8229

The fix for this regression is calling ProcessUrls before DCP info is pushed into the snapshot.

[davidfowl]

cc @DamianEdwards

[DamianEdwards]

Does moving this to DcpExecutor impact adding URLs to custom resources? Or is the name just misleading 😁

[JamesNK]

I moved ProcessUrls out of the OnResourceStarting method. That method is only called by the DCP. I believe this only would have been called for DCP resources.

Do you intend to ProcessUrls to be called for non-DCP resources? I assume non-DCP resources should have ResourceUrlAnnotation added to them directly.

[DamianEdwards]

Hmm this might actually be an oversight in the original implementation. We had intended for URLs to be supported on any resource type, not just resources with endpoints (which is why the WithUrls extension method is only constrained to IResource) but the code in ApplicationOrchestrator is only calling callbacks for IResourceWithEndpoints resources. I'll log an issue to track this.

[DamianEdwards]

#9128