Upgrade to .NET Aspire 9.1

[eerhardt]

• Update all Aspire package versions, including the latest .NET and ASP.NET Core versions.
• Remove LangVersion preview to work around Array.Contains no longer works in latest .NET 9 SDK (9.0.200) npgsql/efcore.pg#3461
• Update to latest MEAI
• Add support for using an existing OpenAI account via AsExisting

[eerhardt]

This is working around Array.Contains no longer works in latest .NET 9 SDK (9.0.200) (npgsql/efcore.pg#3461)

[stephentoub]

cc: @roji, @jaredpar

[Copilot]

Copilot reviewed 5 out of 12 changed files in this pull request and generated no comments.

Files not reviewed (7)

• Directory.Build.props: Language not supported
• Directory.Packages.props: Language not supported
• global.json: Language not supported
• nuget.config: Language not supported
• src/eShop.AppHost/eShop.AppHost.csproj: Language not supported
• tests/Catalog.FunctionalTests/Catalog.FunctionalTests.csproj: Language not supported
• tests/Ordering.FunctionalTests/Ordering.FunctionalTests.csproj: Language not supported

Comments suppressed due to low confidence (1)

src/WebApp/Components/Chatbot/ChatState.cs:78

• Ensure that the new method GetResponseAsync is covered by tests to verify its behavior.

var response = await _chatClient.GetResponseAsync(Messages, _chatOptions);

[eerhardt]

FYI @stephentoub @SteveSandersonMS on the change here and in WebApp/Extensions/Extensions.cs.

These 2 things were pretty tricky.

1. I was getting infinite recursion on Ollama since builder.Services.AddEmbeddingGenerator is adding the IEmbeddingGenerator<string, Embedding<float>> service, and this was trying to get the same service. So it recurses forever.
2. In OpenAI code below, the call to Build() was failing because it was passing in an empty IServiceProvider to line 51, and that factory was trying to get the OpenAIClient out of the sp, which was empty.

[stephentoub]

I didn't quite follow that. But is there something you'd recommend we change to make something easier?

[eerhardt]

But is there something you'd recommend we change to make something easier?

For the 2nd problem (the call to Build()), there is some awkwardness IMO. Take a look at the existing code:

eShop/src/Catalog.API/Extensions/Extensions.cs

Lines 50 to 53 in 1ba1cc7

	builder.Services.AddEmbeddingGenerator(sp => sp.GetRequiredService<OpenAIClient>().AsEmbeddingGenerator(builder.Configuration["AI:OpenAI:EmbeddingModel"]!))
	.UseOpenTelemetry()
	.UseLogging()
	.Build();

Code like this will never work.

The call to Build() doesn't pass an IServiceProvider. Thus the sp that is passed to the callback on line 50 will be empty, and won't contain any services. This is because the Build() method looks like this:

    public IEmbeddingGenerator<TInput, TEmbedding> Build(IServiceProvider? services = null)
    {
        services ??= EmptyServiceProvider.Instance;

Maybe Build should have a non-optional IServiceProvider parameter? There is a constructor that takes a Func<IServiceProvider, IEmbeddingGenerator<TInput, TEmbedding>> innerGeneratorFactory which won't work without a valid IServiceProvider being passed. Requiring the IServiceProvider would indicate to the caller that you shouldn't call Build() when adding services to your application, since you don't have an IServiceProvider yet.

For the infinite recursion problem, I'm not sure I have a suggestion. I think this was just confusion on my part trying to understand the old vs the new API usage. When writing new code, I don't think anyone would would implement the "innerGeneratorFactory" by directly returning the same service from the IServiceProvider . This is the function that is supposed to create the generator.

[stephentoub]

To make sure I understand, you're saying that you not being forced to think about passing a serviceProvider to Build (even if you decide not to pass one) is the problem?

[eerhardt]

I'm saying I shouldn't ever call Build() in this code. And if I was forced to pass an IServiceProvider explicitly, it would be obvious that I shouldn't call it here.

The way the existing code currently looks, I would have expected the IServiceProvider from builder.Services (where this code started) to be used. But it isn't used, instead an empty one is passed to the callback on line 50.

[eiriktsarpalis]

The call to Build() in this location is redundant, since it simply returns an IChatClient instance that is being discarded. If I'm honest I find the pattern used by the AddChatClient/AddEmbeddingGenerator methods somewhat strange and I can see why it could lead to confusion. Instead of doing

builder.Services.AddChatClient(sp => new Implementation())
     .UseOpenTelemetry()
     .UseLogging();

Why not just have the method return void and encourage the equivalent pattern:

builder.Services.AddChatClient(sp => new Implementation()
     .AsBuilder()
     .UseOpenTelemetry() 
     .UseLogging()
     .Build());

It might be more lines of code but the intent is clearer.

[eerhardt]

Once Return AspireOpenAIClientBuilder from AddOpenAIClientFromConfiguration (dotnet/aspire#7763) is merged, this code will get even more simple. (At least the OpenAI part will. We will need to update the Ollama Aspire integration to follow the same API pattern.)

builder.AddOpenAIClientFromConfiguration("openai");
builder.Services.AddEmbeddingGenerator(sp => sp.GetRequiredService<OpenAIClient>().AsEmbeddingGenerator(builder.Configuration["AI:OpenAI:EmbeddingModel"]!))
    .UseOpenTelemetry()
    .UseLogging();

becomes

builder.AddOpenAIClientFromConfiguration("openai")
    .AddEmbeddingGenerator(builder.Configuration["AI:OpenAI:EmbeddingModel"]!)
    .UseLogging();  // I'm not positive this is required, but I don't see Aspire calling it today

And with further changes like Configure OpenAI models in the app host (dotnet/aspire#6577) we will be able to drop the call to builder.Configuration, and it will simply be:

builder.AddOpenAIClientFromConfiguration("openai-embedding")
    .AddEmbeddingGenerator()
    .UseLogging();  // I'm not positive this is required, but I don't see Aspire calling it today

Since the model/deployment name will be passed via the connection string named "openai-embedding".

[stephentoub]

.UseLogging(); // I'm not positive this is required, but I don't see Aspire calling it today

Assuming UseOpenTelemetry is being used, UseLogging is probably not necessary.

[eerhardt]

Assuming UseOpenTelemetry is being used

Yes, it is by default. The Aspire integration enables OpenTelemetry unless someone disables tracing:

https://github.com/dotnet/aspire/blob/3d3ccd4eb8b3f300a91b54f763cbad4ade71ad24/src/Components/Aspire.OpenAI/AspireOpenAIClientBuilderEmbeddingGeneratorExtensions.cs#L59-L61

        return builder.DisableTracing
            ? result
            : new OpenTelemetryEmbeddingGenerator<string, Embedding<float>>(result);

but diffing this code with what's in UseOpenTelemetry(), maybe we need to update it to pass an ILogger in:

https://github.com/dotnet/extensions/blob/576fa221f57f80423a4852e446cee2aa2e937aab/src/Libraries/Microsoft.Extensions.AI/Embeddings/OpenTelemetryEmbeddingGeneratorBuilderExtensions.cs#L36-L43

            loggerFactory ??= services.GetService<ILoggerFactory>();


            var generator = new OpenTelemetryEmbeddingGenerator<TInput, TEmbedding>(
                innerGenerator,
                loggerFactory?.CreateLogger(typeof(OpenTelemetryEmbeddingGenerator<TInput, TEmbedding>)),
                sourceName);
            configure?.Invoke(generator);
            return generator;

I've opened Update OpenAI OpenTelemetry integration to pass an ILogger (dotnet/aspire#7771) for this.

[eerhardt]

We will need to update the Ollama Aspire integration to follow the same API pattern.

This was done in CommunityToolkit/Aspire#465. We can update here once it ships in a stable version of the Aspire Ollama integration.

cc @aaronpowell

[eerhardt]

This isn't necessary, but it felt good to use the new feature. This nests the redis resource under the basketApi resource in the dashboard (since it is the only one that uses it).

[captainsafia]

LGTM!

[stephentoub]

Thanks!