NATS Aspire container by mtmk · Pull Request #1175 · microsoft/aspire

Ziya Suzen (mtmk) · 2023-12-02T06:07:21Z

This PR is an effort to introduce NATS container and client support.

Implemented most of the features as much as I could gather from other implementations. There are a few open tasks as far as I can see which may be left for another PR:

NATS server configuration is very basic at the moment. I'm not sure how the manifest is going to reflect all the configuration options. Currently only option is to enable JetStream for local development. Also no clustering option is implemented.
Tracing is still in our backlog Support distributed tracing / OpenTelemetry nats-io/nats.net#53
Same with metrics Add OTEL Metrics nats-io/nats.net#316

Oisin Grehan (oising) · 2023-12-08T19:53:00Z

Ziya Suzen (@mtmk) I'm rebasing this on latest main to fix merge conflicts and update any broken integration points. Also updating to use centralized package management /cc Reuben Bond (@ReubenBond)

Stefán Jökull Sigurðarson (stebet) · 2024-01-13T09:53:59Z

Oooh exciting! Does it also have cluster/JetStream support?

Ziya Suzen (mtmk) · 2024-01-14T20:27:44Z

Oooh exciting! Does it also have cluster/JetStream support?

Not yet 😅 but I think you can add the argument yourself for now if you like:

var nats = builder.AddNatsContainer("nats-server").WithArgs("-js");

