[ 
https://issues.apache.org/jira/browse/CASSANDRA-14275?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16386419#comment-16386419
 ] 

Aleksey Yeschenko commented on CASSANDRA-14275:
-----------------------------------------------

Hey o.

It's an ok start, but we can make it better:

1. {{ClientState()}} constructor doesn't need {{this.driverName = null;}} or 
{{this.driverVersion = null;}} assignments. You were probably just mimicking 
the other assignments in it, but you shouldn't have. They are there because 
{{isInternal}} and {{remoteAddress}} fields are {{final}}. {{driverName}} and 
{{driverVersion}} aren't, and will be set to null anyway.
2. Likewise, in {{ClientState(InetSocketAddress remoteAddress)}} constructor, 
{{this.driverName = null;}} assignment is not needed.
3. {{ClientState forExternalCalls(SocketAddress remoteAddress, String 
driverName, String driverVersion)}} method is not used by anything, and neither 
do we need {{ClientState(InetSocketAddress remoteAddress, String driverName, 
String driverVersion)}} constructor. You might be tempted to create them just 
because we might need them one day, but we try to generally avoid that. So I'd 
get rid of them both.
4. {{setDriverInfo(String driverName, String driverVersion)}} should probably 
be split into two methods, one for driver name and another for driver version. 
See below for reason why.
5. You shouldn't put the {{STARTUP}} handling code into {{Message}} class. What 
you did belongs to {{StartupMessage.execute()}} method.
6. {{"DRIVER_NAME"}} and {{"DRIVER_VERSION"}} should be constantized once in 
{{StartupMessage}}, like the rest of the options.
7. It should be possible to only set the driver name, but we shouldn't set 
driver version if name is not set.
8. I would prefer if you made {{getDriverVersion()}} and {{getDriverName()}} 
methods in {{ClientState}} return an {{Optional<String>}} on read instead of 
using empty strings on write.

This is all for now, though I might have more in the second round. If you don't 
mind, can you squash all the existing comments into one, and address review 
feedback in separate commits on top of the squashed one?

Cheers (:

> Cassandra Driver should send identification information to Server
> -----------------------------------------------------------------
>
>                 Key: CASSANDRA-14275
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-14275
>             Project: Cassandra
>          Issue Type: New Feature
>          Components: Core
>            Reporter: Dinesh Joshi
>            Assignee: Dinesh Joshi
>            Priority: Major
>             Fix For: 4.x
>
>
> Currently there doesn't seem to be any way to readily identify the driver 
> that clients are using to connect to Cassandra. Add the capability of 
> identifying the driver through metadata information much like how HTTP 
> Clients identify themselves through User-Agent HTTP header. This is useful 
> for debugging in large deployments where clients tend to use different 
> drivers, wrappers and language bindings to connect to Cassandra. This can 
> help surface issues as well as help detect clients which are using older or 
> unsupported drivers.
> The identification information should be a string that unambiguously 
> identifies the driver. It should include information such as the name of the 
> driver, it's version, CQL version, Platform (Linux, macOS, Windows, etc.) and 
> architecture (x86, x86_64).
> We should surface this information in `nodetool clientstats` command.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to