Add Kafka Component

[g7ed6e]

Relates to #884

[mitchdenny]

Overall, the app model side of this is looking pretty good. I think we need some end to end integration tests for this.

[mitchdenny]

Once health checks are implemented, please wire-up the Kafa component inside our smoke test (new):

1. Add container to app host here: https://github.com/dotnet/aspire/blob/60bd2b0b5c29ea02507265199070d4dac1fad712/tests/testproject/TestProject.AppHost/TestProgram.cs#L18
2. Add component to the integration test project: https://github.com/dotnet/aspire/blob/60bd2b0b5c29ea02507265199070d4dac1fad712/tests/testproject/TestProject.IntegrationServiceA/Program.cs#L5

[g7ed6e]

@mitchdenny Can you add Confluent.Kafka 2.3.0 and AspNetCore.HealthChecks.Kafka 7.0.0 ?

[davidfowl]

I think we want a single component for kafka, not a package for the consumer and produce. Also, the component should be Aspire.Confluent.Kafka since that's the name of the underlying client library we're using.

PS: People are out of vacation this week so it's slow 😄

[riverar]

Side comment: As additional components get developed/added, I wonder if we'll need an ingredients label so developers know which components (and which versions) do and do not have health checks, telemetry, etc.

[mitchdenny]

I wonder if we'll need an ingredients label so developers know which components (and which versions) do and do not have health checks, telemetry, etc.

We probably shouldn't be merging it if it doesn't have telemetry, health-checks etc. In terms of dependency graph, I think we get that for free from NuGet so I don't think we need to do anything extra there.

[mitchdenny]

@mitchdenny Can you add Confluent.Kafka 2.3.0 and AspNetCore.HealthChecks.Kafka 7.0.0 ?

They should be in dotnet-public shortly.

[g7ed6e]

/azp run

[azure-pipelines[bot]]

Commenter does not have sufficient privileges for PR 951 in repo dotnet/aspire

[eerhardt]

Also can you update

• https://github.com/dotnet/aspire/blob/main/src/Components/Telemetry.md
• https://github.com/dotnet/aspire/blob/main/src/Components/Aspire_Components_Progress.md

[g7ed6e]

@mitchdenny Can you add Confluent.Kafka 2.3.0 and AspNetCore.HealthChecks.Kafka 7.0.0 ?

They should be in dotnet-public shortly.

@mitchdenny Thank you. Confluent.Kafka is there but AspNetCore.HealthChecks.Kafka is not found.

[g7ed6e]

@eerhardt @davidfowl Activity is not implemented in Confluent.Kafka. I'm thinking to wrap IProducer and IConsumer in a decorator to intercept call and start appropriate Activity. However i'm concerned about maintainability. Is it worth it ?

edit: we may also think to a source gen + source interceptor same concern about maintainability. edit2: MessageMetadata base class of of Message<TKey, TValueexposes a Headers collection.

[forlack]

I am the PM owner over our Kafka Clients at Confluent. Let me know if you need any help or collaboration from some of our engineers. Happy to answer any questions we can. Thanks!

[g7ed6e]

I am the PM owner over our Kafka Clients at Confluent. Let me know if you need any help or collaboration from some of our engineers. Happy to answer any questions we can. Thanks!

Thank you @forlack for reaching that PR. The missing piece would be to get OTel Distributed tracing support in Confluent.Kafka. My thinking is that it could be implemented from both the inside and the outside of Confluent.Kafka (however inside would be easier) but having builtin support for this in Confluent.Kafka would benefit to more people and libraries. I'm linking here the related Confluent.Kafka issue.

[mitchdenny]

@mitchdenny Thank you. Confluent.Kafka is there but AspNetCore.HealthChecks.Kafka is not found.

Sorry about that @g7ed6e , I made a mistake with that import. It is there now.

[lachmatt]

