From 8b07beb79646088363149af723149de37424854a Mon Sep 17 00:00:00 2001 From: Brian Laub Date: Tue, 27 Feb 2024 16:56:02 -0500 Subject: [PATCH 01/14] try to log error messages when getaddrinfo fails --- .../clients/DefaultDialogueDnsResolver.java | 58 ++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java index d4f4f173d..a63615d7c 100644 --- a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java +++ b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java @@ -16,14 +16,18 @@ package com.palantir.dialogue.clients; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.palantir.dialogue.core.DialogueDnsResolver; import com.palantir.logsafe.Preconditions; +import com.palantir.logsafe.SafeArg; import com.palantir.logsafe.UnsafeArg; import com.palantir.logsafe.logger.SafeLogger; import com.palantir.logsafe.logger.SafeLoggerFactory; import java.net.InetAddress; import java.net.UnknownHostException; +import java.util.Map; +import org.immutables.value.Value; enum DefaultDialogueDnsResolver implements DialogueDnsResolver { INSTANCE; @@ -41,8 +45,60 @@ public ImmutableSet resolve(String hostname) { } return ImmutableSet.copyOf(results); } catch (UnknownHostException e) { - log.warn("Unknown host '{}'", UnsafeArg.of("hostname", hostname), e); + ExtractedGaiError gaiError = extractGaiErrorString(e); + log.warn( + "Unknown host '{}'", + SafeArg.of("gaiErrorMessage", gaiError.getErrorMessage()), + SafeArg.of("gaiErrorType", gaiError.getErrorType()), + UnsafeArg.of("hostname", hostname), + e); return ImmutableSet.of(); } } + + // these strings were taken from glibc-2.39, but likely have not changed in quite a while + // strings may be different on BSD systems like macos + // TODO(dns): update this list to try to match against known strings on other platforms + private static final Map EXPECTED_GAI_ERROR_STRINGS = ImmutableMap.builder() + .put("Address family for hostname not supported", "EAI_ADDRFAMILY") + .put("Temporary failure in name resolution", "EAI_AGAIN") + .put("Bad value for ai_flags", "EAI_BADFLAGS") + .put("Non-recoverable failure in name resolution", "EAI_FAIL") + .put("ai_family not supported", "EAI_FAMILY") + .put("Memory allocation failure", "EAI_MEMORY") + .put("No address associated with hostname", "EAI_NODATA") + .put("Name or service not known", "EAI_NONAME") + .put("Servname not supported for ai_socktype", "EAI_SERVICE") + .put("ai_socktype not supported", "EAI_SOCKTYPE") + .put("System error", "EAI_SYSTEM") + .put("Processing request in progress", "EAI_INPROGRESS") + .put("Request canceled", "EAI_CANCELED") + .put("Request not canceled", "EAI_NOTCANCELED") + .put("All requests done", "EAI_ALLDONE") + .put("Interrupted by a signal", "EAI_INTR") + .put("Parameter string not correctly encoded", "EAI_IDN_ENCODE") + .put("Result too large for supplied buffer", "EAI_OVERFLOW") + .buildOrThrow(); + + @Value.Immutable + interface ExtractedGaiError { + @Value.Parameter + String getErrorMessage(); + + @Value.Parameter + String getErrorType(); + } + + private static ExtractedGaiError extractGaiErrorString(UnknownHostException exception) { + try { + for (Map.Entry entry : EXPECTED_GAI_ERROR_STRINGS.entrySet()) { + if (exception.getMessage() != null && exception.getMessage().contains(entry.getKey())) { + return ImmutableExtractedGaiError.of(entry.getKey(), entry.getValue()); + } + } + } catch (Exception e) { + return ImmutableExtractedGaiError.of("unknown", "unknown"); + } + return ImmutableExtractedGaiError.of("unknown", "unknown"); + } } From 9fa92eef817353b114fa47b18ab7d0e7254efc65 Mon Sep 17 00:00:00 2001 From: Brian Laub Date: Tue, 27 Feb 2024 18:43:52 -0500 Subject: [PATCH 02/14] treat UnknownHostException from cache separately --- .../clients/DefaultDialogueDnsResolver.java | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java index a63615d7c..acf60bb7d 100644 --- a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java +++ b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java @@ -90,15 +90,27 @@ interface ExtractedGaiError { } private static ExtractedGaiError extractGaiErrorString(UnknownHostException exception) { + if (exception == null) { + return ImmutableExtractedGaiError.of("null", "null"); + } + try { - for (Map.Entry entry : EXPECTED_GAI_ERROR_STRINGS.entrySet()) { - if (exception.getMessage() != null && exception.getMessage().contains(entry.getKey())) { - return ImmutableExtractedGaiError.of(entry.getKey(), entry.getValue()); + StackTraceElement[] trace = exception.getStackTrace(); + if (trace.length > 0) { + StackTraceElement top = trace[0]; + if ("java.net.InetAddress$CachedLookup".equals(top.getClassName())) { + return ImmutableExtractedGaiError.of("cached", "cached"); + } + + for (Map.Entry entry : EXPECTED_GAI_ERROR_STRINGS.entrySet()) { + if (exception.getMessage() != null && exception.getMessage().contains(entry.getKey())) { + return ImmutableExtractedGaiError.of(entry.getKey(), entry.getValue()); + } } } + return ImmutableExtractedGaiError.of("unknown", "unknown"); } catch (Exception e) { return ImmutableExtractedGaiError.of("unknown", "unknown"); } - return ImmutableExtractedGaiError.of("unknown", "unknown"); } } From 36f65028283556562176a636e2384e5472657270 Mon Sep 17 00:00:00 2001 From: Brian Laub Date: Wed, 28 Feb 2024 10:36:02 -0500 Subject: [PATCH 03/14] use an enum for GaiError --- .../clients/DefaultDialogueDnsResolver.java | 90 ++++++++++--------- 1 file changed, 50 insertions(+), 40 deletions(-) diff --git a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java index acf60bb7d..5f978fc3f 100644 --- a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java +++ b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java @@ -16,7 +16,6 @@ package com.palantir.dialogue.clients; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.palantir.dialogue.core.DialogueDnsResolver; import com.palantir.logsafe.Preconditions; @@ -26,8 +25,7 @@ import com.palantir.logsafe.logger.SafeLoggerFactory; import java.net.InetAddress; import java.net.UnknownHostException; -import java.util.Map; -import org.immutables.value.Value; +import java.util.Optional; enum DefaultDialogueDnsResolver implements DialogueDnsResolver { INSTANCE; @@ -45,11 +43,11 @@ public ImmutableSet resolve(String hostname) { } return ImmutableSet.copyOf(results); } catch (UnknownHostException e) { - ExtractedGaiError gaiError = extractGaiErrorString(e); + GaiError gaiError = extractGaiErrorString(e); log.warn( "Unknown host '{}'", - SafeArg.of("gaiErrorMessage", gaiError.getErrorMessage()), - SafeArg.of("gaiErrorType", gaiError.getErrorType()), + SafeArg.of("gaiErrorMessage", gaiError.getErrorMessage().orElse(gaiError.name())), + SafeArg.of("gaiErrorType", gaiError.name()), UnsafeArg.of("hostname", hostname), e); return ImmutableSet.of(); @@ -59,39 +57,47 @@ public ImmutableSet resolve(String hostname) { // these strings were taken from glibc-2.39, but likely have not changed in quite a while // strings may be different on BSD systems like macos // TODO(dns): update this list to try to match against known strings on other platforms - private static final Map EXPECTED_GAI_ERROR_STRINGS = ImmutableMap.builder() - .put("Address family for hostname not supported", "EAI_ADDRFAMILY") - .put("Temporary failure in name resolution", "EAI_AGAIN") - .put("Bad value for ai_flags", "EAI_BADFLAGS") - .put("Non-recoverable failure in name resolution", "EAI_FAIL") - .put("ai_family not supported", "EAI_FAMILY") - .put("Memory allocation failure", "EAI_MEMORY") - .put("No address associated with hostname", "EAI_NODATA") - .put("Name or service not known", "EAI_NONAME") - .put("Servname not supported for ai_socktype", "EAI_SERVICE") - .put("ai_socktype not supported", "EAI_SOCKTYPE") - .put("System error", "EAI_SYSTEM") - .put("Processing request in progress", "EAI_INPROGRESS") - .put("Request canceled", "EAI_CANCELED") - .put("Request not canceled", "EAI_NOTCANCELED") - .put("All requests done", "EAI_ALLDONE") - .put("Interrupted by a signal", "EAI_INTR") - .put("Parameter string not correctly encoded", "EAI_IDN_ENCODE") - .put("Result too large for supplied buffer", "EAI_OVERFLOW") - .buildOrThrow(); + private enum GaiError { + EAI_ADDRFAMILY("Address family for hostname not supported"), + EAI_AGAIN("Temporary failure in name resolution"), + EAI_BADFLAGS("Bad value for ai_flags"), + EAI_FAIL("Non-recoverable failure in name resolution"), + EAI_FAMILY("ai_family not supported"), + EAI_MEMORY("Memory allocation failure"), + EAI_NODATA("No address associated with hostname"), + EAI_NONAME("Name or service not known"), + EAI_SERVICE("Servname not supported for ai_socktype"), + EAI_SOCKTYPE("ai_socktype not supported"), + EAI_SYSTEM("System error"), + EAI_INPROGRESS("Processing request in progress"), + EAI_CANCELED("Request canceled"), + EAI_NOTCANCELED("Request not canceled"), + EAI_ALLDONE("All requests done"), + EAI_INTR("Interrupted by a signal"), + EAI_IDN_ENCODE("Parameter string not correctly encoded"), + EAI_OVERFLOW("Result too large for supplied buffer"), + CACHED(), + NULL(), + UNKNOWN(); - @Value.Immutable - interface ExtractedGaiError { - @Value.Parameter - String getErrorMessage(); + private final Optional errorMessage; - @Value.Parameter - String getErrorType(); + GaiError() { + this.errorMessage = Optional.empty(); + } + + GaiError(String errorMessage) { + this.errorMessage = Optional.of(errorMessage); + } + + Optional getErrorMessage() { + return errorMessage; + } } - private static ExtractedGaiError extractGaiErrorString(UnknownHostException exception) { + private static GaiError extractGaiErrorString(UnknownHostException exception) { if (exception == null) { - return ImmutableExtractedGaiError.of("null", "null"); + return GaiError.NULL; } try { @@ -99,18 +105,22 @@ private static ExtractedGaiError extractGaiErrorString(UnknownHostException exce if (trace.length > 0) { StackTraceElement top = trace[0]; if ("java.net.InetAddress$CachedLookup".equals(top.getClassName())) { - return ImmutableExtractedGaiError.of("cached", "cached"); + return GaiError.CACHED; } - for (Map.Entry entry : EXPECTED_GAI_ERROR_STRINGS.entrySet()) { - if (exception.getMessage() != null && exception.getMessage().contains(entry.getKey())) { - return ImmutableExtractedGaiError.of(entry.getKey(), entry.getValue()); + for (GaiError error : GaiError.values()) { + if (error.getErrorMessage().isPresent()) { + if (exception + .getMessage() + .contains(error.getErrorMessage().get())) { + return error; + } } } } - return ImmutableExtractedGaiError.of("unknown", "unknown"); + return GaiError.UNKNOWN; } catch (Exception e) { - return ImmutableExtractedGaiError.of("unknown", "unknown"); + return GaiError.UNKNOWN; } } } From 92a2d79ccf3ac7e65e29feb9f719e699dcfda20d Mon Sep 17 00:00:00 2001 From: Brian Laub Date: Wed, 28 Feb 2024 10:36:46 -0500 Subject: [PATCH 04/14] remove unnecessary null check --- .../dialogue/clients/DefaultDialogueDnsResolver.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java index 5f978fc3f..04144bb56 100644 --- a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java +++ b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java @@ -77,7 +77,6 @@ private enum GaiError { EAI_IDN_ENCODE("Parameter string not correctly encoded"), EAI_OVERFLOW("Result too large for supplied buffer"), CACHED(), - NULL(), UNKNOWN(); private final Optional errorMessage; @@ -96,10 +95,6 @@ Optional getErrorMessage() { } private static GaiError extractGaiErrorString(UnknownHostException exception) { - if (exception == null) { - return GaiError.NULL; - } - try { StackTraceElement[] trace = exception.getStackTrace(); if (trace.length > 0) { From 79056c4941ac3f72c5fe25fe27427c9ae04b96c9 Mon Sep 17 00:00:00 2001 From: Brian Laub Date: Wed, 28 Feb 2024 10:39:28 -0500 Subject: [PATCH 05/14] refactor a bit --- .../clients/DefaultDialogueDnsResolver.java | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java index 04144bb56..a12b2afd6 100644 --- a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java +++ b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java @@ -19,6 +19,7 @@ import com.google.common.collect.ImmutableSet; import com.palantir.dialogue.core.DialogueDnsResolver; import com.palantir.logsafe.Preconditions; +import com.palantir.logsafe.Safe; import com.palantir.logsafe.SafeArg; import com.palantir.logsafe.UnsafeArg; import com.palantir.logsafe.logger.SafeLogger; @@ -44,12 +45,7 @@ public ImmutableSet resolve(String hostname) { return ImmutableSet.copyOf(results); } catch (UnknownHostException e) { GaiError gaiError = extractGaiErrorString(e); - log.warn( - "Unknown host '{}'", - SafeArg.of("gaiErrorMessage", gaiError.getErrorMessage().orElse(gaiError.name())), - SafeArg.of("gaiErrorType", gaiError.name()), - UnsafeArg.of("hostname", hostname), - e); + log.warn("Unknown host '{}'", SafeArg.of("gaiError", gaiError), UnsafeArg.of("hostname", hostname), e); return ImmutableSet.of(); } } @@ -89,8 +85,9 @@ private enum GaiError { this.errorMessage = Optional.of(errorMessage); } - Optional getErrorMessage() { - return errorMessage; + @Safe + String errorMessage() { + return errorMessage.orElse(name()); } } @@ -104,10 +101,8 @@ private static GaiError extractGaiErrorString(UnknownHostException exception) { } for (GaiError error : GaiError.values()) { - if (error.getErrorMessage().isPresent()) { - if (exception - .getMessage() - .contains(error.getErrorMessage().get())) { + if (error.errorMessage.isPresent()) { + if (exception.getMessage().contains(error.errorMessage.get())) { return error; } } From 377e143abd2b5117030ca33a0c291f89edd3cfdb Mon Sep 17 00:00:00 2001 From: Brian Laub Date: Wed, 28 Feb 2024 11:02:32 -0500 Subject: [PATCH 06/14] more changes --- .../dialogue/clients/DefaultDialogueDnsResolver.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java index a12b2afd6..45f00885e 100644 --- a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java +++ b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java @@ -45,7 +45,12 @@ public ImmutableSet resolve(String hostname) { return ImmutableSet.copyOf(results); } catch (UnknownHostException e) { GaiError gaiError = extractGaiErrorString(e); - log.warn("Unknown host '{}'", SafeArg.of("gaiError", gaiError), UnsafeArg.of("hostname", hostname), e); + log.warn( + "Unknown host '{}'", + SafeArg.of("gaiErrorType", gaiError.name()), + SafeArg.of("gaiErrorMessage", gaiError.errorMessage()), + UnsafeArg.of("hostname", hostname), + e); return ImmutableSet.of(); } } @@ -87,7 +92,7 @@ private enum GaiError { @Safe String errorMessage() { - return errorMessage.orElse(name()); + return errorMessage.orElseGet(this::name); } } @@ -102,7 +107,8 @@ private static GaiError extractGaiErrorString(UnknownHostException exception) { for (GaiError error : GaiError.values()) { if (error.errorMessage.isPresent()) { - if (exception.getMessage().contains(error.errorMessage.get())) { + if (exception.getMessage() != null + && exception.getMessage().contains(error.errorMessage.get())) { return error; } } From ca9e483a155656af62eafa2581a9f9cefc6b96cc Mon Sep 17 00:00:00 2001 From: Brian Laub Date: Wed, 28 Feb 2024 12:43:21 -0500 Subject: [PATCH 07/14] add metrics and tests --- dialogue-clients/metrics.md | 2 + .../clients/DefaultDialogueDnsResolver.java | 23 +++++++---- .../main/metrics/dialogue-core-metrics.yml | 6 +++ .../DefaultDialogueDnsResolverTest.java | 41 ++++++++++++++++++- 4 files changed, 63 insertions(+), 9 deletions(-) diff --git a/dialogue-clients/metrics.md b/dialogue-clients/metrics.md index 09fe0c72e..dccea84d8 100644 --- a/dialogue-clients/metrics.md +++ b/dialogue-clients/metrics.md @@ -68,6 +68,8 @@ Dialogue DNS metrics. - `success`: DNS resolution succeeded using `InetAddress.getAllByName`. - `fallback`: DNS resolution using the primary mechanism failed, however addresses were available in the fallback cache. - `failure`: No addresses could be resolved for the given hostname. +- `client.dns.lookupError` (meter): DNS resolver query failures. + - `error-type`: Describes the error type returned by getaddrinfo() when lookup fails. ### client.uri Dialogue URI parsing metrics. diff --git a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java index 45f00885e..507835f72 100644 --- a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java +++ b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java @@ -24,15 +24,20 @@ import com.palantir.logsafe.UnsafeArg; import com.palantir.logsafe.logger.SafeLogger; import com.palantir.logsafe.logger.SafeLoggerFactory; +import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; import java.net.InetAddress; import java.net.UnknownHostException; import java.util.Optional; -enum DefaultDialogueDnsResolver implements DialogueDnsResolver { - INSTANCE; - +final class DefaultDialogueDnsResolver implements DialogueDnsResolver { private static final SafeLogger log = SafeLoggerFactory.get(DefaultDialogueDnsResolver.class); + private final ClientDnsMetrics metrics; + + DefaultDialogueDnsResolver(TaggedMetricRegistry registry) { + this.metrics = ClientDnsMetrics.of(registry); + } + @Override public ImmutableSet resolve(String hostname) { Preconditions.checkNotNull(hostname, "hostname is required"); @@ -51,6 +56,7 @@ public ImmutableSet resolve(String hostname) { SafeArg.of("gaiErrorMessage", gaiError.errorMessage()), UnsafeArg.of("hostname", hostname), e); + metrics.lookupError(gaiError.name()).mark(); return ImmutableSet.of(); } } @@ -97,6 +103,10 @@ String errorMessage() { } private static GaiError extractGaiErrorString(UnknownHostException exception) { + if (exception.getMessage() == null) { + return GaiError.UNKNOWN; + } + try { StackTraceElement[] trace = exception.getStackTrace(); if (trace.length > 0) { @@ -106,11 +116,8 @@ private static GaiError extractGaiErrorString(UnknownHostException exception) { } for (GaiError error : GaiError.values()) { - if (error.errorMessage.isPresent()) { - if (exception.getMessage() != null - && exception.getMessage().contains(error.errorMessage.get())) { - return error; - } + if (error.errorMessage.isPresent() && exception.getMessage().contains(error.errorMessage.get())) { + return error; } } } diff --git a/dialogue-clients/src/main/metrics/dialogue-core-metrics.yml b/dialogue-clients/src/main/metrics/dialogue-core-metrics.yml index af8a4c9f0..e8f48022e 100644 --- a/dialogue-clients/src/main/metrics/dialogue-core-metrics.yml +++ b/dialogue-clients/src/main/metrics/dialogue-core-metrics.yml @@ -37,3 +37,9 @@ namespaces: - value: failure docs: No addresses could be resolved for the given hostname. docs: DNS resolver query metrics, on a per-hostname basis. + lookupError: + type: meter + tags: + - name: error-type + docs: Describes the error type returned by getaddrinfo() when lookup fails. + docs: DNS resolver query failures. diff --git a/dialogue-clients/src/test/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolverTest.java b/dialogue-clients/src/test/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolverTest.java index f7db06a3f..07b7943be 100644 --- a/dialogue-clients/src/test/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolverTest.java +++ b/dialogue-clients/src/test/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolverTest.java @@ -18,9 +18,13 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.assertj.core.api.Assumptions.assumeThat; import com.google.common.collect.ImmutableSet; +import com.palantir.dialogue.core.DialogueDnsResolver; import com.palantir.logsafe.exceptions.SafeNullPointerException; +import com.palantir.tritium.metrics.registry.DefaultTaggedMetricRegistry; +import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; import java.net.InetAddress; import java.net.UnknownHostException; import org.junit.jupiter.api.Test; @@ -66,7 +70,42 @@ void malformedIpv6_noBrackets() { assertThat(resolve("::z")).isEmpty(); } + @Test + void unknown_host() { + assumeThat(System.getProperty("os.name").toLowerCase().startsWith("linux")) + .describedAs("GAI Error Strings are only defined for Linux environments") + .isTrue(); + + TaggedMetricRegistry registry = new DefaultTaggedMetricRegistry(); + DialogueDnsResolver resolver = new DefaultDialogueDnsResolver(registry); + + String badHost = "alksdjflajsdlkfjalksjflkadjsf.com"; + ImmutableSet result = resolver.resolve(badHost); + + assertThat(result).isEmpty(); + ClientDnsMetrics metrics = ClientDnsMetrics.of(registry); + assertThat(metrics.lookupError("EAI_NONAME").getCount()).isGreaterThan(0); + } + + @Test + void unknown_host_from_cache() { + TaggedMetricRegistry registry = new DefaultTaggedMetricRegistry(); + DialogueDnsResolver resolver = new DefaultDialogueDnsResolver(registry); + + String badHost = "alksdjflajsdlkfjalksjflkadjsf.com"; + ImmutableSet result = resolver.resolve(badHost); + + assertThat(result).isEmpty(); + + // should resolve from cache + ImmutableSet result2 = resolver.resolve(badHost); + assertThat(result2).isEmpty(); + ClientDnsMetrics metrics = ClientDnsMetrics.of(registry); + assertThat(metrics.lookupError("CACHED").getCount()).isGreaterThan(0); + } + private static ImmutableSet resolve(String hostname) { - return DefaultDialogueDnsResolver.INSTANCE.resolve(hostname); + DialogueDnsResolver resolver = new DefaultDialogueDnsResolver(new DefaultTaggedMetricRegistry()); + return resolver.resolve(hostname); } } From 4ecfd6c98675bdea81d05141f30898c0a4feac2f Mon Sep 17 00:00:00 2001 From: Brian Laub Date: Wed, 28 Feb 2024 12:56:38 -0500 Subject: [PATCH 08/14] add missing change --- .../com/palantir/dialogue/clients/ReloadingClientFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/ReloadingClientFactory.java b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/ReloadingClientFactory.java index 103b98190..22ad614c4 100644 --- a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/ReloadingClientFactory.java +++ b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/ReloadingClientFactory.java @@ -161,7 +161,7 @@ default ConjureRuntime runtime() { @Value.Default default DialogueDnsResolver dnsResolver() { return new CachingFallbackDnsResolver( - new ProtocolVersionFilteringDialogueDnsResolver(DefaultDialogueDnsResolver.INSTANCE), + new ProtocolVersionFilteringDialogueDnsResolver(new DefaultDialogueDnsResolver(taggedMetrics())), taggedMetrics()); } From e1895dfc9538875bf1079f170f71e32a860f7834 Mon Sep 17 00:00:00 2001 From: Brian Laub Date: Wed, 28 Feb 2024 12:57:10 -0500 Subject: [PATCH 09/14] match hostname directly against exception message when checking for cached negative lookup --- .../clients/DefaultDialogueDnsResolver.java | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java index 507835f72..953ddfcab 100644 --- a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java +++ b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java @@ -27,6 +27,7 @@ import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; import java.net.InetAddress; import java.net.UnknownHostException; +import java.util.Objects; import java.util.Optional; final class DefaultDialogueDnsResolver implements DialogueDnsResolver { @@ -49,7 +50,7 @@ public ImmutableSet resolve(String hostname) { } return ImmutableSet.copyOf(results); } catch (UnknownHostException e) { - GaiError gaiError = extractGaiErrorString(e); + GaiError gaiError = extractGaiErrorString(e, hostname); log.warn( "Unknown host '{}'", SafeArg.of("gaiErrorType", gaiError.name()), @@ -102,23 +103,19 @@ String errorMessage() { } } - private static GaiError extractGaiErrorString(UnknownHostException exception) { + private static GaiError extractGaiErrorString(UnknownHostException exception, String requestedHostname) { if (exception.getMessage() == null) { return GaiError.UNKNOWN; } try { - StackTraceElement[] trace = exception.getStackTrace(); - if (trace.length > 0) { - StackTraceElement top = trace[0]; - if ("java.net.InetAddress$CachedLookup".equals(top.getClassName())) { - return GaiError.CACHED; - } + if (Objects.equals(requestedHostname, exception.getMessage())) { + return GaiError.CACHED; + } - for (GaiError error : GaiError.values()) { - if (error.errorMessage.isPresent() && exception.getMessage().contains(error.errorMessage.get())) { - return error; - } + for (GaiError error : GaiError.values()) { + if (error.errorMessage.isPresent() && exception.getMessage().contains(error.errorMessage.get())) { + return error; } } return GaiError.UNKNOWN; From 8fb49639bbcec39c8aa1d57f94b1b06812078918 Mon Sep 17 00:00:00 2001 From: Brian Laub Date: Wed, 28 Feb 2024 13:03:21 -0500 Subject: [PATCH 10/14] fixes --- dialogue-clients/metrics.md | 2 +- .../dialogue/clients/DefaultDialogueDnsResolver.java | 6 +++--- .../src/main/metrics/dialogue-core-metrics.yml | 2 +- .../clients/DefaultDialogueDnsResolverTest.java | 10 ++++++---- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/dialogue-clients/metrics.md b/dialogue-clients/metrics.md index dccea84d8..80353d894 100644 --- a/dialogue-clients/metrics.md +++ b/dialogue-clients/metrics.md @@ -68,7 +68,7 @@ Dialogue DNS metrics. - `success`: DNS resolution succeeded using `InetAddress.getAllByName`. - `fallback`: DNS resolution using the primary mechanism failed, however addresses were available in the fallback cache. - `failure`: No addresses could be resolved for the given hostname. -- `client.dns.lookupError` (meter): DNS resolver query failures. +- `client.dns.failure` (meter): DNS resolver query failures. - `error-type`: Describes the error type returned by getaddrinfo() when lookup fails. ### client.uri diff --git a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java index 953ddfcab..4484f69a2 100644 --- a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java +++ b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java @@ -50,14 +50,14 @@ public ImmutableSet resolve(String hostname) { } return ImmutableSet.copyOf(results); } catch (UnknownHostException e) { - GaiError gaiError = extractGaiErrorString(e, hostname); + GaiError gaiError = extractGaiError(e, hostname); log.warn( "Unknown host '{}'", SafeArg.of("gaiErrorType", gaiError.name()), SafeArg.of("gaiErrorMessage", gaiError.errorMessage()), UnsafeArg.of("hostname", hostname), e); - metrics.lookupError(gaiError.name()).mark(); + metrics.failure(gaiError.name()).mark(); return ImmutableSet.of(); } } @@ -103,7 +103,7 @@ String errorMessage() { } } - private static GaiError extractGaiErrorString(UnknownHostException exception, String requestedHostname) { + private static GaiError extractGaiError(UnknownHostException exception, String requestedHostname) { if (exception.getMessage() == null) { return GaiError.UNKNOWN; } diff --git a/dialogue-clients/src/main/metrics/dialogue-core-metrics.yml b/dialogue-clients/src/main/metrics/dialogue-core-metrics.yml index e8f48022e..2e614a8e6 100644 --- a/dialogue-clients/src/main/metrics/dialogue-core-metrics.yml +++ b/dialogue-clients/src/main/metrics/dialogue-core-metrics.yml @@ -37,7 +37,7 @@ namespaces: - value: failure docs: No addresses could be resolved for the given hostname. docs: DNS resolver query metrics, on a per-hostname basis. - lookupError: + failure: type: meter tags: - name: error-type diff --git a/dialogue-clients/src/test/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolverTest.java b/dialogue-clients/src/test/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolverTest.java index 07b7943be..10bc61060 100644 --- a/dialogue-clients/src/test/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolverTest.java +++ b/dialogue-clients/src/test/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolverTest.java @@ -27,6 +27,7 @@ import com.palantir.tritium.metrics.registry.TaggedMetricRegistry; import java.net.InetAddress; import java.net.UnknownHostException; +import java.util.UUID; import org.junit.jupiter.api.Test; class DefaultDialogueDnsResolverTest { @@ -84,24 +85,25 @@ void unknown_host() { assertThat(result).isEmpty(); ClientDnsMetrics metrics = ClientDnsMetrics.of(registry); - assertThat(metrics.lookupError("EAI_NONAME").getCount()).isGreaterThan(0); + assertThat(metrics.failure("EAI_NONAME").getCount()).isEqualTo(1); } @Test void unknown_host_from_cache() { TaggedMetricRegistry registry = new DefaultTaggedMetricRegistry(); DialogueDnsResolver resolver = new DefaultDialogueDnsResolver(registry); + ClientDnsMetrics metrics = ClientDnsMetrics.of(registry); - String badHost = "alksdjflajsdlkfjalksjflkadjsf.com"; + String badHost = UUID.randomUUID() + ".palantir.com"; ImmutableSet result = resolver.resolve(badHost); assertThat(result).isEmpty(); + assertThat(metrics.failure("EAI_NONAME").getCount()).isEqualTo(1); // should resolve from cache ImmutableSet result2 = resolver.resolve(badHost); assertThat(result2).isEmpty(); - ClientDnsMetrics metrics = ClientDnsMetrics.of(registry); - assertThat(metrics.lookupError("CACHED").getCount()).isGreaterThan(0); + assertThat(metrics.failure("CACHED").getCount()).isGreaterThan(0); } private static ImmutableSet resolve(String hostname) { From 57fa8d8f7223020bc03513c69ae54074515312bd Mon Sep 17 00:00:00 2001 From: Brian Laub Date: Wed, 28 Feb 2024 13:06:23 -0500 Subject: [PATCH 11/14] fix test --- .../dialogue/clients/DefaultDialogueDnsResolverTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dialogue-clients/src/test/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolverTest.java b/dialogue-clients/src/test/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolverTest.java index 10bc61060..6ec9f2979 100644 --- a/dialogue-clients/src/test/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolverTest.java +++ b/dialogue-clients/src/test/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolverTest.java @@ -80,7 +80,7 @@ void unknown_host() { TaggedMetricRegistry registry = new DefaultTaggedMetricRegistry(); DialogueDnsResolver resolver = new DefaultDialogueDnsResolver(registry); - String badHost = "alksdjflajsdlkfjalksjflkadjsf.com"; + String badHost = UUID.randomUUID() + ".palantir.com"; ImmutableSet result = resolver.resolve(badHost); assertThat(result).isEmpty(); From b7590d25b829ca47ff83d917870ad2df5079a25c Mon Sep 17 00:00:00 2001 From: Brian Laub Date: Wed, 28 Feb 2024 13:07:00 -0500 Subject: [PATCH 12/14] fix test again --- .../dialogue/clients/DefaultDialogueDnsResolverTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dialogue-clients/src/test/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolverTest.java b/dialogue-clients/src/test/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolverTest.java index 6ec9f2979..b739726ac 100644 --- a/dialogue-clients/src/test/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolverTest.java +++ b/dialogue-clients/src/test/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolverTest.java @@ -103,7 +103,7 @@ void unknown_host_from_cache() { // should resolve from cache ImmutableSet result2 = resolver.resolve(badHost); assertThat(result2).isEmpty(); - assertThat(metrics.failure("CACHED").getCount()).isGreaterThan(0); + assertThat(metrics.failure("CACHED").getCount()).isEqualTo(1); } private static ImmutableSet resolve(String hostname) { From 71c251f9aed46917c26d7ffa32fc7db676cddce2 Mon Sep 17 00:00:00 2001 From: Brian Laub Date: Wed, 28 Feb 2024 13:11:38 -0500 Subject: [PATCH 13/14] fix log formatting --- .../palantir/dialogue/clients/DefaultDialogueDnsResolver.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java index 4484f69a2..98528e439 100644 --- a/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java +++ b/dialogue-clients/src/main/java/com/palantir/dialogue/clients/DefaultDialogueDnsResolver.java @@ -52,7 +52,7 @@ public ImmutableSet resolve(String hostname) { } catch (UnknownHostException e) { GaiError gaiError = extractGaiError(e, hostname); log.warn( - "Unknown host '{}'", + "Unknown host '{}'. {}: {}", SafeArg.of("gaiErrorType", gaiError.name()), SafeArg.of("gaiErrorMessage", gaiError.errorMessage()), UnsafeArg.of("hostname", hostname), From c48c3d7deb16ed5a488e605cceeb734cc8cd4338 Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Wed, 28 Feb 2024 18:43:19 +0000 Subject: [PATCH 14/14] Add generated changelog entries --- changelog/@unreleased/pr-2179.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/@unreleased/pr-2179.v2.yml diff --git a/changelog/@unreleased/pr-2179.v2.yml b/changelog/@unreleased/pr-2179.v2.yml new file mode 100644 index 000000000..697c4b87e --- /dev/null +++ b/changelog/@unreleased/pr-2179.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: try to log error messages when getaddrinfo fails + links: + - https://github.com/palantir/dialogue/pull/2179