-
Notifications
You must be signed in to change notification settings - Fork 40.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Cassandra driver Actuator metrics #23008
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
/* | ||
* Copyright 2020 the original author or authors. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* https://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.springframework.boot.actuate.autoconfigure.metrics.cassandra; | ||
|
||
import com.datastax.oss.driver.api.core.CqlSession; | ||
import com.datastax.oss.driver.api.core.config.DriverConfigLoader; | ||
import com.datastax.oss.driver.internal.metrics.micrometer.MicrometerDriverContext; | ||
import io.micrometer.core.instrument.MeterRegistry; | ||
|
||
import org.springframework.beans.factory.ObjectProvider; | ||
import org.springframework.boot.actuate.autoconfigure.metrics.MetricsAutoConfiguration; | ||
import org.springframework.boot.actuate.autoconfigure.metrics.export.simple.SimpleMetricsExportAutoConfiguration; | ||
import org.springframework.boot.autoconfigure.AutoConfigureAfter; | ||
import org.springframework.boot.autoconfigure.AutoConfigureBefore; | ||
import org.springframework.boot.autoconfigure.EnableAutoConfiguration; | ||
import org.springframework.boot.autoconfigure.cassandra.CassandraAutoConfiguration; | ||
import org.springframework.boot.autoconfigure.cassandra.CassandraProperties; | ||
import org.springframework.boot.autoconfigure.cassandra.CqlSessionBuilderCustomizer; | ||
import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; | ||
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; | ||
import org.springframework.context.ApplicationContext; | ||
import org.springframework.context.annotation.Bean; | ||
import org.springframework.context.annotation.Configuration; | ||
|
||
/** | ||
* {@link EnableAutoConfiguration Auto-configuration} for metrics on all available | ||
* {@link CqlSession Cassandra sessions}. | ||
* | ||
* @author Erik Merkle | ||
* @since 2.4.0 | ||
*/ | ||
@Configuration | ||
@AutoConfigureAfter({ MetricsAutoConfiguration.class, SimpleMetricsExportAutoConfiguration.class }) | ||
@AutoConfigureBefore({ CassandraAutoConfiguration.class }) | ||
@ConditionalOnClass({ MeterRegistry.class, MicrometerDriverContext.class }) | ||
@ConditionalOnBean({ MeterRegistry.class }) | ||
public class CassandraMetricsAutoConfiguration { | ||
|
||
@Bean | ||
public CassandraMetricsPostProcessor cassandraMetricsPostProcessor(ApplicationContext context, | ||
CassandraProperties properties, DriverConfigLoader driverConfigLoader, | ||
ObjectProvider<CqlSessionBuilderCustomizer> builderCustomizers) { | ||
return new CassandraMetricsPostProcessor(context, properties, driverConfigLoader, builderCustomizers); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,116 @@ | ||
/* | ||
* Copyright 2020 the original author or authors. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* https://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.springframework.boot.actuate.autoconfigure.metrics.cassandra; | ||
|
||
import java.security.NoSuchAlgorithmException; | ||
|
||
import javax.net.ssl.SSLContext; | ||
|
||
import com.datastax.oss.driver.api.core.CqlSessionBuilder; | ||
import com.datastax.oss.driver.api.core.config.DriverConfigLoader; | ||
import com.datastax.oss.driver.api.core.context.DriverContext; | ||
import com.datastax.oss.driver.api.core.session.ProgrammaticArguments; | ||
import com.datastax.oss.driver.internal.metrics.micrometer.MicrometerDriverContext; | ||
import io.micrometer.core.instrument.MeterRegistry; | ||
|
||
import org.springframework.beans.factory.ObjectProvider; | ||
import org.springframework.beans.factory.config.BeanPostProcessor; | ||
import org.springframework.boot.autoconfigure.cassandra.CassandraProperties; | ||
import org.springframework.boot.autoconfigure.cassandra.CqlSessionBuilderCustomizer; | ||
import org.springframework.context.ApplicationContext; | ||
import org.springframework.core.Ordered; | ||
|
||
/** | ||
* {@link BeanPostProcessor} that configures Cassandra metrics. This is necessary as the | ||
* CqlSessionBuilder bean provided by {@link CassandraAutoConfigure} must be overridden in | ||
* order to enable driver metrics. The post-processing here will do just that. | ||
* | ||
* @author Erik Merkle | ||
*/ | ||
class CassandraMetricsPostProcessor implements BeanPostProcessor, Ordered { | ||
|
||
private final ApplicationContext context; | ||
|
||
private final CassandraProperties cassandraProperties; | ||
|
||
private final DriverConfigLoader driverConfigLoader; | ||
|
||
private final ObjectProvider<CqlSessionBuilderCustomizer> builderCustomizers; | ||
|
||
CassandraMetricsPostProcessor(ApplicationContext context, CassandraProperties cassandraProperties, | ||
DriverConfigLoader driverConfigLoader, ObjectProvider<CqlSessionBuilderCustomizer> builderCustomizers) { | ||
this.context = context; | ||
this.cassandraProperties = cassandraProperties; | ||
this.driverConfigLoader = driverConfigLoader; | ||
this.builderCustomizers = builderCustomizers; | ||
} | ||
|
||
@Override | ||
public Object postProcessAfterInitialization(Object bean, String beanName) { | ||
if (bean instanceof CqlSessionBuilder) { | ||
MetricsSessionBuilder builder = new MetricsSessionBuilder(this.context.getBean(MeterRegistry.class)); | ||
// the Metrics enabled builder will need all of the same config as a regular | ||
// CqlSessionBUilder | ||
builder.withConfigLoader(this.driverConfigLoader); | ||
configureAuthentication(this.cassandraProperties, builder); | ||
configureSsl(this.cassandraProperties, builder); | ||
builder.withKeyspace(this.cassandraProperties.getKeyspaceName()); | ||
this.builderCustomizers.orderedStream().forEach((customizer) -> customizer.customize(builder)); | ||
// override the builder bean | ||
return builder; | ||
} | ||
return bean; | ||
} | ||
|
||
private void configureAuthentication(CassandraProperties properties, MetricsSessionBuilder builder) { | ||
if (properties.getUsername() != null) { | ||
builder.withAuthCredentials(properties.getUsername(), properties.getPassword()); | ||
} | ||
} | ||
|
||
private void configureSsl(CassandraProperties properties, MetricsSessionBuilder builder) { | ||
if (properties.isSsl()) { | ||
try { | ||
builder.withSslContext(SSLContext.getDefault()); | ||
} | ||
catch (NoSuchAlgorithmException ex) { | ||
throw new IllegalStateException("Could not setup SSL default context for Cassandra", ex); | ||
} | ||
} | ||
} | ||
|
||
@Override | ||
public int getOrder() { | ||
return Ordered.HIGHEST_PRECEDENCE; | ||
} | ||
|
||
static class MetricsSessionBuilder extends CqlSessionBuilder { | ||
|
||
private final MeterRegistry registry; | ||
|
||
MetricsSessionBuilder(MeterRegistry registry) { | ||
this.registry = registry; | ||
} | ||
|
||
@Override | ||
protected DriverContext buildContext(DriverConfigLoader configLoader, | ||
ProgrammaticArguments programmaticArguments) { | ||
return new MicrometerDriverContext(configLoader, programmaticArguments, this.registry); | ||
} | ||
|
||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
/* | ||
* Copyright 2020 the original author or authors. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* https://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
/** | ||
* Auto-configuration for Cassandra metrics. | ||
*/ | ||
package org.springframework.boot.actuate.autoconfigure.metrics.cassandra; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,149 @@ | ||
/* | ||
* Copyright 2020 the original author or authors. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* https://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
package org.springframework.boot.actuate.autoconfigure.metrics.cassandra; | ||
|
||
import java.net.InetSocketAddress; | ||
import java.time.Duration; | ||
import java.util.Arrays; | ||
import java.util.concurrent.TimeUnit; | ||
|
||
import com.datastax.oss.driver.api.core.ConsistencyLevel; | ||
import com.datastax.oss.driver.api.core.CqlSession; | ||
import com.datastax.oss.driver.api.core.CqlSessionBuilder; | ||
import com.datastax.oss.driver.api.core.config.DefaultDriverOption; | ||
import com.datastax.oss.driver.api.core.cql.SimpleStatement; | ||
import com.datastax.oss.driver.api.core.metrics.DefaultNodeMetric; | ||
import com.datastax.oss.driver.api.core.metrics.DefaultSessionMetric; | ||
import io.micrometer.core.instrument.MeterRegistry; | ||
import org.junit.jupiter.api.Test; | ||
import org.rnorth.ducttape.TimeoutException; | ||
import org.rnorth.ducttape.unreliables.Unreliables; | ||
import org.testcontainers.containers.CassandraContainer; | ||
import org.testcontainers.containers.ContainerLaunchException; | ||
import org.testcontainers.containers.wait.strategy.AbstractWaitStrategy; | ||
import org.testcontainers.junit.jupiter.Container; | ||
import org.testcontainers.junit.jupiter.Testcontainers; | ||
|
||
import org.springframework.boot.actuate.autoconfigure.metrics.test.MetricsRun; | ||
import org.springframework.boot.autoconfigure.AutoConfigurations; | ||
import org.springframework.boot.autoconfigure.cassandra.CassandraAutoConfiguration; | ||
import org.springframework.boot.autoconfigure.cassandra.DriverConfigLoaderBuilderCustomizer; | ||
import org.springframework.boot.test.context.runner.ApplicationContextRunner; | ||
import org.springframework.context.annotation.Bean; | ||
import org.springframework.context.annotation.Configuration; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
|
||
/** | ||
* Tests for {@link CassandraMetricsAutoConfiguration}. | ||
* | ||
* @author Erik Merkle | ||
*/ | ||
@Testcontainers(disabledWithoutDocker = true) | ||
public class CassandraMetricsAutoConfigurationIntegrationTests { | ||
|
||
@Container | ||
static final CassandraContainer<?> cassandra = new CassandraContainer<>().withStartupAttempts(5) | ||
.withStartupTimeout(Duration.ofMinutes(10)).waitingFor(new CassandraWaitStrategy()); | ||
|
||
private final ApplicationContextRunner contextRunner = new ApplicationContextRunner().with(MetricsRun.simple()) | ||
.withConfiguration( | ||
AutoConfigurations.of(CassandraMetricsAutoConfiguration.class, CassandraAutoConfiguration.class)) | ||
.withPropertyValues( | ||
"spring.data.cassandra.contact-points:" + cassandra.getHost() + ":" | ||
+ cassandra.getFirstMappedPort(), | ||
"spring.data.cassandra.local-datacenter=datacenter1", "spring.data.cassandra.read-timeout=20s", | ||
"spring.data.cassandra.connect-timeout=10s"); | ||
|
||
/** | ||
* Cassandra driver metrics should be enabled by default as long as the desired | ||
* metrics are enabled in the Driver's configuration. | ||
*/ | ||
@Test | ||
void autoConfiguredCassandraIsInstrumented() { | ||
this.contextRunner.withUserConfiguration(SimpleDriverConfigLoaderBuilderMetricsConfig.class).run((context) -> { | ||
CqlSessionBuilder builder = context.getBean(CqlSessionBuilder.class); | ||
assertThat(builder).isInstanceOf(CassandraMetricsPostProcessor.MetricsSessionBuilder.class); | ||
MeterRegistry registry = context.getBean(MeterRegistry.class); | ||
// execute queries to peg metrics | ||
CqlSession session = context.getBean(CqlSession.class); | ||
SimpleStatement statement = SimpleStatement.newInstance("SELECT release_version FROM system.local") | ||
.setConsistencyLevel(ConsistencyLevel.LOCAL_ONE); | ||
for (int i = 0; i < 10; ++i) { | ||
assertThat(session.execute(statement).one()).isNotNull(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you need to loop 10 times here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I loop here just to peg the request count more than once and verify in the assertion below. 10 was just some arbitrary value that was more than 1. |
||
} | ||
// assert Session metrics | ||
String sessionMetricPrefix = session.getContext().getSessionName() + "."; | ||
assertThat( | ||
registry.get(sessionMetricPrefix + DefaultSessionMetric.CONNECTED_NODES.getPath()).gauge().value()) | ||
.isEqualTo(1d); | ||
assertThat(registry.get(sessionMetricPrefix + DefaultSessionMetric.CQL_REQUESTS.getPath()).timer().count()) | ||
.isEqualTo(10L); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was a bit confused by that. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Testcontainers wait strategy does execute a query, whereas the wait strategy implementation in the Spring Boot Cassandra Autoconfigure test does not. The query is basic (it just queries the database version) and should succeed when the Cassandra cluster is up and ready. If the query fails or times out, it's likely due to the cluster not being available, which would mean that this integration test likely won't succeed either. The wait strategy implementation I copied and removed just waits for a Cassandra session builder to be built, which doesn't guarantee that the cluster is ready to respond to queries (although I haven't had that happen in practice locally). Maybe the ideal solution is to update the wait strategy implementation in Testcontainers so that it is more similar to the implementation in the Spring Boot test here, and then just use that as necessary for Spring Boot Cassandra ITs? |
||
assertThat(registry.get(sessionMetricPrefix + DefaultSessionMetric.BYTES_SENT.getPath()).counter().count()) | ||
.isGreaterThan(1d); | ||
assertThat( | ||
registry.get(sessionMetricPrefix + DefaultSessionMetric.BYTES_RECEIVED.getPath()).counter().count()) | ||
.isGreaterThan(1d); | ||
// assert Node metrics | ||
String nodeMetricPrefix = sessionMetricPrefix + "nodes." + cassandra.getHost() + ":" | ||
+ cassandra.getMappedPort(9042) + "."; | ||
assertThat(registry.get(nodeMetricPrefix + DefaultNodeMetric.BYTES_SENT.getPath()).counter().count()) | ||
.isGreaterThan(1d); | ||
|
||
}); | ||
} | ||
|
||
@Configuration(proxyBeanMethods = false) | ||
static class SimpleDriverConfigLoaderBuilderMetricsConfig { | ||
|
||
@Bean | ||
DriverConfigLoaderBuilderCustomizer customizer() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If that represents what users have to do in order to get metrics support, I think something is missing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For Driver 4.x, metrics are not enabled by default. To enable metrics in the driver, the driver must be configured with the list of enabled metrics. This can be done programmatically, with System properties or by providing a driver config file. For this test, it was easiest to just provide a Driver config customizer bean. I can change the test to enable the metrics under test via System properties or with an external driver configuration file if you prefer. Are you concerned with how metrics are enabled in the driver? Or should I be enabling metrics automatically in the Actuator? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If we provide first-class support for metrics here, the user shouldn't have to write code to enable it ideally. Looking at other metrics we have, they are usually enabled by default, sometimes with a flag that indicates if metrics for that particular I don't have an opinion as whether the flag should be enabled by default or not but listing the metrics seem a bit tedious and inconsistent for an "out-of-the-box" scenario. Paging @shakuzen to get some more feedback on this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not very familiar with Cassandra driver specifics, but it's probably only metrics on specific queries which might have high cardinality or privacy concerns (if query parameters/values are tagged, for example) that we wouldn't want to enable by default, I think. Connection pool or node or session metrics seem generally useful and probably safe from the aforementioned concerns. |
||
return (builder) -> builder | ||
.withStringList(DefaultDriverOption.METRICS_SESSION_ENABLED, Arrays.asList( | ||
DefaultSessionMetric.CONNECTED_NODES.getPath(), DefaultSessionMetric.CQL_REQUESTS.getPath(), | ||
DefaultSessionMetric.BYTES_SENT.getPath(), DefaultSessionMetric.BYTES_RECEIVED.getPath())) | ||
.withStringList(DefaultDriverOption.METRICS_NODE_ENABLED, | ||
Arrays.asList(DefaultNodeMetric.BYTES_SENT.getPath())); | ||
} | ||
|
||
} | ||
|
||
static final class CassandraWaitStrategy extends AbstractWaitStrategy { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated but rather than copy/pasting this, I wonder if it wouldn't be better if Testcontainers had some sort of support for this. Feel free to chime in in the issue I've just created. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Testcontainers does heave a Cassandra wait strategy implementation here which is pretty similar to the implementation I copied. I could swap the implementations in Spring for the one Testcontainers provides. That said, I have an issue to try to update Testcontainers from Driver 3.x to Driver 4.x here. My PR is under review, though it hasn't gotten a lot of traction. The reason I mention it is because I haven't yet addressed the Driver 3.x API tie in that is used in their wait strategy. From a Testcontainer perspective, they would prefer to remove the coupling to specific Cassandra Driver APIs (so driver version changes in the future don't impact Testcontainers). Providing a Driver API agnostic wait strategy would have to be included to achieve that. |
||
|
||
@Override | ||
protected void waitUntilReady() { | ||
try { | ||
Unreliables.retryUntilSuccess((int) this.startupTimeout.getSeconds(), TimeUnit.SECONDS, () -> { | ||
getRateLimiter().doWhenReady(() -> cqlSessionBuilder().build()); | ||
return true; | ||
}); | ||
} | ||
catch (TimeoutException ex) { | ||
throw new ContainerLaunchException( | ||
"Timed out waiting for Cassandra to be accessible for query execution"); | ||
} | ||
} | ||
|
||
private CqlSessionBuilder cqlSessionBuilder() { | ||
return CqlSession.builder() | ||
.addContactPoint(new InetSocketAddress(this.waitStrategyTarget.getHost(), | ||
this.waitStrategyTarget.getFirstMappedPort())) | ||
.withLocalDatacenter("datacenter1").withAuthCredentials("cassandra", "cassandra"); | ||
} | ||
|
||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a
CqlSessionBuilderCustomizer
that should be used for this. Please adapt the contract in the Cassandra API so that an override of the builder is not necessary.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the review. Currently, metrics binding in the driver happens a little deeper than the Session builder level (at the DriverContext level). I'll look into trying to modify the driver code to allow for using the Customizer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, much appreciated.