-
Notifications
You must be signed in to change notification settings - Fork 16
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 more specific metrics and logging for DNS lookup failures #2179
Changes from 10 commits
8b07beb
9fa92ee
36f6502
92a2d79
79056c4
377e143
ca9e483
4ecfd6c
e1895df
8fb4963
57fa8d8
b7590d2
71c251f
c48c3d7
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 |
---|---|---|
|
@@ -18,11 +18,16 @@ | |
|
||
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 java.util.UUID; | ||
import org.junit.jupiter.api.Test; | ||
|
||
class DefaultDialogueDnsResolverTest { | ||
|
@@ -66,7 +71,43 @@ 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"; | ||
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. Anyone could pay ~$10 to break this test. Let's use something like: String badHost = UUID.randomUUID() + ".palantir.com"; 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. fixed 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 think that only went into one of the two tests using this pattern, we need to update both |
||
ImmutableSet<InetAddress> result = resolver.resolve(badHost); | ||
|
||
assertThat(result).isEmpty(); | ||
ClientDnsMetrics metrics = ClientDnsMetrics.of(registry); | ||
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 = UUID.randomUUID() + ".palantir.com"; | ||
ImmutableSet<InetAddress> result = resolver.resolve(badHost); | ||
|
||
assertThat(result).isEmpty(); | ||
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. let's add 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. added |
||
assertThat(metrics.failure("EAI_NONAME").getCount()).isEqualTo(1); | ||
|
||
// should resolve from cache | ||
ImmutableSet<InetAddress> result2 = resolver.resolve(badHost); | ||
assertThat(result2).isEmpty(); | ||
assertThat(metrics.failure("CACHED").getCount()).isGreaterThan(0); | ||
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.
|
||
} | ||
|
||
private static ImmutableSet<InetAddress> resolve(String hostname) { | ||
return DefaultDialogueDnsResolver.INSTANCE.resolve(hostname); | ||
DialogueDnsResolver resolver = new DefaultDialogueDnsResolver(new DefaultTaggedMetricRegistry()); | ||
return resolver.resolve(hostname); | ||
} | ||
} |
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.