Produce deterministic "client.id" by default#5308
Open
Konstantin Demin (rockdrilla) wants to merge 5 commits intoconfluentinc:masterfrom
Open
Produce deterministic "client.id" by default#5308Konstantin Demin (rockdrilla) wants to merge 5 commits intoconfluentinc:masterfrom
Konstantin Demin (rockdrilla) wants to merge 5 commits intoconfluentinc:masterfrom
Conversation
This allows to mark functions to not be inlined even if compiler decides that inlining is possible. Signed-off-by: Konstantin Demin <[email protected]>
This change will raise minimum requirements for Windows builds due to InitOnceExecuteOnce() usage: Windows Vista / Windows Server 2008 are minimum supported systems. Signed-off-by: Konstantin Demin <[email protected]>
Also initialize buffer with zeros (with good intentions). Signed-off-by: Konstantin Demin <[email protected]>
Set "client.id" to "rdkafka@{hostname}" and fallback to "rdkafka" (previous default value) if there're problems with retrieving current host name.
Signed-off-by: Konstantin Demin <[email protected]>
Signed-off-by: Konstantin Demin <[email protected]>
|
🎉 All Contributor License Agreements have been signed. Ready to merge. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It's no secret that many applications don't set
client.idthemselves, making identification on the Kafka broker difficult.The problem becomes more acute when applications run in container orchestration environments (Kubernetes, Nomad, Docker, etc.) and connect to external Kafka clusters. In such cases, identifying the actual producer/consumer instance becomes nearly impossible without a meaningful
client.id.This MR addresses the issue by providing a deterministic default
client.idin the form"rdkafka@{hostname}", falling back to the previous default"rdkafka"if the hostname cannot be retrieved.A quick note on intent:
This MR isn't meant to call out or blame any specific software or project for not setting
client.id. Many legitimate use cases rely on defaults, and the goal here is simply to make those defaults more useful for operational visibility - especially in modern cloud-native environments. The change is proposed as a general improvement to librdkafka's out-of-the-box experience.Nota bene:
This MR consists of two logical parts:
Part 1: Non-breaking improvements (safe to merge)
Define RD_NOINLINE as opposite to RD_INLINEEnsure rd_kafka_transport_init() is called onceAdjust buffer size to avoid hostname truncation(applies to example application)Part 2: Potentially breaking change (requires discussion)
Produce deterministic "client.id" by defaultAdjust "client.id" in examples and testsRegarding
CHANGELOG.md: I believe these changes should be part of the next release (e.g., 2.14.0). If accepted, I can update the changelog with something like:InitOnceExecuteOnceclient.id(rdkafka@{hostname}) when none is providedThoughts? Feedback welcome!