> I think we should force client software names and versions to follow a regular expression and disconnect if they do not.
Disconnecting is not really a great error propagation method since it leaves the client oblivious to what went wrong. Instead suggest we return an ApiVersionResponse with an error code and a human-readable error message field. Den ons 18 sep. 2019 kl 20:05 skrev Colin McCabe <cmcc...@apache.org>: > Hi David, > > Thanks for the KIP! > > Nitpick: in the intro paragraph, "Operators of Apache Kafka clusters have > literally no information about the clients connected to their clusters" > seems a bit too strong. We have some information, right? For example, the > client ID, where clients are connecting from, etc. Maybe it would be > clearer to say "very little information about the type of client > software..." > > Instead of ClientName and ClientVersion, how about ClientSoftwareName and > ClientSoftwareVersion? This might make it clearer what the fields are > for. I can see people getting confused about the difference between > ClientName and ClientId, which sound pretty similar. Adding "Software" to > the field name makes it much clearer what the difference is between these > fields. > > In the "ApiVersions Request/Response Handling" section, it seems like > there is some out-of-date text. Specifically, it says "we propose to add > the supported version of the ApiVersionsRequest in the response sent back > to the client alongside the error...". But on the other hand, elsewhere in > the KIP, we say "ApiVersionsResponse is bumped to version 3 but does not > have any changes in the schema" Based on the offline discussion we had, > the correct text is the latter (we're not changing ApiVersionsRerponse). > So we should remove the text about adding stuff to ApiVersionsResponse. > > In a similar vein, the comment " // Version 3 is similar to version 2" > should be " // Version 3 is identical to version 2" or something like > that. Although I guess technically things which are identical are also > similar, the current phrasing could be misleading. > > Now that KIP-482 has been accepted, I think there are a few things that > are worth clarifying in the KIP. Firstly, ApiVersionsRequest v3 should be > a "flexible version". Mainly, that means its request header will support > optional tagged fields. However, ApiVersionsResponse v3 will *not* support > optional tagged fields in its response header. This is necessary because-- > as you said-- the broker must look at a fixed offset to find the error > code, regardless of the response version. > > I think we should force client software names and versions to follow a > regular expression and disconnect if they do not. This will prevent issues > when using these strings in metrics names. Probably we want something like: > > [\.\-A-Za-z0-9]?[\.\-A-Za-z0-9 ]*[\.\-A-Za-z0-9]? > > Notice this does _not* include underscores, since they get converted to > dots in JMX, causing ambiguity. It also doesn't allow the first or last > character to be a space. > > best, > Colin > > > On Mon, Sep 16, 2019, at 04:39, Mickael Maison wrote: > > +1 (non binding) > > Thanks for the KIP! > > > > On Mon, Sep 16, 2019 at 12:07 PM David Jacot <dja...@confluent.io> > wrote: > > > > > > Hi all, > > > > > > I would like to start a vote on KIP-511: > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-511%3A+Collect+and+Expose+Client%27s+Name+and+Version+in+the+Brokers > > > > > > Best, > > > David > > >
