spring-projects / spring-data-cassandra

Provides support to increase developer productivity in Java when using Apache Cassandra. Uses familiar Spring concepts such as a template classes for core API usage and lightweight repository style data access.
https://spring.io/projects/spring-data-cassandra/
Apache License 2.0
381 stars 311 forks source link

Add constructor to CassandraAdminTemplate accepting CqlOperations [DATACASS-675] #846

Closed spring-projects-issues closed 5 years ago

spring-projects-issues commented 5 years ago

Abhijit Sarkar opened DATACASS-675 and commented

Use Case: I'm trying to capture Cassandra client-side query latency metrics using Micrometer. Cassandra Java driver exposes some metrics through Dropwizard, but there's no way to bridge Dropwizard -> Micrometer for general meters like timers and histograms (there exists a Dropwizard MetricRegistryListener, but the methods in it are invoked after a meter has been added to the registry, thus eliminating any chance of interception - filed https://github.com/dropwizard/dropwizard/issues/2840).

Implementation Approach: The idea is to time all calls to CqlOperations. This is, however, easier said than done due to the following problems in the code.

  1. For some unknown reason, AbstractCassandraConfiguration.cassandraTemplate returns a concrete class CassandraAdminTemplate, not the interface type CassandraAdminOperations. This makes it very difficult to proxy the returned object.
  2. CassandraAdminTemplate does not implement a constructor that allows invocation of the superclass constructor CassandraTemplate(CqlOperations cqlOperations, CassandraConverter converter). Had this been possible, simply passing in a proxied CqlOperations would have allowed for a clean way of capturing the metrics.

I'm currently forced to resort to ugly reflections hacks.

public CassandraAdminTemplate cassandraTemplate() throws Exception {
    CassandraAdminTemplate cassandraAdminTemplate = super.cassandraTemplate();

    if (getMetricsEnabled()) {
        Timer latencyTimer = meterRegistry.timer("cassandra.requests");
        ReflectionUtils.doWithFields(cassandraAdminTemplate.getClass(), cqlOperations -> {
                    cqlOperations.setAccessible(true);
                    Field modifiersField = ReflectionUtils.findField(Field.class, "modifiers");
                    modifiersField.setAccessible(true);
                    modifiersField.setInt(cqlOperations, cqlOperations.getModifiers() & ~Modifier.FINAL);
                    ProxyFactory proxyFactory = new ProxyFactory(cqlOperations.get(cassandraAdminTemplate));
                    proxyFactory.addAdvice((MethodInterceptor) invocation -> latencyTimer.recordCallable(() -> {
                        try {
                            return invocation.proceed();
                        } catch (Throwable t) {
                            throw new RuntimeException(t);
                        }
                    }));
                    cqlOperations.set(cassandraAdminTemplate, proxyFactory.getProxy());
                }, field -> field.getName().equals("cqlOperations")
        );
    }
    return cassandraAdminTemplate;
}

Affects: 2.1.9 (Lovelace SR9)

Referenced from: commits https://github.com/spring-projects/spring-data-cassandra/commit/946ef41da22e0b7d7a721451edb2228ec6553a3a

spring-projects-issues commented 5 years ago

Mark Paluch commented

Thanks for your elaborate request.

Configuration classes typically return the most specific type to include these type of information into the Spring bean definition. Why do you want to plug metrics into the Template API and not directly into the driver. The Datastax driver exposes a dedicated API to track latencies. As of driver 4.0, the metrics registry is going to be pluggable

spring-projects-issues commented 5 years ago

Abhijit Sarkar commented

I wasn’t aware of the QueryLogger, but I see that it is a LatencyTracker. I’ve already checked that the latency tracker is not appropriate for client requests. When a query was made for system table, what it logged instead was use system. As for Configuration class returning specific bean types, I don’t really see a good reason for that. Moreover, as I said in the ticket, simply exposing the constructor that accepts a CqlOperation solves the problem at hand

spring-projects-issues commented 5 years ago

Mark Paluch commented

It makes sense anyway to align constructors