Merge pull request #23 from hivemq/improvement/13749-improve-logging/…

…latest

Log discovered and gone addresses only once

gradle.properties

@@ -1,4 +1,4 @@
-version=4.2.3
+version=4.2.4
 #
 # main dependencies
 #

src/main/java/com/hivemq/extensions/cluster/discovery/dns/DnsDiscoveryCallback.java

@@ -34,15 +34,19 @@
 
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 
+import static com.hivemq.extensions.cluster.discovery.dns.ExtensionConstants.EXTENSION_NAME;
+
 /**
  * Cluster discovery using DNS resolution of round-robin A records.
  * Uses non-blocking netty API for DNS resolution, reads discovery parameters as environment variables.
@@ -60,11 +64,13 @@ class DnsDiscoveryCallback implements ClusterDiscoveryCallback {
     private final @NotNull InetAddressValidator addressValidator;
 
     private final @NotNull AtomicInteger addressesCount = new AtomicInteger(0);
+    private final @NotNull AtomicReference<List<String>> foundHostsRef = new AtomicReference<>(List.of());
 
     private @Nullable ClusterNodeAddress ownAddress;
 
     DnsDiscoveryCallback(
-            final @NotNull DnsDiscoveryConfigExtended configuration, final @NotNull DnsDiscoveryMetrics metrics) {
+            final @NotNull DnsDiscoveryConfigExtended configuration,
+            final @NotNull DnsDiscoveryMetrics metrics) {
         this.eventLoopGroup = new NioEventLoopGroup();
         this.addressValidator = InetAddressValidator.getInstance();
         this.configuration = configuration;
@@ -103,17 +109,20 @@ private void loadClusterNodeAddresses(final @NotNull ClusterDiscoveryOutput clus
                 metrics.getQuerySuccessCount().inc();
             }
         } catch (final TimeoutException | InterruptedException e) {
-            log.error("Timeout while getting other node addresses");
+            log.error("{}: Timeout while getting other node addresses.", EXTENSION_NAME);
             metrics.getQueryFailedCount().inc();
             addressesCount.set(0);
         }
     }
 
     private @Nullable List<ClusterNodeAddress> loadOtherNodes() throws TimeoutException, InterruptedException {
+        if (ownAddress == null) {
+            return null;
+        }
 
         final Optional<String> discoveryAddress = configuration.getDiscoveryAddress();
         if (discoveryAddress.isEmpty()) {
-            log.warn("Discovery address not set, skipping dns query.");
+            log.warn("{}: Discovery address not set, skipping DNS query.", EXTENSION_NAME);
             return null;
         }
         final int discoveryTimeout = configuration.getResolutionTimeout();
@@ -129,24 +138,37 @@ private void loadClusterNodeAddresses(final @NotNull ClusterDiscoveryOutput clus
                 inetSocketAddress)));
 
         try (final DnsNameResolver resolver = dnsNameResolverBuilder.build()) {
-
             final Future<List<InetAddress>> addresses = resolver.resolveAll(discoveryAddress.get());
             final List<ClusterNodeAddress> clusterNodeAddresses =
                     addresses.get(discoveryTimeout, TimeUnit.SECONDS)
                             .stream()
-                            // Skip any possibly unresolved elements
+                            // skip any possibly unresolved elements
                             .filter(Objects::nonNull)
-                            // Check if the discoveryAddress address we got from the DNS is a valid IP address
+                            // check if the discoveryAddress address we got from the DNS is a valid IP address
                             .filter((address) -> addressValidator.isValid(address.getHostAddress()))
                             .map((address) -> new ClusterNodeAddress(address.getHostAddress(), ownAddress.getPort()))
                             .collect(Collectors.toList());
 
-            clusterNodeAddresses.forEach((address) -> log.debug("Found address: '{}'", address.getHost()));
+            final List<String> foundHosts = new ArrayList<>();
+            final List<String> lastFoundHosts = foundHostsRef.get();
+            clusterNodeAddresses.forEach((address) -> {
+                final String host = address.getHost();
+                foundHosts.add(host);
+                if (!lastFoundHosts.contains(host)) {
+                    log.debug("{}: Discovered new address '{}'.", EXTENSION_NAME, host);
+                }
+            });
+            lastFoundHosts.forEach(host -> {
+                if (!foundHosts.contains(host)) {
+                    log.debug("{}: Discovered address '{}' is gone.", EXTENSION_NAME, host);
+                }
+            });
+            foundHostsRef.set(foundHosts);
             addressesCount.set(clusterNodeAddresses.size());
 
             return clusterNodeAddresses;
         } catch (final ExecutionException ex) {
-            log.error("Failed to resolve DNS record for address '{}'.", discoveryAddress, ex);
+            log.error("{}: Failed to resolve DNS record for address '{}'.", EXTENSION_NAME, discoveryAddress, ex);
             metrics.getQueryFailedCount().inc();
             addressesCount.set(0);
         }

src/main/java/com/hivemq/extensions/cluster/discovery/dns/DnsDiscoveryExtensionMain.java

@@ -31,7 +31,7 @@
 import java.io.File;
 
 /**
- * This is the main class of the dns discovery extension, which is instantiated during the HiveMQ start up process.
+ * This is the main class of the DNS discovery extension, which is instantiated during the HiveMQ start up process.
  *
  * @author Anja Helmbrecht-Schaar
  */

