jedis icon indicating copy to clipboard operation
jedis copied to clipboard

Feature request to add a prometheus collector in jedis

Open aarengee opened this issue 1 year ago • 8 comments

Feature request to add a prometheus collector in jedis

Add a implementation of Collector in src/main/java/redis/clients/jedis/metrics package or utils maybe

Basically a 1:1 clone of what HikariCP ships with here

Using this all

  • Jedispool instances behind a JedisCluster object created for a redis cluster ( 6 pools for 6 node cluster )
  • Jedispool instances for a standalone redis master instance ( 1 pool for a single redis instance )

can be instrumented using a simple line of code RedisCollector.getCollector().track(myjedisClusterOrPool)

Why as part of jedis and not as left upto user in there code.

  • In my opinion shipping it with jedis will add value to the library as it provides visibility to the user about the metrics as an inbuilt rather than having to instrument it separately.

  • As and when we add more metrics we can keep adding gauges/histogram/counter to the same collector . And the user wont even know these metrics are automatically available in the same registry . As soon as you upgrade your jedis library dependency metric is available. If I create it as a separate project then a pull request needs to be raised on that project , that project will have to upgrade jedis version , add those new metrics . All that

  • Prometheus is one of the most common timeseries in the industry with a simple to implement interface .

Why not do this in jedis ?

  • Need to add io.prometheus as a dependency
  • jedis wants to be light weight and not be opinionated about any other tool like prometheus

Sample code

import io.prometheus.client.Collector;
import io.prometheus.client.GaugeMetricFamily;
import io.prometheus.client.CollectorRegistry;
import redis.clients.jedis.JedisCluster;
import redis.clients.jedis.JedisPool;

import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;

import static java.util.Collections.singletonList;

public class RedisCollector extends Collector {

  private static final List<String> LABEL_NAMES = singletonList("host");

  private static Map<String, JedisPool> poolStatsMap = new ConcurrentHashMap<>();

  private static RedisCollector redisCollector;

  public static RedisCollector getCollector() {
    if (redisCollector == null) {
      redisCollector = new RedisCollector().register();
    }
    return redisCollector;
  }

  public static RedisCollector getCollector(CollectorRegistry registry) {
    if (redisCollector == null) {
      redisCollector = new RedisCollector();
      registry.register(redisCollector);
    }
    return redisCollector;
  }

  @Override
  public List<MetricFamilySamples> collect() {
    return Arrays.asList(
        createGauge("jedis_pool_num_active", "Jedis Pool Active connections", JedisPool::getNumActive),
        createGauge("jedis_pool_idle_connections", "Jedis Pool Idle connections", JedisPool::getNumIdle),
        createGauge("jedis_pool_num_waiters", "Jedis Pool Waiting connections", JedisPool::getNumWaiters)

    );
  }

  public void track(JedisCluster cluster) {
    poolStatsMap = cluster.getClusterNodes();
  }

  public void track(String poolName, JedisPool pool) {
    poolStatsMap.put(poolName, pool);
  

  private GaugeMetricFamily createGauge(String metric, String help,
                                        Function<JedisPool, Integer> driverFunction) {
    GaugeMetricFamily metricFamily = new GaugeMetricFamily(metric, help, LABEL_NAMES);
    poolStatsMap.forEach((poolName, pool) -> metricFamily.addMetric(singletonList(poolName), driverFunction.apply(pool)));
    return metricFamily;
  }
}

Sample metrics

jedis_cluster_num_active{host="127.0.0.1:7006",} 0.0
jedis_cluster_num_active{host="127.0.0.1:7004",} 0.0
jedis_cluster_num_active{host="127.0.0.1:7005",} 0.0
jedis_cluster_num_active{host="127.0.0.1:7002",} 0.0
jedis_cluster_num_active{host="127.0.0.1:7003",} 0.0
jedis_cluster_num_active{host="127.0.0.1:7001",} 1.0

jedis_cluster_idle_connections{host="127.0.0.1:7006",} 50.0
jedis_cluster_idle_connections{host="127.0.0.1:7004",} 50.0
jedis_cluster_idle_connections{host="127.0.0.1:7005",} 50.0
jedis_cluster_idle_connections{host="127.0.0.1:7002",} 50.0
jedis_cluster_idle_connections{host="127.0.0.1:7003",} 50.0
jedis_cluster_idle_connections{host="127.0.0.1:7001",} 50.0

jedis_cluster_num_waiters{host="127.0.0.1:7006",} 0.0
jedis_cluster_num_waiters{host="127.0.0.1:7004",} 0.0
jedis_cluster_num_waiters{host="127.0.0.1:7005",} 0.0
jedis_cluster_num_waiters{host="127.0.0.1:7002",} 0.0
jedis_cluster_num_waiters{host="127.0.0.1:7003",} 0.0
jedis_cluster_num_waiters{host="127.0.0.1:7001",} 0.0

I would really like to contribute. Thanks for your consideration .

aarengee avatar Jul 15 '22 16:07 aarengee

@aarengee Thank you for descriptive feature request. Keeping it here for discussion.

sazzad16 avatar Jul 16 '22 07:07 sazzad16

Hi @sazzad16 by when can we expect to close or get comments on this ? Like a month . Not pushing just asking the median time given past experience

aarengee avatar Jul 20 '22 07:07 aarengee

@aarengee It could take several days/weeks get some discussions. Collaborators do it voluntarily besides their daily job.

From me, personally, we added Gson and org.json since Jedis 4.0.0 and we kind of have backlash for both of those. So at this moment I have more negative perception about adding new dependency than I usually do.

sazzad16 avatar Jul 21 '22 05:07 sazzad16

Yes I am very much grateful to all collaborators working on all OSS. You guys are the unsung heroes doing this not out of necessity and not for monetary gains. Fortune 500 companies save millions cause of OSS :)

Can you tag some active collaborators ? If the sentiment is same that adding prom support is out of scope which I completely understand then I will proceed to create a a simple three file project ( pom + java + test ) on my own account . Will also link it here just in case people want to use contribute or better decide to add it to jedis later on .

Thanks @sazzad16

aarengee avatar Jul 21 '22 14:07 aarengee

Pinging @dengliming @yangbodong22011 @mina-asham @gkorland @chayim if you have some time.

sazzad16 avatar Jul 21 '22 15:07 sazzad16

@aarengee Thanks for your ticket.

The functions you describe (get maxTotal, maxIdle, minIdle, etc.) can be obtained and monitored directly through the JMX port (p.s. Jedis's DEFAULT_JMX_ENABLE is true by default), it is more suitable as a function of Apache common-pool2, not Jedis, that's exactly what it is now.

As a client, Jedis really needs to provide Trace functions, like Lettuce's Command Latency Metrics or go-redis with opentelemetry, there is no clear roadmap yet.

I will proceed to create a simple three file project ( pom + java + test ) on my own account . Will also link it here just in case people want to use contribute or better decide to add it to jedis later on .

Very welcome and thanks.

yangbodong22011 avatar Jul 25 '22 08:07 yangbodong22011

@yangbodong22011 hello, is there any news about latency meterics

BiLuoHen avatar Aug 16 '23 02:08 BiLuoHen