added cosmosdb sample

[bradygaster]

Adds Cosmos DB sample per #74

[DamianEdwards]

@bradygaster you'll need to figure out how to make this sample pass the test cleanly as currently the apiservice fails to start it seems.

[bradygaster]

Pretty sure I captured all the changes suggested by @DamianEdwards and @Pilchie save 1 - i couldn't get the app to work properly when i have RunAsEmulator in the App Host code. If @Pilchie has guidance on how to work past the error i've sent him i'll implement it, otherwise i think this has everything everyone requested and is ready to go.

[DamianEdwards]

I still see some build warnings and the test still isn't passing?

[bradygaster]

added the RunAsEmulator command as it matches the implementation in the playground project and the comment @DamianEdwards requested, in lieu of the fact that i couldn't get it working with the emulator.

[bradygaster]

Guess the emulator is broken on my machine. For once, it not working on my machine isn't a bad thing? :)

[DamianEdwards]

Only thing left would be to add a test case at https://github.com/dotnet/aspire-samples/blob/main/tests/SamplesIntegrationTests/AppHostTests.cs#L125 so that we get proper coverage of this new sample.

[bradygaster]

Made a few small tweaks - otel refs and removal of the gRPC-related items in defaults.

[bradygaster]

cc @Pilchie so he sees how @DamianEdwards was able to work through this - I pinged the team today so mention we'd run into these issues, so wanted to give y'all an update on the completion. Thanks @DamianEdwards!

[davidfowl]

We should put this code in the component.

cc @eerhardt

[eerhardt]

1. @Pilchie mentioned offline that they were going to change the emulator to not respond until it was ready. That would solve the issues with getting SSL errors during startup.
2. We could add this to the CosmosClientOptions.HttpFactory by default. It would be better if the library had resilience built into it though.

[DamianEdwards]

I think if we set the HttpClientFactory option we then own all other aspects setting up the HttpClient instance that the Cosmos client currently handles, including those affected by properties on the connecting string, e.g. disabling the SSL validation check. This is what I ran into when attempting that approach.

[bradygaster]

Do we want to merge this as-is or edit the code in the component in such a manner we can make the sample less-meh? I vote for merging it and then I'll update it once we change the component if @eerhardt informs me once the work is done. :)

[eerhardt]

I'd agree we should wait for the component changes.

FYI - the component is owned by @Pilchie's team. We should work with them to get it updated as necessary.

[Pilchie]

Just wanted to comment that I'm aware of this and following up. I think there is a plan to build some of this into the container itself so that this sort of thing isn't necessary, but I will let you know. cc @kirankumarkolli

[DamianEdwards]

@bradygaster @Pilchie I pushed some changes that hopefully make this more reliable:

• Added retries to the DatabaseBootstrapper using a Polly resiliency pipeline with enough retries and time to allow the emulator container to start (hopefully)
• Changed the DatabaseBootstrapper from a raw IHostedService to a BackgroundService so its startup doesn't impact the lifecycle of the app. As it was, Kestrel couldn't start until the DatabaseBootstrapper had successfully completed, and if it failed the app would just crash.
• Made DatabaseBootstrapper double as an IHealthCheck so it can report its status and the test can ensure /health succeeds before moving on to trying to hit /todos

It seems problematic that the Cosmos DB component doesn't enable enough retries by default to allow the emulator to startup. Also I found it confusing that the Cosmos DB resource reports itself as having only a TCP endpoint but then it clearly uses HTTP at runtime, but because it doesn't use IHttpClientFactory it doesn't get any of the default Aspire HTTP resiliency applied. I attempted to wire the Cosmos client up to IHttpClientFactory but found it more complex than simply wrapping retries around the creation logic due to having to separate it from the default resiliency (as that wasn't resilient enough) and having to manually handle the HTTPS cert again.

[eerhardt]

Suggested change

	<PackageReference Include="Azure.Identity" Version="1.12.0" />
	<PackageReference Include="Microsoft.Azure.Cosmos" Version="3.41.0" />

Why are these necessary? You should be able to just have line 10.

[DamianEdwards]

Vulnerability in Azure.Identity in versions lower than that and we haven't updated the component yet. I just followed the bouncing ball in the NuGet Package Manager UI in VS.

[DamianEdwards]

@bradygaster @eerhardt We shouldn't have samples with vulnerable packages, even transitively. This is what I see in VS for the transitive reference to Azure.Identity

Customers need to update transitive references to resolve these issues and so IMO it's appropriate to do so in samples too. When we ship updated versions of the Aspire packages that update the transitive versions used we can remove the explicit references.

[eerhardt]

This is odd to have in our sample, when the decision was to not have a health check in the component.

dotnet/aspire#359 (comment)

[DamianEdwards]

Right but this isn't a health check of Cosmos, it's a health check of whether the bootstrapper has completed successfully. Once the bootstrapper has completed this health check is always healthy and doesn't hit Cosmos on its own.