Edit: also added options to AddNatsContainer which essentially does the above, even though I'm not sure if that's the correct approach. (Really need to look at others to see how they've done it)

var nats = builder.AddNatsContainer("nats-server", enableJetStream = true);

# Conflicts: # Aspire.sln

# Conflicts: # Directory.Packages.props

Ziya Suzen (mtmk) · 2024-02-12T19:33:34Z

Oisin Grehan (@oising) do you think we're ready for review?

Ziya Suzen (mtmk) · 2024-02-12T19:38:22Z

@azure-pipelinesazure-pipelines
/ dotnet.aspire
src/Aspire.Hosting.Nats/Aspire.Hosting.Nats.csproj#L0
src/Aspire.Hosting.Nats/Aspire.Hosting.Nats.csproj(0,0): error NU1102: (NETCORE_ENGINEERING_TELEMETRY=Restore) Unable to find package NATS.Net with version (>= 2.1.0)

Mitch Denny (@mitchdenny), i'm not sure why ci is failing NATS.Net v2.1.0 in up on NuGet https://www.nuget.org/packages/NATS.Net/2.1.0

David Fowler (davidfowl) · 2024-02-12T19:47:18Z

@@ -0,0 +1,17 @@
+using NATS.Client.Core;


Can we make this app a little more real? Can it use pubsub as well?

sure, let me have a go!

I created a simple JetStream example since it's easier demonstrate persisted messages using Web API. As for pubsub I suppose we'd need a frontend using SignalR or Blazor. Maybe a simple chat or something?

David Fowler (davidfowl) · 2024-03-03T22:27:04Z

+        {
+            await foreach (var msg in nats.SubscribeAsync<AppEvent>("events.>", cancellationToken: _cts.Token).ConfigureAwait(false))
+            {
+                Console.WriteLine($"Processing event: {msg.Data}");


David Fowler (davidfowl) · 2024-03-03T22:29:34Z

+
+        var manifest = await ManifestUtils.GetManifest(nats.Resource);
+
+        Assert.Equal("container.v0", manifest["type"]?.ToString());


Ziya Suzen (@mtmk) we made some changes here https://github.com/dotnet/aspire/blob/f6a2f9f0b5aca2553aeab6d09ec657cffc30d9fd/tests/Aspire.Hosting.Tests/Kafka/AddKafkaTests.cs#L75-L95.

BTW I'm sorry that these changes must feel like a roller coaster 😄. FWIW, it's helping us land on solid patterns that seem to scale well 🚀

no worries, my fault really having the PR open so long. thanks for bearing with me 🙏 also loving to witness the evolution 😍

If you are interested in more evolution:

https://github.com/dotnet/aspire/blob/525c59ad9d58de54c71071e248b060061dfecd56/src/Aspire.Hosting/Redis/RedisBuilderExtensions.cs#L26

https://github.com/dotnet/aspire/blob/525c59ad9d58de54c71071e248b060061dfecd56/src/Aspire.Hosting/Redis/RedisResource.cs#L16-L19

https://github.com/dotnet/aspire/blob/525c59ad9d58de54c71071e248b060061dfecd56/src/Aspire.Hosting/Redis/RedisResource.cs#L33

https://github.com/dotnet/aspire/blob/525c59ad9d58de54c71071e248b060061dfecd56/src/Aspire.Hosting/Redis/RedisResource.cs#L63

We're trying to get rid of all the manual string formatting and building up the object model to represent all kids of references. Makes it less error prone and brings consistency to the model.

Eric Erhardt (eerhardt) · 2024-03-04T21:24:00Z

+    <PackageReference Include="Microsoft.Extensions.Configuration.Binder" />
+    <PackageReference Include="Microsoft.Extensions.Diagnostics.HealthChecks" />
+    <PackageReference Include="Microsoft.Extensions.Hosting.Abstractions" />
+    <PackageReference Include="NATS.Net" />


Our naming rules are https://github.com/dotnet/aspire/tree/main/src/Components#naming.

The name of this component should be Aspire.NATS.Net to align with those rules.

Eric Erhardt (eerhardt) · 2024-03-04T21:24:32Z

@@ -0,0 +1,23 @@
+<Project Sdk="Microsoft.NET.Sdk">


This project and folder should be named Nats.AppHost.

Eric Erhardt (eerhardt)

Can you also update

Eric Erhardt (eerhardt) · 2024-03-04T21:26:23Z

+    protected override void SetHealthCheck(NatsClientSettings options, bool enabled)
+        => throw new NotImplementedException();


We have health checks in the component. We should enable these tests.

Eric Erhardt (eerhardt) · 2024-03-04T21:28:00Z

+
+namespace Aspire.Nats.Client;
+
+public class NatsHealthCheck(INatsConnection connection) : IHealthCheck


This shouldn't be public.

Eric Erhardt (eerhardt) · 2024-03-04T21:31:16Z

+        return Task.FromResult(connection.ConnectionState == NatsConnectionState.Open
+            ? HealthCheckResult.Healthy()
+            : HealthCheckResult.Unhealthy());
+    }


https://github.com/Xabaril/AspNetCore.Diagnostics.HealthChecks/blob/73abc7a094f9ef625b83455898a21d22d2fa6a82/src/HealthChecks.Nats/NatsHealthCheck.cs#L74-L83 has more complicated logic than this. Is this enough to be able to tell if the connection is healthy?

updated to be a bit more granular i.e. added degraded state. But description and stats only exist on the old client Nats.Client v1. One we're using here is v2 and it's a rewrite and most of that is not exposed yet.

Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>

Eric Erhardt (eerhardt) · 2024-03-05T12:56:44Z

+          "properties": {
+            "Net": {
+              "type": "object",
+              "properties": {


Are any of the underlying NatsOpts worth being able to be configured in the configuration? See some of the other components for an example on how we hook up the config binder.

Ex:

https://github.com/dotnet/aspire/blob/184b1297c324a310930c05aca37b2b66cbcc222e/src/Components/Aspire.StackExchange.Redis/AspireRedisExtensions.cs#L150-L155

https://github.com/dotnet/aspire/blob/184b1297c324a310930c05aca37b2b66cbcc222e/src/Components/Aspire.StackExchange.Redis/ConfigurationSchema.json#L21-L45

I shall look into this in another PR

# Conflicts: # Directory.Packages.props

Sébastien Ros (sebastienros) · 2024-03-07T01:35:01Z

Fixed conflicts, added manifest.json and renamed AddNats() to AddNatsClient to follow a recent change we did in other libraries to prevent conflict with the distributed app extension methods.

Tried the sample, worked for me, then using the Swagger I found an issue, it's not taking the custom json converter for TimeSpan properties in the config object, I believe it's an issue in SwaggerUI. (found domaindrivendev/Swashbuckle.AspNetCore#2500)

Ziya Suzen (@mtmk) if you are ok with my changes I'll merge it.

David Fowler (davidfowl) · 2024-03-07T07:32:55Z

Thank you Ziya Suzen (@mtmk) ! I look forward to future contributions!

NATS Aspire container

a4dcc11

ghost added the area-integrations Issues pertaining to Aspire Integrations packages label Dec 2, 2023

Dan Moseley (danmoseley) reviewed Dec 2, 2023

View reviewed changes

Comment thread src/Aspire.Hosting.Nats/Aspire.Hosting.Nats.csproj Outdated

Use central package management

135a213

Jose Perez Rodriguez (joperezr) added the community-contribution Indicates that the PR has been added by a community member label Dec 11, 2023

Merge branch 'main' into nats

2088f5b

Ziya Suzen (mtmk) mentioned this pull request Jan 8, 2024

Add OpenTelemetry Instrumentation nats-io/nats.net#124

Merged

5 tasks

Mitch Denny (mitchdenny) self-assigned this Jan 10, 2024

Ziya Suzen (mtmk) added 11 commits January 14, 2024 20:31

Add telemetry to defaults

7b9690f

Add JetStream option

732c8f1

Merge branch 'main' into nats

6c2d73c

# Conflicts: # Aspire.sln

Fixed build

56126f5

Merge branch 'main' into nats

35a8972

# Conflicts: # Directory.Packages.props

Fixed sln merge issue

35f22fe

Moved sample to playgrounds

7d94f67

sln fixes

27ec961

Updated playground nats

70987ad

Moved hosting bits

b5c2c37

Merge fix

28b2112

Fixed spelling

47d5268

David Fowler (davidfowl) reviewed Feb 12, 2024

View reviewed changes

Slightly more involved NATS sample

4d2ca61

David Fowler (davidfowl) reviewed Feb 12, 2024

View reviewed changes

Comment thread playground/nats/Nats.ApiService/Program.cs Outdated

Ziya Suzen (mtmk) added 2 commits February 12, 2024 21:19

NATS sample tweak

7916e6b

Rename to Aspire.Nats.Client and added config

27a8609

Ziya Suzen (mtmk) added 4 commits March 3, 2024 07:05

Merge remote-tracking branch 'upstream/main' into nats

a40a832

nats with image

1405623

nats image tweaks

8eb4d94

revert

4d488e2

David Fowler (davidfowl) reviewed Mar 3, 2024

View reviewed changes

Ziya Suzen (mtmk) added 2 commits March 4, 2024 08:46

Merge remote-tracking branch 'upstream/main' into nats

4afdad8

nats manifest test

1a62a75

Eric Erhardt (eerhardt) reviewed Mar 4, 2024

View reviewed changes

Ziya Suzen (mtmk) and others added 8 commits March 5, 2024 05:26

Update tests/Aspire.Nats.Client.Tests/ConfigurationTests.cs

3606984

Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>

rename nats AppHost

6eefa74

rename Aspire.NATS.Net

388bd85

nats health checks tweak

f31d49f

nats component test tweaks

2c10732

Merge remote-tracking branch 'upstream/main' into nats

f98cfa6

nats component docs

0eab699

nats resource string constants

1f773f9

Eric Erhardt (eerhardt) reviewed Mar 5, 2024

View reviewed changes

Sébastien Ros (sebastienros) added 3 commits March 6, 2024 17:03

Merge branch 'main' into nats

15ffa21

# Conflicts: # Directory.Packages.props

Add manifest

7836160

Rename AddNats to AddNatsClient

d005020

Sébastien Ros (sebastienros) approved these changes Mar 7, 2024

View reviewed changes

Sébastien Ros (sebastienros) merged commit 44cde5d into microsoft:main Mar 7, 2024

Ziya Suzen (mtmk) deleted the nats branch March 7, 2024 07:24

Eric Erhardt (eerhardt) mentioned this pull request Mar 11, 2024

[Components] New NATS component article dotnet/docs-aspire#483

Closed

github-actions Bot locked and limited conversation to collaborators Apr 28, 2024


		var manifest = await ManifestUtils.GetManifest(nats.Resource);

		Assert.Equal("container.v0", manifest["type"]?.ToString());

		protected override void SetHealthCheck(NatsClientSettings options, bool enabled)
		=> throw new NotImplementedException();


		namespace Aspire.Nats.Client;

		public class NatsHealthCheck(INatsConnection connection) : IHealthCheck

Uh oh!

Conversation

Ziya Suzen (mtmk) commented Dec 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Oisin Grehan (oising) commented Dec 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Stefán Jökull Sigurðarson (stebet) commented Jan 13, 2024

Uh oh!

Ziya Suzen (mtmk) commented Jan 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ziya Suzen (mtmk) commented Feb 12, 2024

Uh oh!

Ziya Suzen (mtmk) commented Feb 12, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Eric Erhardt (eerhardt) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sébastien Ros (sebastienros) commented Mar 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

David Fowler (davidfowl) commented Mar 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

Ziya Suzen (mtmk) commented Dec 2, 2023 •

edited

Loading

Oisin Grehan (oising) commented Dec 8, 2023 •

edited

Loading

Ziya Suzen (mtmk) commented Jan 14, 2024 •

edited

Loading

Sébastien Ros (sebastienros) commented Mar 7, 2024 •

edited

Loading