Expose metrics for the Redis client

[draxly]

Hello!

We are using the Redis client for some mission critical operations and would really like to monitor it like we do with other Vertx components.

The metrics would include the metrics of the command queue (i.e the commands that have been sent but not replied), response times in the 95th and 99th percentile for requests/responses etc.

Please refer to the discussion at https://groups.google.com/forum/#!topic/vertx/srYI30_-IPU for the discussion leading up to this wish list item.

Kind regards, Tim

[dratler]

this is still relevant ?

[pmlopes]

Hi yes. Would you like to contribute? We need to look at what metrics we would like to have and add the collector on the right place.

[dratler]

Hi @pmlopes , which git repo is using this one : ?

• https://github.com/vert-x3/vertx-stack
• https://github.com/vert-x3/vertx-micrometer-metrics
• https://github.com/vert-x3/vertx-dropwizard-metrics

or other ?

[vietj]

for doing this, we should find how metrics can be used, i.e currently Vert.x core provides an SPI for metrics.

We need to see how this integrates with this.

So I think we need to

1. define what are the metrics we want to expose
2. verify how Vert.x core metrics can be reused for this or be modified

On 25 Apr 2020, at 11:34, Shay Dratler notifications@github.com wrote:

Hi @pmlopes https://github.com/pmlopes , which git repo is using this one : ?

https://github.com/vert-x3/vertx-stack https://github.com/vert-x3/vertx-stack https://github.com/vert-x3/vertx-micrometer-metrics https://github.com/vert-x3/vertx-micrometer-metrics https://github.com/vert-x3/vertx-dropwizard-metrics https://github.com/vert-x3/vertx-dropwizard-metrics or other ?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/vert-x3/issues/issues/182#issuecomment-619350862, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABXDCXZHOZKUA3BGS4E4A3ROKVBBANCNFSM4CN5U3XQ.

[vietj]

More generally speaking, this defines a generic need for clients that require to expose metrics.

Vert.x core defines an SPI for HTTP client metrics and I believe we see how this can be reused and made more generics, i.e here I am thinking at the notion of:

1. pooling (i.e pool usage)
2. client request queuing metrics 3 latency of a request
3. client request latencies : which is the queuing latency + the request latency
4. bytes transferred per request
5. error reporting

so we need to see how HttpClientMetrics can be made generic to work for most client

1/ HTTP (current one) 2/ Redis 3/ SQL client

On 25 Apr 2020, at 11:37, Julien Viet julien@julienviet.com wrote:

for doing this, we should find how metrics can be used, i.e currently Vert.x core provides an SPI for metrics.

We need to see how this integrates with this.

So I think we need to

1. define what are the metrics we want to expose
2. verify how Vert.x core metrics can be reused for this or be modified

On 25 Apr 2020, at 11:34, Shay Dratler <notifications@github.com mailto:notifications@github.com> wrote:

Hi @pmlopes https://github.com/pmlopes , which git repo is using this one : ?

https://github.com/vert-x3/vertx-stack https://github.com/vert-x3/vertx-stack https://github.com/vert-x3/vertx-micrometer-metrics https://github.com/vert-x3/vertx-micrometer-metrics https://github.com/vert-x3/vertx-dropwizard-metrics https://github.com/vert-x3/vertx-dropwizard-metrics or other ?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/vert-x3/issues/issues/182#issuecomment-619350862, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABXDCXZHOZKUA3BGS4E4A3ROKVBBANCNFSM4CN5U3XQ.

[dratler]

Hi @vietj , I have dag a little bit and here are my findings :	file Name	Metrics Recommendation
HttpClientMetrics	1. poll usage 2. latency of a request 3. error reporting
TCPMetrics	1. poll usage 2. latency of a request 3. error reporting
VertxMetrics	1. poll usage 2. latency of a request 3. error reporting
EventBusMetrics	1. poll usage 2. error reporting
SqlClientMetrics	1. poll usage 2. error reporting
RedisClientMetrics	1. poll usage 2. error reporting

what do you think?

[vietj]

@dratler the SPI is in vertx-core package : io.vertx.core.spi.metrics

[vietj]

I think we could start by having HttpClientMetrics being

1. being decoupled from HTTP protocol
2. not extend anymore TCPMetrics so it can be used when Vert.x does not control the TCP layer (e.g JDBC)

[vietj]

I will try doing such decoupling in a branch and then we can see how this can be reused in redis

[dratler]

Hi @vietj , I would like to then I can better understand what you have in mind or you can post if via GIST.

[vietj]

@dratler yes it would be good, I would like to have in Vert.x core HttpClientMetrics interface to become ClientMetrics instead so it can be used by HttpClient like today but also by other clients such as this Redis client.

This requires some changes in the SPI and the implementations of the interface but it seems worth to be done in master.

Does it make sense to you ?

[dratler]

HI @vietj , Can we append the new methods\functions to SPI ? I don't want to delete methods been used, I think for first part I would like to append.

[vietj]

Hi,

it is not clear what you mean :-)

On 27 Apr 2020, at 13:05, Shay Dratler notifications@github.com wrote:

HI @vietj https://github.com/vietj , Can we append the new methods\functions to SPI ? I don't want to delete methods been used, I think for first part I would like to append.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/vert-x3/issues/issues/182#issuecomment-619909651, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABXDCU2EUM3U2I7ZVT6ZCLROVRHXANCNFSM4CN5U3XQ.

[dratler]

Hi @vietj , you said the following :

I think we could start by having HttpClientMetrics being decoupled from HTTP protocol

I would like to add more functionalities and not delete functionalities for start.

[vietj]

@dratler ok, do the best you can we'll see after

[dratler]

Hi @vietj , I have found within class ClientHttpEndpointBase the following comment : private final HttpClientMetrics metrics; // Shall be removed later combining the PoolMetrics with HttpClientMetrics do you think it's preferred ?

[vietj]

@dratler please have a look at this branch https://github.com/eclipse-vertx/vert.x/tree/client-metrics

this is what I think we need to use, however I don't know how yet report request/response information in a way it can be used by metrics implementation without the need to downcast the generic requests and responses to the specific implementations

however this would report client activity in a generic fashion and do more for specific HTTP client.

[vietj]

note we have a similar issue with tracing.

[vietj]

maybe also from client perspective we don't need to report such accurate data on the client and thus we could only focus on URL and endpoint pool size + queuing

[vietj]

or we can do it specifically for HTTP and not care for other than just having an mere String for the request that would be the URI for HTTP and SQL statement for SQL, etc...

[dratler]

Hi @vietj , beside of dummy DummyVertxMetrics.class I have found, I would like to start with this task, where do you recommend I should start reading code? and if it's missing then where do you recommend I should start?

[dratler]

Hi @vietj I have a basic PR for getting all Redis metrics. for https://github.com/eclipse-vertx/vert.x/tree/client-metrics I'm getting 404 page not found therefor I have worked by look and feel for InfoDb and HttpClientMetrics. is that a good Idea ?

[vietj]

@dratler now this branch is merged in master so it should work