redis / jedis

Redis Java client
https://redis.github.io/jedis/
MIT License
11.9k stars 3.87k forks source link

Feature request to add a prometheus collector in jedis #3069

Open aarengee opened 2 years ago

aarengee commented 2 years ago

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

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.

Why not do this in jedis ?

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 .

sazzad16 commented 2 years ago

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

aarengee commented 2 years ago

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

sazzad16 commented 2 years ago

@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.

aarengee commented 2 years ago

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

sazzad16 commented 2 years ago

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

yangbodong22011 commented 2 years ago

@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.

BiLuoHen commented 1 year ago

@yangbodong22011 hello, is there any news about latency meterics

seal-7 commented 5 months ago

This is very much needed, we are using Jedis in high scale systems and in case of issue having this metrics helps a lot in root causing.