src/main/java/com/hivemq/extensions/cluster/discovery/dns/ExtensionConstants.java

@@ -0,0 +1,26 @@
+/*
+ * Copyright 2018-present HiveMQ GmbH
+ *
+ * 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
+ *
+ *     http://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 com.hivemq.extensions.cluster.discovery.dns;
+
+import com.hivemq.extension.sdk.api.annotations.NotNull;
+
+public final class ExtensionConstants {
+
+    public static final @NotNull String EXTENSION_NAME = "HiveMQ DNS Cluster Discovery Extension";
+
+    private ExtensionConstants() {
+    }
+}

src/main/java/com/hivemq/extensions/cluster/discovery/dns/configuration/ConfigurationFileReader.java

@@ -41,7 +41,7 @@ public ConfigurationFileReader(final @NotNull File extensionHomeFolder) {
     }
 
     /**
-     * Method that loads and reloads the configuration for the dns discovery properties, by (re)creating the
+     * Method that loads and reloads the configuration for the DNS discovery properties, by (re)creating the
      * configuration.
      *
      * @return DnsDiscoveryConfigFile The configuration from the config file or default values.

src/main/java/com/hivemq/extensions/cluster/discovery/dns/configuration/DnsDiscoveryConfigExtended.java

@@ -23,6 +23,8 @@
 import java.net.InetSocketAddress;
 import java.util.Optional;
 
+import static com.hivemq.extensions.cluster.discovery.dns.ExtensionConstants.EXTENSION_NAME;
+
 /**
  * Configuration class that encapsulates the dnsDiscoveryConfig to enable usage of environment Variables.
  *
@@ -60,12 +62,11 @@ public class DnsDiscoveryConfigExtended {
 
     void dnsServerAddress() {
         final String envDnsServerAddress = configEnvironment.getEnvDnsServerAddress();
-
         if (envDnsServerAddress != null && !envDnsServerAddress.isBlank()) {
             try {
                 dnsServerAddress = processDnsServerAddress(envDnsServerAddress);
             } catch (final Exception e) {
-                log.error("Could not read the dns server address from the environment variable.");
+                log.error("{}: Could not read the DNS server address from the environment variable.", EXTENSION_NAME);
                 throw new ConfigurationException(e);
             }
         } else {
@@ -75,10 +76,11 @@ void dnsServerAddress() {
                 if (propDnsServerAddress != null && !propDnsServerAddress.isBlank()) {
                     dnsServerAddress = processDnsServerAddress(propDnsServerAddress);
                 } else {
-                    log.debug("No dns server address was set in the configuration file or environment variable.");
+                    log.debug("{}: No DNS server address was set in the configuration file or environment variable.",
+                            EXTENSION_NAME);
                 }
             } catch (final Exception e) {
-                log.error("Could not read the dns server address from the properties file.");
+                log.error("{}: Could not read the DNS server address from the properties file.", EXTENSION_NAME);
                 throw new ConfigurationException(e);
             }
         }
@@ -91,7 +93,7 @@ void dnsServerAddress() {
             try {
                 port = Integer.parseInt(dnsServerAddress.split(":")[1]);
             } catch (final NumberFormatException e) {
-                log.error("The dns server address port could not be read. Taking default port 53.");
+                log.error("{}: The DNS server address port could not be read. Taking default port 53.", EXTENSION_NAME);
                 port = 53;
             }
             return new InetSocketAddress(address, port);
@@ -102,7 +104,6 @@ void dnsServerAddress() {
 
     void discoveryAddress() {
         final String envDiscoveryAddress = configEnvironment.getEnvDiscoveryAddress();
-
         if (envDiscoveryAddress != null && !envDiscoveryAddress.isEmpty()) {
             discoveryAddress = envDiscoveryAddress;
         } else {
@@ -111,80 +112,80 @@ void discoveryAddress() {
                 if (propDiscoveryAddress != null && !propDiscoveryAddress.isBlank()) {
                     discoveryAddress = propDiscoveryAddress;
                 } else {
-                    log.warn("No discovery address was set in the configuration file or environment variable.");
+                    log.warn("{}: No discovery address was set in the configuration file or environment variable.",
+                            EXTENSION_NAME);
                 }
             } catch (final Exception e) {
-                log.error("Could not read the discovery address from the properties file.");
+                log.error("{}: Could not read the discovery address from the properties file.", EXTENSION_NAME);
                 throw new ConfigurationException(e);
             }
         }
     }
 
     void resolutionTimeout() {
         final String envResolutionTimeout = configEnvironment.getEnvResolutionTimeout();
-
         if (envResolutionTimeout != null && !envResolutionTimeout.isEmpty()) {
             try {
                 resolutionTimeout = Integer.parseInt(envResolutionTimeout);
                 return;
             } catch (final NumberFormatException e) {
                 log.error(
-                        "Resolution timeout from env {} could not be parsed to int. Fallback to configuration value 'resolutionTimeout'.",
+                        "{}: Resolution timeout from env {} could not be parsed to int. Fallback to configuration value 'resolutionTimeout'.",
+                        EXTENSION_NAME,
                         DnsDiscoveryConfigEnvironment.DISCOVERY_TIMEOUT_ENV);
             }
         }
-
         try {
             final int propResolutionTimeout = configFile.getFileResolutionTimeout();
 
             if (propResolutionTimeout != -1) {
                 resolutionTimeout = propResolutionTimeout;
             } else {
                 log.debug(
-                        "No resolution timeout was set in the configuration file or environment variable. Defaulting to {}.",
+                        "{}: No resolution timeout was set in the configuration file or environment variable. Defaulting to {}.",
+                        EXTENSION_NAME,
                         resolutionTimeout);
             }
         } catch (final Exception e) {
-            log.error("Could not read the resolution timeout from the properties file.");
+            log.error("{}: Could not read the resolution timeout from the properties file.", EXTENSION_NAME);
             throw new ConfigurationException(e);
         }
     }
 
     void reloadInterval() {
         final String envReloadInterval = configEnvironment.getEnvReloadInterval();
-
         if (envReloadInterval != null && !envReloadInterval.isBlank()) {
             try {
                 reloadInterval = Integer.parseInt(envReloadInterval);
                 return;
             } catch (final NumberFormatException e) {
                 log.error(
-                        "Reload interval from env {} could not be parsed to int. Fallback to configuration value 'reloadInterval'.",
+                        "{}: Reload interval from env {} could not be parsed to int. Fallback to configuration value 'reloadInterval'.",
+                        EXTENSION_NAME,
                         DnsDiscoveryConfigEnvironment.DISCOVERY_RELOAD_INTERVAL_ENV);
             }
         }
-
         try {
             final int propReloadInterval = configFile.getFileReloadInterval();
-
             if (propReloadInterval != -1) {
                 reloadInterval = propReloadInterval;
             } else {
                 log.debug(
-                        "No reload interval was set in the configuration file or environment variable. Defaulting to {}.",
+                        "{}: No reload interval was set in the configuration file or environment variable. Defaulting to {}.",
+                        EXTENSION_NAME,
                         reloadInterval);
             }
         } catch (final Exception e) {
-            log.error("Could not read the reload interval from the properties file.");
+            log.error("{}: Could not read the reload interval from the properties file.", EXTENSION_NAME);
             throw new ConfigurationException(e);
         }
     }
 
     /**
-     * Getter for the dns server address. Its value is either from an environment
-     * variable or a properties configuration.
+     * Getter for the DNS server address. Its value is either from an environment
+     * variable or a property configuration.
      *
-     * @return String - the dns server address
+     * @return String - the DNS server address
      */
     public @NotNull Optional<InetSocketAddress> getDnsServerAddress() {
         return Optional.ofNullable(dnsServerAddress);