Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-3402: kafkaAdmin clusterId configuration is ignored if observability is enabled and bootstrap supplier is not set #3403

Closed
wants to merge 1 commit into from

Conversation

varmenise
Copy link
Contributor

fixes GH-3402 (#3402)

  • Re-set clusterId after creating new KafkaAdmin

@pivotal-cla
Copy link

@varmenise Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-cla
Copy link

@varmenise Thank you for signing the Contributor License Agreement!

… if observability is enabled and bootstrap supplier is not set

fixes spring-projectsGH-3402 (spring-projects#3402)

* Re-set clusterId after creating new KafkaAdmin
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

Any chances to prove the problem with some unit test, please ?

Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

I think I'm OK with this.
Although some unit test to cover would be nice.
Or modify existing one ObservationIntegrationTests.

Please, also run ./gradlew check locally to be sure in all the passing tests and Checkstyle rules.

Then add you name to the @author list of the affected classes.

* @since 3.3.0
*/
public String getClusterId() {
return clusterId;
Copy link
Member

Choose a reason for hiding this comment

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

Requires this.

@@ -213,6 +213,14 @@ public void setClusterId(String clusterId) {
this.clusterId = clusterId;
}

/**
* Get the clusterId property.
* @since 3.3.0
Copy link
Member

Choose a reason for hiding this comment

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

Since we have agreed that this is a bug and there is no easy workaround (but fix is simple), then this one is going to be back-ported to supported versions. Therefore this @since is 3.1.8

@varmenise
Copy link
Contributor Author

I ll fix this as I get some time, thanks for reviewing!

@artembilan
Copy link
Member

@varmenise ,
gentle reminder.
We have a couple days left before release on Monday.

Thanks

@sobychacko
Copy link
Contributor

Merged via: f11c7ca

Backporte to 3.2.x an 3.1.x.

@sobychacko sobychacko closed this Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KafkaAdmin clusterId configuration is ignored when observability is enabled and bootstrap supplier is not set
4 participants