hi @g7ed6e, would it be possible to extract e.g. metrics into instrumentation library, which could then be referenced from here (similarly to how StackExchange.Redis component references instrumentation library)? I know this requires additional effort, but I think it would benefit OTel community. Have you considered that approach?

[g7ed6e]

hi @g7ed6e, would it be possible to extract e.g. metrics into instrumentation library, which could then be referenced from here (similarly to how StackExchange.Redis component references instrumentation library)?

I know this requires additional effort, but I think it would benefit OTel community.

Have you considered that approach?

Hi @lachmatt I agree it looks like the best long term approach. I will open an issue and a PR there.

edit: otel support to be provided by https://github.com/open-telemetry/opentelemetry-dotnet-contrib/issues/1485

[g7ed6e]

Thanks, @g7ed6e for this contribution. It is really starting to take shape!

Do you plan on getting open-telemetry/opentelemetry-dotnet-contrib#1493 done before this PR can be merged? Or do you want to put the code in now, and when the OTel change is done, take the code out of here and use the official OTel support?

Hi @eerhardt would you be open to have this PR merged with healthcheck support only ? (It requires a bit of code cleanup to drop metrics introduced here so please let me know) and then, once otel package is ready, i can open a follow up PR here to integrate traces and metrics support. The main benefit I see is to avoid conflicts and/or breaking changes between the two implementations. The otel implementation will be driven by otel maintainers and I cannot guarantee there won't be breaking changes.

[lachmatt]

I am the PM owner over our Kafka Clients at Confluent. Let me know if you need any help or collaboration from some of our engineers. Happy to answer any questions we can. Thanks!

hi @forlack, is there any chance Confluent`s Kafka .NET Client could be instrumented using e.g ActivitySource API to provide tracing support? It was discussed in the past in https://github.com/confluentinc/confluent-kafka-dotnet/issues/1269

[eerhardt]

Hi @eerhardt would you be open to have this PR merged with healthcheck support only ? (It requires a bit of code cleanup to drop metrics introduced here so please let me know) and then, once otel package is ready, i can open a follow up PR here to integrate traces and metrics support. The main benefit I see is to avoid conflicts and/or breaking changes between the two implementations. The otel implementation will be driven by otel maintainers and I cannot guarantee there won't be breaking changes.

I think it would be OK to merge the PR with the current Metrics implementation as it is. It would be a nice way to get people using it and get feedback. .NET Aspire is still in preview, and should be for a few months, so making breaking telemetry changes is acceptable.

Note that we can keep the Kafka Aspire component in "preview" / "unstable" until we get the otel changes in and consumed here.

cc @davidfowl @DamianEdwards - are you OK with that plan?

[DamianEdwards]

Note that we can keep the Kafka Aspire component in "preview" / "unstable" until we get the otel changes in and consumed here.

Sounds good.

[g7ed6e]

Hi @mitchdenny may I ask you to make AspNetCore.HealthChecks.Kafka 8.0.0 available to the nuget feeds please ?

@eerhardt The PR is rebased and commits are squashed. The CI should pass once the above package is in one of the dotnet nuget feed.

[mitchdenny]

@g7ed6e importing now.

[g7ed6e]

@g7ed6e importing now.

Thanks @mitchdenny CI now completes successfully

[eerhardt]

I pushed 2 commits to update the ConfigurationSchema.json file to be more complete (sorted and using the generator from #1383). I put them in 2 commits for easy reviewing - first just sorts the existing properties. The second can be diff'd by itself to see what the generator changed.

Unfortunately I hit a bug with the ConfigurationBinder source generator - https://github.com/dotnet/runtime/issues/96652. We aren't able to automate updating the ConfigurationSchema.json file automatically until we have a fix for that issue.

[mitchdenny]

I think this is really close. A question or two from my perspective, but from an app model point of view this looks good.

[g7ed6e]

@eerhardt I just squashed end rebased until the Orleans support adding.

[davidfowl]

Thanks so much @g7ed6e!