Add client statistics.

[cezarfx]

Stats are collected on the client application, when system property -Dcom.oracle.nosql.sdk.nosqldriver.stats.profile=[regular|full] is used, for an interval of time, set by -Dcom.oracle.nosql.sdk.nosqldriver.stats.interval=600 in seconds.

The following stats are collected for each type of request: count, wire latencies, request and response sizes, errors, retries, retry delay, rate limit delay. Also if rateLimiting is enabled and concurrent connections number.

StatsConfig also provides an API to programmatically configure the statistics collection, to selectively start and stop collection and get access to collected statistics.

[gmfeinberg]

I still have some general comments:

1. The relationship among StatsConfig, StatsConfigImpl, Stats and how a user gets/uses/configures stats is confusing. Usually in our system a "Config" class is created essentially as a complex options object to construction of another class, in this case Stats. This is not the case here. StatsConfig is an interface, StatsConfigImpl is a "live" implementation in that changes to it at run time will (I think) affect the running stats. This is confusing and not typical. I would expect a StatsConfig object (not interface) that is used to create Stats and is not live after that. Ultimately we may want direct user control over starting/stopping/acquiring stats and how that might happen is not clear either. Maybe I'm missing something from the spec

2. I suggested separately to create a getName() interface on Request to avoid the calls like request.getClass().getSimpleName(). Is there a reason to not do this? It'd be simpler and more efficient. Minimally create a local method to do this (String getName(Request)). This approach would also make the mapping of request names more reliable than code like this: String requestClass = kvRequest.getClass().getSimpleName(); requestClass = requestClass.endsWith("Request") ? requestClass.substring(0, requestClass.length() - 7) : requestClass; rStat = requests.get(requestClass);

3. New files are missing Copyright headers

4. Stats.java has no comments at all

[connelly38]

1. The relationship among StatsConfig, StatsConfigImpl, Stats and how a user gets/uses/configures stats is confusing. Usually in our system a "Config" class is created essentially as a complex options object to construction of another class, in this case Stats. This is not the case here. StatsConfig is an interface, StatsConfigImpl is a "live" implementation in that changes to it at run time will (I think) affect the running stats. This is confusing and not typical. I would expect a StatsConfig object (not interface) that is used to create Stats and is not live after that. Ultimately we may want direct user control over starting/stopping/acquiring stats and how that might happen is not clear either. Maybe I'm missing something from the spec

After reading this again, I agree with George. StatsConfig should just be a class. No need for an Impl.

[gmfeinberg]

I'm also confused about what the extra query stats are storing. Can you talk about them? I didn't expect stats per statement that would include the statement and driver side query plan

[cezarfx]

I'm also confused about what the extra query stats are storing. Can you talk about them? I didn't expect stats per statement that would include the statement and driver side query plan

The query request stats are too generic when trying to find out what query is doing something significant. The extra query stats show the query stats on a per query statement base. This way certain query can be identified by checking its stats.

[cezarfx]

1. The relationship among StatsConfig, StatsConfigImpl, Stats and how a user gets/uses/configures stats is confusing. Usually in our system a "Config" class is created essentially as a complex options object to construction of another class, in this case Stats. This is not the case here. StatsConfig is an interface, StatsConfigImpl is a "live" implementation in that changes to it at run time will (I think) affect the running stats. This is confusing and not typical. I would expect a StatsConfig object (not interface) that is used to create Stats and is not live after that. Ultimately we may want direct user control over starting/stopping/acquiring stats and how that might happen is not clear either. Maybe I'm missing something from the spec

StatsConfig is the interface that allows users to enable/disable the collection of stats and set which stats to collect. It is live because users can start/stop or change collection at anytime, useful for debugging hard to reproduce or transitory issues.

2. I suggested separately to create a getName() interface on Request to avoid the calls like request.getClass().getSimpleName(). Is there a reason to not do this? It'd be simpler and more efficient. Minimally create a local method to do this (String getName(Request)). This approach would also make the mapping of request names more reliable than code like this: String requestClass = kvRequest.getClass().getSimpleName(); requestClass = requestClass.endsWith("Request") ? requestClass.substring(0, requestClass.length() - 7) : requestClass; rStat = requests.get(requestClass);

Adding a getName() would be another way to do it.

3. New files are missing Copyright headers Adding headers.

4. Stats.java has no comments at all Adding comments.

[gmfeinberg]

@cezarfx There are javadoc warnings in mvn. See "mvn javadoc:javadoc"