Add support for Azure Event Hubs

[oising]

Initial support for Azure Event Hubs, as per #1147

• Minimally adapt the Service Bus component and resource for Event Hubs
• Add top-level support for EventHubProducerClient
• Add top-level support for EventHubConsumerClient
• Add top-level support for EventProcessorClient
  • Allow providing component name for a BlobContainerClient to use (read connection string and instantiate local instance)
  • Read BlobContainerClient from keyed DI using its Aspire connection name
• Add top-level support for PartitionReceiver
• Add playground sample AppHost and projects (modify davidfowl's test project)
• Add ability to configure credential used in Azure Provisioner (via AzureProvisionCredential in AppHost configuration)
• Add xmldoc documentation for components etc
• Update README

Optional extras (may defer to a second PR)

• Add support for chained AddHub(...) calls for top-level EH resource to allow variable config of hubs
  • Implement as child resource to allow deep-linking (i.e. provide connection string including EntityPath)
• Implement Microsoft.Extensions.Azure style top-level component AddEventHubs(...) and hang clients configuration from clientBuilder
• Add Streaming sample with Orleans

Aspire API shortcomings

EventProcessorClient requires a BlobContainerClient for managing EH checkpoints. Ideally we should be able to pass a connection name to a mounted aspire blob client component and retrieve the instance from the service provider, but there is no plumbing to get it in the component infra. The unbuilt service collection is available on the distributed builder and it could be fished out of there, assuming it was added before the current component, but this seems flakey. Instead, I'm creating a new blob client using the blob connection name (taken from user-supplied settings) and creating my own.

Microsoft Reviewers: Open in CodeFlow

[davidfowl]

The azure provisioning stuff here is outdated so I would just delete all of it since it isn't required.

[davidfowl]

This entire PR seems based on a really old version of aspire. Everything is bicep based now.

[oising]

This entire PR seems based on a really old version of aspire. Everything is bicep based now.

I think your definition of "really old" doesn't match with mine :) I'm working off of main, and I cloned barely a week ago? I'll update it.

[davidfowl]

I'm not sure what happened in this PR, but if you look at main the Aspire.Hosting.Azure.Provisioning package has been deleted... 😄

#2603

Somehow this brought it back.

[oising]

I'm not sure what happened in this PR, but if you look at main the Aspire.Hosting.Azure.Provisioning package has been deleted... 😄

#2603

Somehow this brought it back.

I'll start from scratch -- tbh, this first pass was just about understanding how components and resources work (in this week's build lol)

[oising]

Restarting for the third time, because of #2875

Done.

[davidfowl]

@sebastienros @eerhardt for the component review
@mitchdenny @JoshLove-msft for the resource review. CDK codegen is also required here.

[davidfowl]

Also @jsquire for azure sdk review

[oising]

I rebased to stay up to date and now I'm hitting:

Application orchestrator dependency check had an unexpected error System.TimeoutException: The operation has timed out.
at Aspire.Hosting.Dcp.DcpDependencyCheck.EnsureDcpDependenciesAsync(CancellationToken cancellationToken) in C:\p\github\aspire\src\Aspire.Hosting\Dcp\DcpDependencyCheck.cs:line 67

on start up.

[davidfowl]

Application orchestrator dependency check had an unexpected error System.TimeoutException: The operation has timed out.
at Aspire.Hosting.Dcp.DcpDependencyCheck.EnsureDcpDependenciesAsync(CancellationToken cancellationToken) in C:\p\github\aspire\src\Aspire.Hosting\Dcp\DcpDependencyCheck.cs:line 67

Consistently?

[oising]

Application orchestrator dependency check had an unexpected error System.TimeoutException: The operation has timed out.
at Aspire.Hosting.Dcp.DcpDependencyCheck.EnsureDcpDependenciesAsync(CancellationToken cancellationToken) in C:\p\github\aspire\src\Aspire.Hosting\Dcp\DcpDependencyCheck.cs:line 67

Consistently?

Yes. I tried bumping the timeout to 5 mins and at some point something else timed out and brought the whole process down. I tried disabling the timeout (setting to 0) and same thing. Tried about a half dozen times.

edit: resolved, see #3003

[eerhardt]

FYI @oising, I pushed some changes you should be aware of. The most important is that I fixed Tracing to work. Just wanted to let you know so you don't force push and revert those changes.

I think this is close to getting the initial merge. My most major concerns are:

• Getting the Storage Blob client. I really think this should come from DI
• The Settings class having properties that don't work depending on the client being used.
• Full testing

If we want to merge this soon to guarantee it gets into preview5, and log issues for these to be followed up, I would be OK with that. FYI @davidfowl

[oising]

FYI @oising, I pushed some changes you should be aware of. The most important is that I fixed Tracing to work. Just wanted to let you know so you don't force push and revert those changes.

I think this is close to getting the initial merge. My most major concerns are:

• Getting the Storage Blob client. I really think this should come from DI
• The Settings class having properties that don't work depending on the client being used.
• Full testing

If we want to merge this soon to guarantee it gets into preview5, and log issues for these to be followed up, I would be OK with that. FYI @davidfowl

Thanks @eerhardt -- I really appreciate the thorough review and insightful suggestions :) I'm about to check in the suggested split settings approach as discussed above. It took a bit of jiggery-pokery with generics to fix a few things, but it seems to be good now.

As for the storage blob client, yes, I agree this is a bit of a rough edge; especially around the lack of unit tests here. As mentioned, the service provider isn't built at this point, but I could build it and look for a blob client within. I think this needs some configuration-by-convention love in the sense that we should probably only permit a keyed blob client so it can be configured in settings. Also, to test the logic around the container creation, I might want to lightly shim the blob client so I can unit test the behaviour without requiring a live connection. I'll spike this tomorrow.

[eerhardt]

As mentioned, the service provider isn't built at this point, but I could build it and look for a blob client within.

This isn't necessary. We should just be able to call a different overload on the AzureClientFactoryBuilder, which gives us the IServiceProvider. See https://github.com/Azure/azure-sdk-for-net/blob/87d426597b261b88f8a6c6c21fab6031f857fb70/sdk/extensions/Microsoft.Extensions.Azure/src/AzureClientFactoryBuilder.cs#L164-L172. (Note it would take some refactoring of the base AzureComponent<TSettings, TClient, TClientOptions> class, as right now the abstract AddClient method takes a TBuilder, when it could just be passed an AzureClientFactoryBuilder.

I think this needs some configuration-by-convention love in the sense that we should probably only permit a keyed blob client so it can be configured in settings.

I see it a different way. It should allow for a keyed blob client, but shouldn't require it to be keyed. If no "blob service key" was configured, it should just get the BlobServiceClient from DI. If a "blob service key" was specified, then use it to get the BlobServiceClient from DI.

That way the simple case "just works". You just need to:

var builder = WebApplication.CreateBuilder(args);

builder.AddAzureBlobClient("blobConnectionName");
builder.AddAzureEventProcessorClient("eventhubns");

and the EventProcessorClient grabs the BlobClient. There's no need to force someone to use keys when there is only one.

[eerhardt]

Thank you, @oising, for this contribution! Great work!

This LGTM to merge for preview5.

Can you log follow up issues for the remaining work? Notably:

• Using DI to get the blob client in the EventProcessor
• Testing
• (I might have missed others. Please ensure it is all logged.)

[davidfowl]

Thank you @oising !!!!!