aspire icon indicating copy to clipboard operation
aspire copied to clipboard

Enhance Azure Event Hubs hosting integration to support AzureEventHubResource child

Open oising opened this issue 1 year ago • 1 comments

Description

Addresses: #5561

  • [x] Add AzureEventHubResource
    • [x] Independent ConnectionString from namespace, includes EntityPath
    • [x] Support Azure Functions bits
  • [ ] Allow configuration of Consumer Groups
    • [ ] Support in Emulator
    • [ ] Support live / Azure
      • [ ] AzureEventHub CDK -- requires support (currently missing)
  • [ ] Allow configuration of Partition Count
    • [ ] Support in Emulator
    • [ ] Support live / Azure
  • [x] Update playground code

This is a breaking change as you can no longer chain .AddEventHub(...) calls, but the connection string will now include the EntityPath. The latter shouldn't cause any issues as the current workaround to this missing element is to override in the settings callback. Settings will continue to override whatever is returned. I believe any breaking changes are limited to the AppHost.

Also, the ordering of RunAsEmulator() is important (as it is in other child-resource hosting components like Azure Storage), e..g

        var resource = builder.AddAzureEventHubs("resource")
                              .AddEventHub("hubx")
                              .RunAsEmulator()
                              .WithHealthCheck("blocking_check");

This used to work because AddEventHub returned the parent AzureEventHubsResource. This is no longer the case. The RunAsEmulator() needs to move to directly after AddAzureEventHubs(...).

Fixes #5561

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:
    • [ ] No
Microsoft Reviewers: Open in CodeFlow

oising avatar Oct 03 '24 13:10 oising

@mitchdenny -- this fix seems to interfere with your logic on the emulator tests. Now that AddEventHub returns AzureEventHubResource, the RunAsEmulator call needs to occur before AddEventHub. You have a check in your RunAsEmulator to throw if there are no event hubs on emulator startup, which in turn affects health check logic too it seems.

oising avatar Oct 03 '24 13:10 oising

@mitchdenny @eerhardt @davidfowl

Do you want me to do anything with this? Do I wait until you've invented something similar for ESB topics and queue child resources, or is there a pattern I can follow now? I assume you're going to go along with the ApplicationModel design here too to normalize the codepaths for the emulator and the real thing?

oising avatar Nov 25 '24 19:11 oising

@sebastienros - any thoughts here? This seems the same as https://github.com/dotnet/aspire/pull/6737

eerhardt avatar Nov 25 '24 19:11 eerhardt

Fixed in 9.1

davidfowl avatar Jan 15 '25 08:01 davidfowl