Skip to content

Conversation

@Mohammad-Junid
Copy link
Contributor

Enable Kafka Producer to sent separate topics to different Kafka Servers, IE: Alarms get to sent to one set of kafka servers, events to another set, metrics to another set.

@Mohammad-Junid Mohammad-Junid marked this pull request as ready for review November 26, 2025 17:38
Copy link
Contributor

@cgorantla cgorantla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are missing a critical piece here. We need this for metrics as well with KafkaPersisterFactory

Copy link
Contributor

@cgorantla cgorantla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to have a end-to-end test that actually sends messages to different Kafka clusters.

Copy link
Contributor

@cgorantla cgorantla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please take care of all review comments including adding a integration test.

Copy link
Contributor

@cgorantla cgorantla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this and this doesn't work. Removing initialization of producer in KafkaProducerManager#init helped.

Copy link
Contributor

@cgorantla cgorantla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested this. Seems to work fine.

Whenever we update config at runtime, we need to stop and start the feature. This needs to be incorporated into the doc.

@aurang-zeb313
Copy link
Member

I have tested it and working as expected

Copy link
Contributor

@cgorantla cgorantla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few comments to cleanup otherwise LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants