Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions module/spring-boot-neo4j/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ dependencies {
optional(project(":core:spring-boot-testcontainers"))
optional(project(":module:spring-boot-health"))
optional("org.testcontainers:neo4j")
optional("org.neo4j.driver:neo4j-java-driver-observation-micrometer")

dockerTestImplementation(project(":core:spring-boot-test"))
dockerTestImplementation(project(":test-support:spring-boot-docker-test-support"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@
import java.time.Duration;
import java.util.List;
import java.util.Locale;
import java.util.ServiceLoader;
import java.util.concurrent.TimeUnit;

import io.micrometer.observation.ObservationRegistry;
import org.jspecify.annotations.Nullable;
import org.neo4j.bolt.connection.BoltConnectionProviderFactory;
import org.neo4j.driver.AuthToken;
import org.neo4j.driver.AuthTokenManager;
import org.neo4j.driver.AuthTokens;
Expand All @@ -32,6 +35,7 @@
import org.neo4j.driver.Driver;
import org.neo4j.driver.GraphDatabase;
import org.neo4j.driver.internal.Scheme;
import org.neo4j.driver.observation.micrometer.MicrometerObservationProvider;

import org.springframework.beans.factory.ObjectProvider;
import org.springframework.boot.autoconfigure.AutoConfiguration;
Expand Down Expand Up @@ -63,6 +67,20 @@
@EnableConfigurationProperties(Neo4jProperties.class)
public final class Neo4jAutoConfiguration {

private static final boolean HAS_DRIVER_METRICS;

static {
boolean metricsObservationProviderFound = true;
try {
Class.forName("org.neo4j.driver.observation.micrometer.MicrometerObservationProvider", false,
Neo4jAutoConfiguration.class.getClassLoader());
}
catch (ClassNotFoundException ex) {
metricsObservationProviderFound = false;
}
HAS_DRIVER_METRICS = metricsObservationProviderFound;
}

@Bean
@ConditionalOnMissingBean(Neo4jConnectionDetails.class)
PropertiesNeo4jConnectionDetails neo4jConnectionDetails(Neo4jProperties properties,
Expand All @@ -73,10 +91,11 @@ PropertiesNeo4jConnectionDetails neo4jConnectionDetails(Neo4jProperties properti
@Bean
@ConditionalOnMissingBean
Driver neo4jDriver(Neo4jProperties properties, Environment environment, Neo4jConnectionDetails connectionDetails,
ObjectProvider<ConfigBuilderCustomizer> configBuilderCustomizers) {
ObjectProvider<ConfigBuilderCustomizer> configBuilderCustomizers,
ObjectProvider<ObservationRegistry> observationRegistryProvider) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't do this here. If we did then we could not configure a Neo4j driver without the observation registry on the classpath.


Config config = mapDriverConfig(properties, connectionDetails,
configBuilderCustomizers.orderedStream().toList());
configBuilderCustomizers.orderedStream().toList(), observationRegistryProvider);
AuthTokenManager authTokenManager = connectionDetails.getAuthTokenManager();
if (authTokenManager != null) {
return GraphDatabase.driver(connectionDetails.getUri(), authTokenManager, config);
Expand All @@ -86,29 +105,29 @@ Driver neo4jDriver(Neo4jProperties properties, Environment environment, Neo4jCon
}

Config mapDriverConfig(Neo4jProperties properties, Neo4jConnectionDetails connectionDetails,
List<ConfigBuilderCustomizer> customizers) {
List<ConfigBuilderCustomizer> customizers,
ObjectProvider<ObservationRegistry> observationRegistryProvider) {
Config.ConfigBuilder builder = Config.builder();
configurePoolSettings(builder, properties.getPool());
configurePoolSettings(builder, properties.getPool(), observationRegistryProvider);
URI uri = connectionDetails.getUri();
String scheme = (uri != null) ? uri.getScheme() : "bolt";
configureDriverSettings(builder, properties, isSimpleScheme(scheme));
builder.withLogging(new Neo4jSpringJclLogging());
customizers.forEach((customizer) -> customizer.customize(builder));
return builder.build();
}

private boolean isSimpleScheme(String scheme) {
String lowerCaseScheme = scheme.toLowerCase(Locale.ENGLISH);
try {
Scheme.validateScheme(lowerCaseScheme);
}
catch (IllegalArgumentException ex) {
if (!ServiceLoader.load(BoltConnectionProviderFactory.class)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's quite a bit of ceremony here. Do we really need to invoke "any" factory this way? Also this adds an import on BoltConnectionProviderFactory...

.stream()
.anyMatch((p) -> p.get().supports(lowerCaseScheme))) {
throw new IllegalArgumentException(String.format("'%s' is not a supported scheme.", scheme));
}
return lowerCaseScheme.equals("bolt") || lowerCaseScheme.equals("neo4j");
return !Scheme.isSecurityScheme(lowerCaseScheme);
}

private void configurePoolSettings(Config.ConfigBuilder builder, Pool pool) {
private void configurePoolSettings(Config.ConfigBuilder builder, Pool pool,
ObjectProvider<ObservationRegistry> observationRegistryProvider) {
if (pool.isLogLeakedSessions()) {
builder.withLeakedSessionsLogging();
}
Expand All @@ -120,12 +139,11 @@ private void configurePoolSettings(Config.ConfigBuilder builder, Pool pool) {
builder.withMaxConnectionLifetime(pool.getMaxConnectionLifetime().toMillis(), TimeUnit.MILLISECONDS);
builder.withConnectionAcquisitionTimeout(pool.getConnectionAcquisitionTimeout().toMillis(),
TimeUnit.MILLISECONDS);
if (pool.isMetricsEnabled()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I like that change. If metrics vs. no metrics is no longer needed, then I think the metricsEnabled flag should go away and have metrics to be exported if an ObservationRegistry is available. That's what the other components are doing at the moment.

Is there a reason to keep having a flag for this or was it just an attempt at migrating what we had before?

builder.withDriverMetrics();
}
else {
builder.withoutDriverMetrics();
}
observationRegistryProvider.ifAvailable((orp) -> {
if (pool.isMetricsEnabled() && HAS_DRIVER_METRICS) {
builder.withObservationProvider(MicrometerObservationProvider.builder(orp).build());
}
});
}

private void configureDriverSettings(Config.ConfigBuilder builder, Neo4jProperties properties,
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
import java.net.URI;
import java.time.Duration;
import java.util.Arrays;
import java.util.stream.Stream;

import io.micrometer.observation.ObservationRegistry;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.junit.jupiter.params.ParameterizedTest;
Expand All @@ -32,6 +34,7 @@
import org.neo4j.driver.Config.ConfigBuilder;
import org.neo4j.driver.Driver;

import org.springframework.beans.factory.ObjectProvider;
import org.springframework.boot.autoconfigure.AutoConfigurations;
import org.springframework.boot.context.properties.source.InvalidConfigurationPropertyValueException;
import org.springframework.boot.neo4j.autoconfigure.Neo4jAutoConfiguration.PropertiesNeo4jConnectionDetails;
Expand Down Expand Up @@ -217,13 +220,6 @@ void authenticationWithBothUsernameAndKerberosShouldNotBeAllowed() {
.withMessage("Cannot specify both username ('Farin') and kerberos ticket ('AABBCCDDEE')");
}

@Test
void poolWithMetricsEnabled() {
Neo4jProperties properties = new Neo4jProperties();
properties.getPool().setMetricsEnabled(true);
assertThat(mapDriverConfig(properties).isMetricsEnabled()).isTrue();
}

@Test
void poolWithLogLeakedSessions() {
Neo4jProperties properties = new Neo4jProperties();
Expand Down Expand Up @@ -322,14 +318,15 @@ void securityWithTrustSystemCertificates() {
.isEqualTo(Config.TrustStrategy.Strategy.TRUST_SYSTEM_CA_SIGNED_CERTIFICATES);
}

@Test
void driverConfigShouldBeConfiguredToUseUseSpringJclLogging() {
assertThat(mapDriverConfig(new Neo4jProperties()).logging()).isInstanceOf(Neo4jSpringJclLogging.class);
}

private Config mapDriverConfig(Neo4jProperties properties, ConfigBuilderCustomizer... customizers) {
return new Neo4jAutoConfiguration().mapDriverConfig(properties,
new PropertiesNeo4jConnectionDetails(properties, null), Arrays.asList(customizers));
new PropertiesNeo4jConnectionDetails(properties, null), Arrays.asList(customizers),
new ObjectProvider<>() {
@Override
public Stream<ObservationRegistry> stream() {
return Stream.empty();
}
});
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ class Neo4jPropertiesTests {
void poolSettingsHaveConsistentDefaults() {
Config defaultConfig = Config.defaultConfig();
Pool pool = new Neo4jProperties().getPool();
assertThat(pool.isMetricsEnabled()).isEqualTo(defaultConfig.isMetricsEnabled());
assertThat(pool.isLogLeakedSessions()).isEqualTo(defaultConfig.logLeakedSessions());
assertThat(pool.getMaxConnectionPoolSize()).isEqualTo(defaultConfig.maxConnectionPoolSize());
assertDuration(pool.getIdleTimeBeforeConnectionTest(), defaultConfig.idleTimeBeforeConnectionTest());
Expand Down
5 changes: 3 additions & 2 deletions platform/spring-boot-dependencies/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -1621,7 +1621,7 @@ bom {
]
}
}
library("Neo4j Java Driver", "5.28.9") {
library("Neo4j Java Driver", "6.0.0") {
alignWith {
version {
from "org.springframework.data:spring-data-neo4j"
Expand All @@ -1630,7 +1630,8 @@ bom {
}
group("org.neo4j.driver") {
modules = [
"neo4j-java-driver"
"neo4j-java-driver",
"neo4j-java-driver-observation-micrometer"
]
}
links {
Expand Down
Loading