Great proposal, this feature is well overdue!

1)
>From an operator's perspective I don't think the kafka client
implementation name and version are sufficient,
I also believe the application name and version are of interest.
You could have all applications in your cluster run the same kafka client
and version, but only one type or
version of an application misbehave and needing to be tracked down.

While the application and client name and version could be combined in the
ClientName/ClientVersion fields by
the user (e.g. like User-Agent), it would not be in a generalized format
and hard for generic monitoring tools to parse correctly.

So I'd suggest keeping ClientName and ClientVersion as the client
implementation name ("java" or "org.apache.kafka...") and version,
which can't be changed by the user/app developer, and providing two
optional fields for the application counterpart:
ApplicationName and ApplicationVersion, which are backed by corresponding
configuration properties (application.name, application.version).

2)
Do ..Name and ..Version need to be two separate fields, seeing how the two
fields are ambigious when separated?
If we're looking to identify unique versions, combining the two fields
would be sufficient (e.g., "java-2.3.1", "librdkafka/1.2.0", "sarama@1.2.3")
and perhaps easier to work with.
The actual format or content of the name-version string is irrelevant as
long as it identifies a unique name+version.


3)
As for allowed characters, will the broker fail the ApiVersionResponse if
any of these fields contain invalid characters,
or will the broker sanitize the strings?
For future backwards compatibility (when the broker constraints change but
clients are not updated) I suggest the latter.

4)
And while we're at it, can we add the broker name and version to the
ApiVersionResponse?
While an application must not use this information to detect features (Hi
Jay!), it is good for troubleshooting
and providing more meaningful logs to the client user in case a feature
(based on the actual api versions) is not available.

/Magnus


Den tors 22 aug. 2019 kl 10:09 skrev David Jacot <dja...@confluent.io>:

> Hi Satish,
>
> Thank you for your feedback!
>
> Please find my answers below.
>
> >> Did you consider taking version property by loading
> “kafka/kafka-version.properties” as a resource while java client is
> initialized?  “kafka/kafka-version.properties” is shipped with
> kafka-clients jar.
>
> I wasn't aware of the property file. It is exactly what I need. Thanks for
> pointing that out!
>
> >> I assume this metric value will be the total no of clients connected
> to a broker irrespective of whether name and version follow the
> expected pattern ([-.\w]+) or not.
>
> That is correct.
>
> >> It seems client name and client version are treated as tags for
> `ConnectedClients` metric. If so, you may implement this metric
> similar to `BrokerTopicMetrics` with topic tag as mentioned here[1].
> When is the metric removed for a specific client-name and
> client-version?
>
> That is correct. Client name and version are treated as tags like in
> BrokerTopicMetrics. My plan is to remove the metric when it goes
> back to zero - when all clients with a given name & version are
> disconnected.
>
> Best,
> David
>
> On Wed, Aug 21, 2019 at 6:52 PM Satish Duggana <satish.dugg...@gmail.com>
> wrote:
>
> > Hi David,
> > Thanks for the KIP. I have a couple of questions.
> >
> > >> For the Java client, the idea is to define two constants in the code
> to
> > store its name and its version. If possible, the version will be set
> > automatically based on metadata coming from gradle (or the repo itself)
> to
> > avoid having to do manual changes.
> >
> > Did you consider taking version property by loading
> > “kafka/kafka-version.properties” as a resource while java client is
> > initialized?  “kafka/kafka-version.properties” is shipped with
> > kafka-clients jar.
> >
> > >> kafka.server:type=ClientMetrics,name=ConnectedClients
> > I assume this metric value will be the total no of clients connected
> > to a broker irrespective of whether name and version follow the
> > expected pattern ([-.\w]+) or not.
> >
> > >>
> >
> kafka.server:type=ClientMetrics,name=ConnectedClients,clientname=([-.\w]+),clientversion=([-.\w]+)
> > It seems client name and client version are treated as tags for
> > `ConnectedClients` metric. If so, you may implement this metric
> > similar to `BrokerTopicMetrics` with topic tag as mentioned here[1].
> > When is the metric removed for a specific client-name and
> > client-version?
> >
> > 1.
> >
> https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/server/KafkaRequestHandler.scala#L231
> >
> > Thanks,
> > Satish.
> >
> >
> >
> >
> > On Wed, Aug 21, 2019 at 5:33 PM David Jacot <dja...@confluent.io> wrote:
> > >
> > > Hi all,
> > >
> > > I would like to start a discussion for KIP-511:
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-511%3A+Collect+and+Expose+Client%27s+Name+and+Version+in+the+Brokers
> > >
> > > Let me know what you think.
> > >
> > > Best,
> > > David
> >
>

Reply via email to