Add Cassandra driver Actuator metrics #21624
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
| @@ -0,0 +1,58 @@ | |||
| /* | |||
| * Copyright 2012-2019 the original author or authors. | |||
There was a problem hiding this comment.
Any reason why the copyright ends at 2019 instead of 2020?
There was a problem hiding this comment.
oversight, from a copy-paste
emerkle826
force-pushed
the
cassandra-metrics
branch
2 times, most recently
from
d1dcbb0
to
35e5acf
Compare
philwebb
added
the
for: team-attention
| return CassandraMetricsBinder.SESSION_PREFIX + "." + CQL_SESSION_NAME + "." + metric; | ||
| } | ||
|
|
||
| private void assertGuageMetric(MeterRegistry registry, String metric, double expectedValue) { |
There was a problem hiding this comment.
Small typo here: Guage -> Gauge.
emerkle826
force-pushed
the
cassandra-metrics
branch
from
35e5acf
to
8e6045c
Compare
philwebb
added
type: enhancement
There was a problem hiding this comment.
Converting from Dropwizard metrics to Micrometer metrics tends to be lossy and have an impedance mismatch. Furthermore, if a MeterBinder is the solution that ends up being used, I'm not sure it belongs in Spring Boot as there is nothing specific to Spring or Spring Boot in it - either the Cassandra driver or Micrometer would seem more appropriate. Spring Boot could make use of that, then. That said, rather than mapping to non-idiomatic Micrometer metrics, it would be best if the Cassandra driver either:
- supported Micrometer directly as an option. Or
- had hooks so that Micrometer could instrument the metrics natively, without passing through Dropwizard metrics.
See micrometer-metrics/micrometer#1397 and micrometer-metrics/micrometer#1359
| /** | ||
| * Description for micrometer metrics. | ||
| */ | ||
| private static final String DESCRIPTION = "Please see reference.conf for more information."; |
There was a problem hiding this comment.
I'm not sure what reference.conf is but this does not seem like a useful description, in my opinion. Descriptions may be published to metric backends, which almost certainly won't have access to the mentioned reference.conf. Even accessing the Actuator endpoint does not mean that person has access to this reference.conf file.
|
|
||
| TIMER_METRICS.forEach((n, supplier) -> { | ||
| String nameTimerMetric = String.format("%s.%s", name, n); | ||
| FunctionCounter.builder(nameTimerMetric, timer, (t) -> supplier.apply(timer).doubleValue()).tags(this.tags) |
There was a problem hiding this comment.
Counters, including FunctionCounter, in Micrometer are expected to be monotonically increasing, which is not the case with these rates, I believe. If you were to convert these, they would be gauges in Micrometer.
| String nameTimerMetric = String.format("%s.%s", name, n); | ||
| FunctionCounter.builder(nameTimerMetric, timer, (t) -> supplier.apply(timer).doubleValue()).tags(this.tags) | ||
| // all rates are calculated per second | ||
| .baseUnit(TimeUnit.SECONDS.name()).description(DESCRIPTION).register(meterRegistry); |
There was a problem hiding this comment.
The rate is per second; not counting seconds, which is what this ends up seeming like - a count of seconds.
| final Snapshot snapshot = timer.getSnapshot(); | ||
| TIMER_SNAPSHOT_METRICS.forEach((n, supplier) -> { | ||
| String nameTimerMetric = String.format("%s.%s", name, n); | ||
| FunctionCounter.builder(nameTimerMetric, timer, (t) -> supplier.apply(snapshot).longValue()).tags(this.tags) |
There was a problem hiding this comment.
Again, not monotonically increasing (except for max) so should not be a FunctionCounter.
| /** | ||
| * Prefix for all Driver configuration parameters. | ||
| */ | ||
| public static final String SESSION_PREFIX = "spring.data.cassandra.driver.session"; |
There was a problem hiding this comment.
Are there metrics not related to sessions available?
snicoll
added
the
status: waiting-for-feedback
Thank you for the review, you have valid points about this sort of metrics binding not being specific to Spring. We (the driver team at Datastax) have discussed it further and have decided to try the approach of providing different metrics bindings for the Cassandra driver (ticket is here). I'll convert this to a draft PR and revisit it once that work is completed. |
spring-projects-issues
added
status: feedback-provided
snicoll
added
status: blocked
|
@emerkle826 thanks for the follow up! |
|
@emerkle826 does this PR still make sense? I understand some major changes are pending in the driver so I am wondering if you would reuse this PR to update us. Thank you. |
|
@snicoll I think this PR will become obsolete once we make changes on the driver side. I'll close it. If a change still needs to happen in Spring Boot after the driver changes, I'll open a new PR as it will most likely not resemble this one much at all. |
|
Thanks for confirming. Let us know if we can help! |
snicoll
removed
status: blocked
Motivation:
The DataStax Java driver for Cassandra can be configured to expose driver session operational metrics. Currently these metrics are not available through Spring Boot Actuator.
Modifications:
This commit adds support for Cassandra driver metrics through Spring Boot Actuator by binding the native driver metrics to Micrometer metrics.
Result:
Users are now able to access Cassandra driver enabled metrics through Actuator .