Skip to content

Add support for Elasticsearch Sniffer #24340

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

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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 spring-boot-project/spring-boot-autoconfigure/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ dependencies {
optional("org.ehcache:ehcache")
optional("org.elasticsearch.client:elasticsearch-rest-client")
optional("org.elasticsearch.client:elasticsearch-rest-high-level-client")
optional("org.elasticsearch.client:elasticsearch-rest-client-sniffer")
optional("org.flywaydb:flyway-core")
optional("org.freemarker:freemarker")
optional("org.glassfish.jersey.core:jersey-server")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* Copyright 2012-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.autoconfigure.elasticsearch.sniff;

import org.elasticsearch.client.RestClient;
import org.elasticsearch.client.RestHighLevelClient;
import org.elasticsearch.client.sniff.Sniffer;
import org.elasticsearch.client.sniff.SnifferBuilder;
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.boot.context.properties.PropertyMapper;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;

import java.time.Duration;

/**
* {@link EnableAutoConfiguration Auto-configuration} for Elasticsearch Sniffer.
*
* @since
*/
@Configuration(proxyBeanMethods = false)
@ConditionalOnClass(RestHighLevelClient.class)
@ConditionalOnMissingBean(RestClient.class)
Copy link
Member

Choose a reason for hiding this comment

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

The purpose of the sniffer is to work based on a RestClient but this auto-configuration doesn't do anything if such bean is present?

@EnableConfigurationProperties(ElasticsearchSnifferProperties.class)
public class ElasticsearchSnifferAutoConfiguration {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it should be a separate auto-configuration. Rather it should be part of what we already auto-configure.


@Configuration(proxyBeanMethods = false)
@ConditionalOnMissingBean(SnifferBuilder.class)
static class SnifferBuilderConfiguration {

@Bean
SnifferBuilderCustomizer defaultSnifferBuilderCustomizer(ElasticsearchSnifferProperties properties) {
Copy link
Member

Choose a reason for hiding this comment

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

For simple use case like this one there is no need to create a customizer for what the auto-configuration does. We can smart small and configure the sniffer based on the properties only.

return new DefaultSnifferBuilderCustomizer(properties);
}

@Bean
Sniffer elasticsearchSnifferBuilder(ElasticsearchSnifferProperties properties,
RestClient elasticsearchRestClient) {
return Sniffer.builder(elasticsearchRestClient).build();
Copy link
Member

Choose a reason for hiding this comment

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

Spring Boot no longer exposes a RestClient. Rather, it is created internally as part of ElasticsearchRestClientAutoConfiguration which is where this code should go.

The documentation also states that order in which those are destroy is important:

It is important to close the Sniffer so that its background thread gets properly shutdown and all of its resources are released. The Sniffer object should have the same lifecycle as the RestClient and get closed right before the client:

}
}

static class DefaultSnifferBuilderCustomizer implements SnifferBuilderCustomizer {

private static final PropertyMapper map = PropertyMapper.get();

private final ElasticsearchSnifferProperties properties;

DefaultSnifferBuilderCustomizer(ElasticsearchSnifferProperties properties) {
this.properties = properties;
}

@Override
public void customize(SnifferBuilder builder) {
map.from(this.properties::getSniffIntervalMillis).whenNonNull().asInt(Duration::toMillis)
.to(builder::setSniffIntervalMillis);
map.from(this.properties::getSniffFailureDelayMillis).whenNonNull().asInt(Duration::toMillis)
.to(builder::setSniffAfterFailureDelayMillis);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* Copyright 2012-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.autoconfigure.elasticsearch.sniff;

import org.elasticsearch.client.sniff.NodesSniffer;
import org.springframework.boot.context.properties.ConfigurationProperties;

import java.time.Duration;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

/**
* Configuration properties for Elasticsearch sniffer.
*
*/
@ConfigurationProperties(prefix = "spring.elasticsearch.sniff")
public class ElasticsearchSnifferProperties {

/**
* Sniffer interval millis
*/
private Duration sniffIntervalMillis = Duration.ofMillis(1L);
Copy link
Member

Choose a reason for hiding this comment

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

A Duration property should not have mills (or any unit) in its name since users are free to chose a different unit if they want to.

Also, the default value is wrong, it should be 5 min.

/**
* Sniffer failure delay millis.
*/
private Duration sniffFailureDelayMillis = Duration.ofMillis(1L);
Copy link
Member

Choose a reason for hiding this comment

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

The default value is not 1ms but 1 minute.


public Duration getSniffIntervalMillis() {
return sniffIntervalMillis;
}

public void setSniffIntervalMillis(Duration sniffIntervalMillis) {
this.sniffIntervalMillis = sniffIntervalMillis;
}

public Duration getSniffFailureDelayMillis() {
return sniffFailureDelayMillis;
}

public void setSniffFailureDelayMillis(Duration sniffFailureDelayMillis) {
this.sniffFailureDelayMillis = sniffFailureDelayMillis;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright 2012-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.autoconfigure.elasticsearch.sniff;

import org.apache.http.client.config.RequestConfig;
import org.apache.http.impl.nio.client.HttpAsyncClientBuilder;
import org.elasticsearch.client.RestClientBuilder;
import org.elasticsearch.client.sniff.SnifferBuilder;

/**
* Callback interface that can be implemented by beans wishing to further customize the
* {@link org.elasticsearch.client.sniff.Sniffer} via a {@link org.elasticsearch.client.sniff.SnifferBuilder} whilst
* retaining default auto-configuration.
*
*/
@FunctionalInterface
public interface SnifferBuilderCustomizer {
Copy link
Member

Choose a reason for hiding this comment

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

We don't need this for a first pass. Those who want to configure the NodesSniffer can do so with their own Sniffer instance.


/**
* Customize the {@link org.elasticsearch.client.sniff.SnifferBuilder}.
* <p>
* Possibly overrides customizations made with the {@code "spring.elasticsearch.client.sniff"}
* configuration properties namespace. For more targeted changes, see

* @param builder the builder to customize
*/
void customize(SnifferBuilder builder);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/*
* Copyright 2012-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.autoconfigure.elasticsearch.sniff;

import org.apache.http.HttpHost;
import org.apache.http.auth.AuthScope;
import org.apache.http.auth.Credentials;
import org.apache.http.client.CredentialsProvider;
import org.apache.http.client.config.RequestConfig;
import org.apache.http.impl.nio.client.HttpAsyncClientBuilder;
import org.assertj.core.api.InstanceOfAssertFactories;
import org.elasticsearch.action.get.GetRequest;
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.client.*;
import org.elasticsearch.client.sniff.ElasticsearchNodesSniffer;
import org.elasticsearch.client.sniff.NodesSniffer;
import org.elasticsearch.client.sniff.Sniffer;
import org.elasticsearch.client.sniff.SnifferBuilder;
import org.junit.jupiter.api.Test;
import org.springframework.boot.autoconfigure.AutoConfigurations;
import org.springframework.boot.autoconfigure.elasticsearch.ElasticsearchRestClientAutoConfiguration;
import org.springframework.boot.autoconfigure.elasticsearch.sniff.SnifferBuilderCustomizer;
import org.springframework.boot.test.context.runner.ApplicationContextRunner;
import org.springframework.boot.testsupport.testcontainers.DockerImageNames;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.testcontainers.elasticsearch.ElasticsearchContainer;
import org.testcontainers.junit.jupiter.Container;
import org.testcontainers.junit.jupiter.Testcontainers;

import java.io.IOException;
import java.time.Duration;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;

/**
* Tests for {@link org.springframework.boot.autoconfigure.elasticsearch.sniff.ElasticsearchSnifferAutoConfiguration}.
*
* @author Asha Somayajula
*/
@Testcontainers(disabledWithoutDocker = true)
class ElasticsearchSnifferAutoConfigurationTests {
Copy link
Member

Choose a reason for hiding this comment

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

This should move to the main ElasticsearchRestClientAutoConfigurationTests. A test should be added with a filtered class loader on the sniffer library to validate things are working as expected.


@Container
static final ElasticsearchContainer elasticsearch = new ElasticsearchContainer(DockerImageNames.elasticsearch())
.withStartupAttempts(5).withStartupTimeout(Duration.ofMinutes(10));

private final ApplicationContextRunner contextRunner = new ApplicationContextRunner()
.withConfiguration(AutoConfigurations.of(ElasticsearchRestClientAutoConfiguration.class));

@Test
void configureShouldOnlyCreateSnifferInstance() {
this.contextRunner.run((context) -> assertThat(context).doesNotHaveBean(RestClient.class)
.hasSingleBean(RestHighLevelClient.class));
this.contextRunner.run((context) -> assertThat(context).hasSingleBean(Sniffer.class));
}

@Configuration(proxyBeanMethods = false)
static class BuilderCustomizerConfiguration {

@Bean
SnifferBuilderCustomizer myCustomizer() {
return new SnifferBuilderCustomizer() {

@Override
public void customize(SnifferBuilder builder) {
builder.setSniffAfterFailureDelayMillis(100);
builder.setSniffIntervalMillis(100);
NodesSniffer nodesSniffer = new NodesSniffer() {
@Override
public List<Node> sniff() throws IOException {
return null;
}
};

builder.setNodesSniffer(nodesSniffer);
}
};
};
}
}