diff --git a/prober/build.gradle b/prober/build.gradle index 3652390d0..3c343dccd 100644 --- a/prober/build.gradle +++ b/prober/build.gradle @@ -25,6 +25,7 @@ dependencies { compile deps['com.google.dagger:dagger'] compile deps['com.google.flogger:flogger'] compile deps['com.google.guava:guava'] + compile deps['com.google.monitoring-client:metrics'] compile deps['io.netty:netty-buffer'] compile deps['io.netty:netty-codec-http'] compile deps['io.netty:netty-codec'] @@ -41,10 +42,12 @@ dependencies { runtime deps['com.google.auto.value:auto-value'] runtime deps['io.netty:netty-tcnative-boringssl-static'] + testCompile deps['com.google.monitoring-client:contrib'] testCompile deps['com.google.truth:truth'] testCompile deps['junit:junit'] testCompile deps['org.mockito:mockito-core'] testCompile project(':third_party') + testCompile project(path: ':core', configuration: 'testRuntime') // Include auto-value in compile until nebula-lint understands // annotationProcessor diff --git a/prober/src/main/java/google/registry/monitoring/blackbox/ProberModule.java b/prober/src/main/java/google/registry/monitoring/blackbox/ProberModule.java index bd88f3a5f..7cca1abb6 100644 --- a/prober/src/main/java/google/registry/monitoring/blackbox/ProberModule.java +++ b/prober/src/main/java/google/registry/monitoring/blackbox/ProberModule.java @@ -21,6 +21,8 @@ import google.registry.monitoring.blackbox.connection.ProbingAction; import google.registry.monitoring.blackbox.modules.CertificateModule; import google.registry.monitoring.blackbox.modules.EppModule; import google.registry.monitoring.blackbox.modules.WebWhoisModule; +import google.registry.util.Clock; +import google.registry.util.SystemClock; import io.netty.bootstrap.Bootstrap; import io.netty.channel.Channel; import io.netty.channel.EventLoopGroup; @@ -54,6 +56,13 @@ public class ProberModule { return OpenSsl.isAvailable() ? SslProvider.OPENSSL : SslProvider.JDK; } + /** {@link Provides} one global {@link Clock} shared by each {@link ProbingSequence}. */ + @Provides + @Singleton + static Clock provideClock() { + return new SystemClock(); + } + /** {@link Provides} one global {@link EventLoopGroup} shared by each {@link ProbingSequence}. */ @Provides @Singleton diff --git a/prober/src/main/java/google/registry/monitoring/blackbox/ProbingSequence.java b/prober/src/main/java/google/registry/monitoring/blackbox/ProbingSequence.java index 0ef6aad36..75053c2fa 100644 --- a/prober/src/main/java/google/registry/monitoring/blackbox/ProbingSequence.java +++ b/prober/src/main/java/google/registry/monitoring/blackbox/ProbingSequence.java @@ -16,9 +16,12 @@ package google.registry.monitoring.blackbox; import com.google.common.flogger.FluentLogger; import google.registry.monitoring.blackbox.connection.ProbingAction; +import google.registry.monitoring.blackbox.exceptions.FailureException; import google.registry.monitoring.blackbox.exceptions.UnrecoverableStateException; +import google.registry.monitoring.blackbox.metrics.MetricsCollector; import google.registry.monitoring.blackbox.tokens.Token; import google.registry.util.CircularList; +import google.registry.util.Clock; import io.netty.bootstrap.Bootstrap; import io.netty.channel.AbstractChannel; import io.netty.channel.ChannelFuture; @@ -45,6 +48,12 @@ public class ProbingSequence extends CircularList { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + /** Shared {@link MetricsCollector} used to record metrics on any step performed. */ + private MetricsCollector metrics; + + /** Shared {@link Clock} used to record latency on any step performed. */ + private Clock clock; + /** Each {@link ProbingSequence} requires a start token to begin running. */ private Token startToken; @@ -57,9 +66,16 @@ public class ProbingSequence extends CircularList { /** {@link ProbingSequence} object that represents first step in the sequence. */ private ProbingSequence first; - /** Standard constructor for {@link ProbingSequence} in the list that assigns value and token. */ - private ProbingSequence(ProbingStep value, Token startToken) { + /** + * Standard constructor for first {@link ProbingSequence} in the list that assigns value and + * token. + */ + private ProbingSequence( + ProbingStep value, MetricsCollector metrics, Clock clock, Token startToken) { + super(value); + this.metrics = metrics; + this.clock = clock; this.startToken = startToken; } @@ -95,6 +111,8 @@ public class ProbingSequence extends CircularList { * get().generateAction}. */ private void runStep(Token token) { + long start = clock.nowUtc().getMillis(); + ProbingAction currentAction; ChannelFuture future; @@ -108,12 +126,28 @@ public class ProbingSequence extends CircularList { } catch (UnrecoverableStateException e) { // On an UnrecoverableStateException, terminate the sequence. logger.atSevere().withCause(e).log("Unrecoverable error in generating or calling action."); + + // Records gathered metrics. + metrics.recordResult( + get().protocol().name(), + get().messageTemplate().name(), + get().messageTemplate().responseName(), + MetricsCollector.ResponseType.ERROR, + clock.nowUtc().getMillis() - start); return; } catch (Exception e) { // On any other type of error, restart the sequence at the very first step. logger.atWarning().withCause(e).log("Error in generating or calling action."); + // Records gathered metrics. + metrics.recordResult( + get().protocol().name(), + get().messageTemplate().name(), + get().messageTemplate().responseName(), + MetricsCollector.ResponseType.ERROR, + clock.nowUtc().getMillis() - start); + // Restart the sequence at the very first step. restartSequence(); return; @@ -125,10 +159,34 @@ public class ProbingSequence extends CircularList { // On a successful result, we log as a successful step, and note a success. logger.atInfo().log(String.format("Successfully completed Probing Step: %s", this)); + // Records gathered metrics. + metrics.recordResult( + get().protocol().name(), + get().messageTemplate().name(), + get().messageTemplate().responseName(), + MetricsCollector.ResponseType.SUCCESS, + clock.nowUtc().getMillis() - start); } else { // On a failed result, we log the failure and note either a failure or error. logger.atSevere().withCause(f.cause()).log("Did not result in future success"); + // Records gathered metrics as either FAILURE or ERROR depending on future's cause. + if (f.cause() instanceof FailureException) { + metrics.recordResult( + get().protocol().name(), + get().messageTemplate().name(), + get().messageTemplate().responseName(), + MetricsCollector.ResponseType.FAILURE, + clock.nowUtc().getMillis() - start); + } else { + metrics.recordResult( + get().protocol().name(), + get().messageTemplate().name(), + get().messageTemplate().responseName(), + MetricsCollector.ResponseType.ERROR, + clock.nowUtc().getMillis() - start); + } + // If not unrecoverable, we restart the sequence. if (!(f.cause() instanceof UnrecoverableStateException)) { restartSequence(); @@ -181,12 +239,18 @@ public class ProbingSequence extends CircularList { private Token startToken; + private MetricsCollector metrics; + + private Clock clock; + /** * This Builder must also be supplied with a {@link Token} to construct a {@link * ProbingSequence}. */ - public Builder(Token startToken) { + public Builder(Token startToken, MetricsCollector metrics, Clock clock) { this.startToken = startToken; + this.metrics = metrics; + this.clock = clock; } /** We take special note of the first repeated step. */ @@ -205,7 +269,7 @@ public class ProbingSequence extends CircularList { @Override protected ProbingSequence create(ProbingStep value) { - return new ProbingSequence(value, startToken); + return new ProbingSequence(value, metrics, clock, startToken); } /** diff --git a/prober/src/main/java/google/registry/monitoring/blackbox/messages/EppRequestMessage.java b/prober/src/main/java/google/registry/monitoring/blackbox/messages/EppRequestMessage.java index 39e8b92eb..ec12d3ce6 100644 --- a/prober/src/main/java/google/registry/monitoring/blackbox/messages/EppRequestMessage.java +++ b/prober/src/main/java/google/registry/monitoring/blackbox/messages/EppRequestMessage.java @@ -44,6 +44,12 @@ import java.util.function.BiFunction; */ public class EppRequestMessage extends EppMessage implements OutboundMessageType { + /** + * String that describes the type of EppRequestMessage: hello, login, create, check, delete, + * logout. + */ + private String name; + /** Corresponding {@link EppResponseMessage} that we expect to receive on a successful request. */ private EppResponseMessage expectedResponse; @@ -60,10 +66,12 @@ public class EppRequestMessage extends EppMessage implements OutboundMessageType * Private constructor for {@link EppRequestMessage} that its subclasses use for instantiation. */ public EppRequestMessage( + String name, EppResponseMessage expectedResponse, String template, BiFunction> getReplacements) { + this.name = name; this.expectedResponse = expectedResponse; this.template = template; this.getReplacements = getReplacements; @@ -125,8 +133,18 @@ public class EppRequestMessage extends EppMessage implements OutboundMessageType return buf; } - /** */ + /** Returns the {@link EppResponseMessage} we expect. */ public EppResponseMessage getExpectedResponse() { return expectedResponse; } + + @Override + public String name() { + return name; + } + + @Override + public String responseName() { + return expectedResponse.name(); + } } diff --git a/prober/src/main/java/google/registry/monitoring/blackbox/messages/HttpRequestMessage.java b/prober/src/main/java/google/registry/monitoring/blackbox/messages/HttpRequestMessage.java index 91e6c9214..90db6f8ba 100644 --- a/prober/src/main/java/google/registry/monitoring/blackbox/messages/HttpRequestMessage.java +++ b/prober/src/main/java/google/registry/monitoring/blackbox/messages/HttpRequestMessage.java @@ -79,7 +79,12 @@ public class HttpRequestMessage extends DefaultFullHttpRequest implements Outbou } @Override - public String toString() { + public String name() { return String.format("Http(s) Request on: %s", headers().get("host")); } + + @Override + public String responseName() { + return "Http Response"; + } } diff --git a/prober/src/main/java/google/registry/monitoring/blackbox/messages/OutboundMessageType.java b/prober/src/main/java/google/registry/monitoring/blackbox/messages/OutboundMessageType.java index 4a7c16243..bc6830e1a 100644 --- a/prober/src/main/java/google/registry/monitoring/blackbox/messages/OutboundMessageType.java +++ b/prober/src/main/java/google/registry/monitoring/blackbox/messages/OutboundMessageType.java @@ -30,8 +30,15 @@ public interface OutboundMessageType { /** * Necessary to inform metrics collector what kind of message is sent down {@link - * io.netty.channel.ChannelPipeline} + * io.netty.channel.ChannelPipeline}. Not equivalent to toString, as to different instances will + * have the same name if they perform the same action. */ - @Override - String toString(); + String name(); + + /** + * Necessary to inform metrics collector what kind of message is sent inbound {@link + * io.netty.channel.ChannelPipeline}. Equivalent to {@code name} but for {@link + * InboundMessageType}. + */ + String responseName(); } diff --git a/prober/src/main/java/google/registry/monitoring/blackbox/metrics/MetricsCollector.java b/prober/src/main/java/google/registry/monitoring/blackbox/metrics/MetricsCollector.java new file mode 100644 index 000000000..dbaf4a866 --- /dev/null +++ b/prober/src/main/java/google/registry/monitoring/blackbox/metrics/MetricsCollector.java @@ -0,0 +1,92 @@ +// Copyright 2019 The Nomulus Authors. All Rights Reserved. +// +// 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 google.registry.monitoring.blackbox.metrics; + +import com.google.common.collect.ImmutableSet; +import com.google.monitoring.metrics.EventMetric; +import com.google.monitoring.metrics.ExponentialFitter; +import com.google.monitoring.metrics.IncrementableMetric; +import com.google.monitoring.metrics.LabelDescriptor; +import com.google.monitoring.metrics.MetricRegistryImpl; +import google.registry.util.NonFinalForTesting; +import javax.inject.Inject; +import javax.inject.Singleton; + +/** Metrics collection instrumentation. */ +@Singleton +public class MetricsCollector { + + /** Three standard Response types to be recorded as metrics: SUCCESS, FAILURE, or ERROR. */ + public enum ResponseType { + SUCCESS, + FAILURE, + ERROR + } + + // Maximum 1 hour latency, this is not specified by the spec, but given we have a one hour idle + // timeout, it seems reasonable that maximum latency is set to 1 hour as well. If we are + // approaching anywhere near 1 hour latency, we'd be way out of SLO anyway. + private static final ExponentialFitter DEFAULT_LATENCY_FITTER = + ExponentialFitter.create(22, 2, 1.0); + + private static final ImmutableSet LABELS = + ImmutableSet.of( + LabelDescriptor.create("protocol", "Name of the protocol."), + LabelDescriptor.create("request", "Name of outbound request"), + LabelDescriptor.create("response", "Name of inbound response"), + LabelDescriptor.create("responseType", "Status of action performed")); + + static final IncrementableMetric responsesCounter = + MetricRegistryImpl.getDefault() + .newIncrementableMetric( + "/prober/responses", + "Total number of responses received by the prober.", + "Responses", + LABELS); + + static final EventMetric latencyMs = + MetricRegistryImpl.getDefault() + .newEventMetric( + "/prober/latency_specific_ms", + "Round-trip time between a request sent and its corresponding response received.", + "Latency Milliseconds", + LABELS, + DEFAULT_LATENCY_FITTER); + + @Inject + MetricsCollector() {} + + /** + * Resets all backend metrics. + * + *

This should only used in tests to clear out states. No production code should call this + * function. + */ + void resetMetric() { + responsesCounter.reset(); + latencyMs.reset(); + } + + @NonFinalForTesting + public void recordResult( + String protocolName, + String requestName, + String responseName, + ResponseType status, + long latency) { + latencyMs.record(latency, protocolName, requestName, responseName, status.name()); + responsesCounter.increment(protocolName, requestName, responseName, status.name()); + } +} diff --git a/prober/src/main/java/google/registry/monitoring/blackbox/modules/EppModule.java b/prober/src/main/java/google/registry/monitoring/blackbox/modules/EppModule.java index 91bb284be..ba68f188e 100644 --- a/prober/src/main/java/google/registry/monitoring/blackbox/modules/EppModule.java +++ b/prober/src/main/java/google/registry/monitoring/blackbox/modules/EppModule.java @@ -34,8 +34,10 @@ import google.registry.monitoring.blackbox.handlers.SslClientInitializer; import google.registry.monitoring.blackbox.messages.EppMessage; import google.registry.monitoring.blackbox.messages.EppRequestMessage; import google.registry.monitoring.blackbox.messages.EppResponseMessage; +import google.registry.monitoring.blackbox.metrics.MetricsCollector; import google.registry.monitoring.blackbox.modules.CertificateModule.LocalSecrets; import google.registry.monitoring.blackbox.tokens.EppToken; +import google.registry.util.Clock; import io.netty.bootstrap.Bootstrap; import io.netty.channel.ChannelHandler; import io.netty.channel.socket.nio.NioSocketChannel; @@ -86,13 +88,15 @@ public class EppModule { @IntoSet static ProbingSequence provideEppLoginCreateCheckDeleteCheckProbingSequence( EppToken.Persistent token, + MetricsCollector metrics, + Clock clock, @Named("hello") ProbingStep helloStep, @Named("loginSuccess") ProbingStep loginSuccessStep, @Named("createSuccess") ProbingStep createSuccessStep, @Named("checkExists") ProbingStep checkStepFirst, @Named("deleteSuccess") ProbingStep deleteSuccessStep, @Named("checkNotExists") ProbingStep checkStepSecond) { - return new ProbingSequence.Builder(token) + return new ProbingSequence.Builder(token, metrics, clock) .add(helloStep) .add(loginSuccessStep) .add(createSuccessStep) @@ -112,6 +116,8 @@ public class EppModule { @IntoSet static ProbingSequence provideEppLoginCreateCheckDeleteCheckLogoutProbingSequence( EppToken.Transient token, + MetricsCollector metrics, + Clock clock, @Named("hello") ProbingStep helloStep, @Named("loginSuccess") ProbingStep loginSuccessStep, @Named("createSuccess") ProbingStep createSuccessStep, @@ -119,7 +125,7 @@ public class EppModule { @Named("deleteSuccess") ProbingStep deleteSuccessStep, @Named("checkNotExists") ProbingStep checkStepSecond, @Named("logout") ProbingStep logoutStep) { - return new ProbingSequence.Builder(token) + return new ProbingSequence.Builder(token, metrics, clock) .add(helloStep) .add(loginSuccessStep) .add(createSuccessStep) @@ -253,7 +259,7 @@ public class EppModule { static EppRequestMessage provideHelloRequestMessage( @Named("greeting") EppResponseMessage greetingResponse) { - return new EppRequestMessage(greetingResponse, null, (a, b) -> ImmutableMap.of()); + return new EppRequestMessage("hello", greetingResponse, null, (a, b) -> ImmutableMap.of()); } /** @@ -270,6 +276,7 @@ public class EppModule { @Named("eppUserId") String userId, @Named("eppPassword") String userPassword) { return new EppRequestMessage( + "login", successResponse, loginTemplate, (clTrid, domain) -> @@ -288,6 +295,7 @@ public class EppModule { @Named("eppUserId") String userId, @Named("eppPassword") String userPassword) { return new EppRequestMessage( + "login", failureResponse, loginTemplate, (clTrid, domain) -> @@ -304,6 +312,7 @@ public class EppModule { @Named("success") EppResponseMessage successResponse, @Named("create") String createTemplate) { return new EppRequestMessage( + "create", successResponse, createTemplate, (clTrid, domain) -> @@ -319,6 +328,7 @@ public class EppModule { @Named("failure") EppResponseMessage failureResponse, @Named("create") String createTemplate) { return new EppRequestMessage( + "create", failureResponse, createTemplate, (clTrid, domain) -> @@ -334,6 +344,7 @@ public class EppModule { @Named("success") EppResponseMessage successResponse, @Named("delete") String deleteTemplate) { return new EppRequestMessage( + "delete", successResponse, deleteTemplate, (clTrid, domain) -> @@ -349,6 +360,7 @@ public class EppModule { @Named("failure") EppResponseMessage failureResponse, @Named("delete") String deleteTemplate) { return new EppRequestMessage( + "delete", failureResponse, deleteTemplate, (clTrid, domain) -> @@ -364,6 +376,7 @@ public class EppModule { @Named("success") EppResponseMessage successResponse, @Named("logout") String logoutTemplate) { return new EppRequestMessage( + "logout", successResponse, logoutTemplate, (clTrid, domain) -> ImmutableMap.of(CLIENT_TRID_KEY, clTrid)); @@ -376,6 +389,7 @@ public class EppModule { @Named("domainExists") EppResponseMessage domainExistsResponse, @Named("check") String checkTemplate) { return new EppRequestMessage( + "check", domainExistsResponse, checkTemplate, (clTrid, domain) -> @@ -391,6 +405,7 @@ public class EppModule { @Named("domainNotExists") EppResponseMessage domainNotExistsResponse, @Named("check") String checkTemplate) { return new EppRequestMessage( + "check", domainNotExistsResponse, checkTemplate, (clTrid, domain) -> diff --git a/prober/src/main/java/google/registry/monitoring/blackbox/modules/WebWhoisModule.java b/prober/src/main/java/google/registry/monitoring/blackbox/modules/WebWhoisModule.java index 67d5abcb7..2fc7f0c02 100644 --- a/prober/src/main/java/google/registry/monitoring/blackbox/modules/WebWhoisModule.java +++ b/prober/src/main/java/google/registry/monitoring/blackbox/modules/WebWhoisModule.java @@ -25,8 +25,10 @@ import google.registry.monitoring.blackbox.handlers.SslClientInitializer; import google.registry.monitoring.blackbox.handlers.WebWhoisActionHandler; import google.registry.monitoring.blackbox.handlers.WebWhoisMessageHandler; import google.registry.monitoring.blackbox.messages.HttpRequestMessage; +import google.registry.monitoring.blackbox.metrics.MetricsCollector; import google.registry.monitoring.blackbox.tokens.WebWhoisToken; import google.registry.util.CircularList; +import google.registry.util.Clock; import io.netty.bootstrap.Bootstrap; import io.netty.channel.Channel; import io.netty.channel.ChannelHandler; @@ -172,9 +174,11 @@ public class WebWhoisModule { @Singleton @IntoSet ProbingSequence provideWebWhoisSequence( - @WebWhoisProtocol ProbingStep probingStep, WebWhoisToken webWhoisToken) { - - return new ProbingSequence.Builder(webWhoisToken).add(probingStep).build(); + @WebWhoisProtocol ProbingStep probingStep, + WebWhoisToken webWhoisToken, + MetricsCollector metrics, + Clock clock) { + return new ProbingSequence.Builder(webWhoisToken, metrics, clock).add(probingStep).build(); } @Provides diff --git a/prober/src/test/java/google/registry/monitoring/blackbox/ProbingSequenceTest.java b/prober/src/test/java/google/registry/monitoring/blackbox/ProbingSequenceTest.java index eebfd8467..a2ad28b83 100644 --- a/prober/src/test/java/google/registry/monitoring/blackbox/ProbingSequenceTest.java +++ b/prober/src/test/java/google/registry/monitoring/blackbox/ProbingSequenceTest.java @@ -16,9 +16,9 @@ package google.registry.monitoring.blackbox; import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doCallRealMethod; import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -27,10 +27,16 @@ import google.registry.monitoring.blackbox.connection.Protocol; import google.registry.monitoring.blackbox.exceptions.FailureException; import google.registry.monitoring.blackbox.exceptions.UndeterminedStateException; import google.registry.monitoring.blackbox.exceptions.UnrecoverableStateException; +import google.registry.monitoring.blackbox.messages.OutboundMessageType; +import google.registry.monitoring.blackbox.metrics.MetricsCollector; +import google.registry.monitoring.blackbox.metrics.MetricsCollector.ResponseType; import google.registry.monitoring.blackbox.tokens.Token; +import google.registry.testing.FakeClock; +import google.registry.util.Clock; import io.netty.channel.Channel; import io.netty.channel.ChannelPromise; import io.netty.channel.embedded.EmbeddedChannel; +import org.joda.time.Duration; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -53,6 +59,11 @@ import org.mockito.Mockito; @RunWith(JUnit4.class) public class ProbingSequenceTest { + private static final String PROTOCOL_NAME = "PROTOCOL"; + private static final String MESSAGE_NAME = "MESSAGE"; + private static final String RESPONSE_NAME = "RESPONSE"; + private static final Duration LATENCY = Duration.millis(2L); + /** Default mock {@link ProbingAction} returned when generating an action with a mockStep. */ private ProbingAction mockAction = Mockito.mock(ProbingAction.class); @@ -71,22 +82,39 @@ public class ProbingSequenceTest { */ private Protocol mockProtocol = Mockito.mock(Protocol.class); + /** + * Default mock {@link OutboundMessageType} returned by {@code mockStep} and occasionally other + * mock {@link ProbingStep}s. + */ + private OutboundMessageType mockMessage = Mockito.mock(OutboundMessageType.class); + /** * {@link EmbeddedChannel} used to create new {@link ChannelPromise} objects returned by mock * {@link ProbingAction}s on their {@code call} methods. */ private EmbeddedChannel channel = new EmbeddedChannel(); + /** Default mock {@link MetricsCollector} passed into each {@link ProbingSequence} tested */ + private MetricsCollector metrics = Mockito.mock(MetricsCollector.class); + + /** Default mock {@link Clock} passed into each {@link ProbingSequence} tested */ + private Clock clock = new FakeClock(); + @Before public void setup() { // To avoid a NullPointerException, we must have a protocol return persistent connection as // false. doReturn(true).when(mockProtocol).persistentConnection(); + doReturn(PROTOCOL_NAME).when(mockProtocol).name(); // In order to avoid a NullPointerException, we must have the protocol returned that stores // persistent connection as false. doReturn(mockProtocol).when(mockStep).protocol(); + doReturn(MESSAGE_NAME).when(mockMessage).name(); + doReturn(RESPONSE_NAME).when(mockMessage).responseName(); + doReturn(mockMessage).when(mockStep).messageTemplate(); + // Allows for test if channel is accurately set. doCallRealMethod().when(mockToken).setChannel(any(Channel.class)); doCallRealMethod().when(mockToken).channel(); @@ -102,7 +130,7 @@ public class ProbingSequenceTest { ProbingStep thirdStep = Mockito.mock(ProbingStep.class); ProbingSequence sequence = - new ProbingSequence.Builder(mockToken) + new ProbingSequence.Builder(mockToken, metrics, clock) .add(firstStep) .add(secondStep) .add(thirdStep) @@ -127,7 +155,7 @@ public class ProbingSequenceTest { ProbingStep thirdStep = Mockito.mock(ProbingStep.class); ProbingSequence sequence = - new ProbingSequence.Builder(mockToken) + new ProbingSequence.Builder(mockToken, metrics, clock) .add(thirdStep) .add(secondStep) .markFirstRepeated() @@ -148,8 +176,15 @@ public class ProbingSequenceTest { @Test public void testRunStep_Success() throws UndeterminedStateException { - // Always returns a succeeded future on call to mockAction. - doReturn(channel.newSucceededFuture()).when(mockAction).call(); + // Always returns a succeeded future on call to mockAction. Also advances the FakeClock by + // standard LATENCY to check right latency is recorded. + doAnswer( + answer -> { + ((FakeClock) clock).advanceBy(LATENCY); + return channel.newSucceededFuture(); + }) + .when(mockAction) + .call(); // Has mockStep always return mockAction on call to generateAction. doReturn(mockAction).when(mockStep).generateAction(any(Token.class)); @@ -165,7 +200,10 @@ public class ProbingSequenceTest { // Build testable sequence from mocked components. ProbingSequence sequence = - new ProbingSequence.Builder(mockToken).add(mockStep).add(secondStep).build(); + new ProbingSequence.Builder(mockToken, metrics, clock) + .add(mockStep) + .add(secondStep) + .build(); sequence.start(); @@ -182,12 +220,25 @@ public class ProbingSequenceTest { // We should have modified the token's channel after the first, succeeded step. assertThat(mockToken.channel()).isEqualTo(channel); + + // Verifies that metrics records the right kind of result (a success with the input protocol + // name and message name). + verify(metrics) + .recordResult( + PROTOCOL_NAME, MESSAGE_NAME, RESPONSE_NAME, ResponseType.SUCCESS, LATENCY.getMillis()); } @Test public void testRunLoop_Success() throws UndeterminedStateException { - // Always returns a succeeded future on call to mockAction. - doReturn(channel.newSucceededFuture()).when(mockAction).call(); + // Always returns a succeeded future on call to mockAction. Also advances the FakeClock by + // standard LATENCY to check right latency is recorded. + doAnswer( + answer -> { + ((FakeClock) clock).advanceBy(LATENCY); + return channel.newSucceededFuture(); + }) + .when(mockAction) + .call(); // Has mockStep always return mockAction on call to generateAction doReturn(mockAction).when(mockStep).generateAction(mockToken); @@ -199,9 +250,21 @@ public class ProbingSequenceTest { // Necessary for success of ProbingSequence runStep method as it calls get().protocol(). doReturn(mockProtocol).when(secondStep).protocol(); + // Necessary for success of ProbingSequence recording metrics as it calls get() + // .messageTemplate.name(). + doReturn(mockMessage).when(secondStep).messageTemplate(); + // We ensure that secondStep has necessary attributes to be successful step to pass on to - // mockStep once more. - doReturn(channel.newSucceededFuture()).when(secondAction).call(); + // mockStep once more. Also have clock time pass by standard LATENCY to ensure right latency + // is recorded. + doAnswer( + answer -> { + ((FakeClock) clock).advanceBy(LATENCY); + return channel.newSucceededFuture(); + }) + .when(secondAction) + .call(); + doReturn(secondAction).when(secondStep).generateAction(mockToken); // We get a secondToken that is returned when we are on our second loop in the sequence. This @@ -217,12 +280,15 @@ public class ProbingSequenceTest { // Build testable sequence from mocked components. ProbingSequence sequence = - new ProbingSequence.Builder(mockToken).add(mockStep).add(secondStep).build(); + new ProbingSequence.Builder(mockToken, metrics, clock) + .add(mockStep) + .add(secondStep) + .build(); sequence.start(); // We expect to have generated actions from mockStep twice (once for mockToken and once for - // secondToken), and we expectto have called each generated action only once, as when we move + // secondToken), and we expect to have called each generated action only once, as when we move // on to mockStep the second time, it will terminate the sequence after calling thirdAction. verify(mockStep).generateAction(mockToken); verify(mockStep).generateAction(secondToken); @@ -236,6 +302,18 @@ public class ProbingSequenceTest { // We should have modified the token's channel after the first, succeeded step. assertThat(mockToken.channel()).isEqualTo(channel); + + // Verifies that metrics records the right kind of result (a success with the input protocol + // name and message name) two times: once for mockStep and once for secondStep. + verify(metrics, times(2)) + .recordResult( + PROTOCOL_NAME, MESSAGE_NAME, RESPONSE_NAME, ResponseType.SUCCESS, LATENCY.getMillis()); + + // Verify that on second pass, since we purposely throw UnrecoverableStateException, we + // record the ERROR. Also, we haven't had any time pass in the fake clock, so recorded + // latency should be 0. + verify(metrics) + .recordResult(PROTOCOL_NAME, MESSAGE_NAME, RESPONSE_NAME, ResponseType.ERROR, 0L); } /** @@ -250,14 +328,26 @@ public class ProbingSequenceTest { ProbingStep secondStep = Mockito.mock(ProbingStep.class); // We create a second token that when used to generate an action throws an - // UnrecoverableStateException to terminate the sequence + // UnrecoverableStateException to terminate the sequence. Token secondToken = Mockito.mock(Token.class); doReturn(secondToken).when(mockToken).next(); - doThrow(new UnrecoverableStateException("")).when(mockStep).generateAction(secondToken); + + // When we throw the UnrecoverableStateException, we ensure that the right latency is + // recorded by advancing the clock by LATENCY. + doAnswer( + answer -> { + ((FakeClock) clock).advanceBy(LATENCY); + throw new UnrecoverableStateException(""); + }) + .when(mockStep) + .generateAction(secondToken); // Build testable sequence from mocked components. ProbingSequence sequence = - new ProbingSequence.Builder(mockToken).add(mockStep).add(secondStep).build(); + new ProbingSequence.Builder(mockToken, metrics, clock) + .add(mockStep) + .add(secondStep) + .build(); sequence.start(); @@ -278,8 +368,15 @@ public class ProbingSequenceTest { @Test public void testRunStep_FailureRunning() throws UndeterminedStateException { - // Returns a failed future when calling the generated mock action. - doReturn(channel.newFailedFuture(new FailureException(""))).when(mockAction).call(); + // Returns a failed future when calling the generated mock action. Also advances FakeClock by + // LATENCY in order to check right latency is recorded. + doAnswer( + answer -> { + ((FakeClock) clock).advanceBy(LATENCY); + return channel.newFailedFuture(new FailureException("")); + }) + .when(mockAction) + .call(); // Returns mock action on call to generate action for ProbingStep. doReturn(mockAction).when(mockStep).generateAction(mockToken); @@ -290,16 +387,42 @@ public class ProbingSequenceTest { // We only expect to have called this action once, as we only get it from one generateAction // call. verify(mockAction).call(); + + // Verifies that metrics records the right kind of result (a failure with the input protocol + // name and message name). + verify(metrics) + .recordResult( + PROTOCOL_NAME, MESSAGE_NAME, RESPONSE_NAME, ResponseType.FAILURE, LATENCY.getMillis()); + + // Verify that on second pass, since we purposely throw UnrecoverableStateException, we + // record the ERROR. We also should make sure LATENCY seconds have passed. + verify(metrics) + .recordResult( + PROTOCOL_NAME, MESSAGE_NAME, RESPONSE_NAME, ResponseType.ERROR, LATENCY.getMillis()); } @Test public void testRunStep_FailureGenerating() throws UndeterminedStateException { - // Create a mock first step that returns the dummy action when called to generate an action. - doThrow(UndeterminedStateException.class).when(mockStep).generateAction(mockToken); + // Mock first step throws an error when generating the first action, and advances the clock + // by LATENCY. + doAnswer( + answer -> { + ((FakeClock) clock).advanceBy(LATENCY); + throw new UndeterminedStateException(""); + }) + .when(mockStep) + .generateAction(mockToken); + // Tests generic behavior we expect when we fail in generating or calling an action. testActionFailure(); // We expect to have never called this action, as we fail each time whenever generating actions. verify(mockAction, times(0)).call(); + + // Verify that we record two errors, first for being unable to generate the action, second + // for terminating the sequence. + verify(metrics, times(2)) + .recordResult( + PROTOCOL_NAME, MESSAGE_NAME, RESPONSE_NAME, ResponseType.ERROR, LATENCY.getMillis()); } } diff --git a/prober/src/test/java/google/registry/monitoring/blackbox/messages/TestMessage.java b/prober/src/test/java/google/registry/monitoring/blackbox/messages/TestMessage.java index 922c3052b..64a727cfd 100644 --- a/prober/src/test/java/google/registry/monitoring/blackbox/messages/TestMessage.java +++ b/prober/src/test/java/google/registry/monitoring/blackbox/messages/TestMessage.java @@ -38,4 +38,14 @@ public class TestMessage implements OutboundMessageType, InboundMessageType { message = args[0]; return this; } + + @Override + public String name() { + return message; + } + + @Override + public String responseName() { + return message; + } } diff --git a/prober/src/test/java/google/registry/monitoring/blackbox/metrics/MetricsCollectorTest.java b/prober/src/test/java/google/registry/monitoring/blackbox/metrics/MetricsCollectorTest.java new file mode 100644 index 000000000..c4e2bdc86 --- /dev/null +++ b/prober/src/test/java/google/registry/monitoring/blackbox/metrics/MetricsCollectorTest.java @@ -0,0 +1,100 @@ +// Copyright 2019 The Nomulus Authors. All Rights Reserved. +// +// 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 google.registry.monitoring.blackbox.metrics; + +import static com.google.monitoring.metrics.contrib.DistributionMetricSubject.assertThat; +import static com.google.monitoring.metrics.contrib.LongMetricSubject.assertThat; + +import com.google.common.collect.ImmutableSet; +import google.registry.monitoring.blackbox.metrics.MetricsCollector.ResponseType; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Unit tests for {@link MetricsCollector}. */ +@RunWith(JUnit4.class) +public class MetricsCollectorTest { + + private final String requestName = "request"; + private final String responseName = "response"; + private final String protocol = "protocol"; + + private final MetricsCollector metrics = new MetricsCollector(); + + @Before + public void setUp() { + metrics.resetMetric(); + } + + @Test + public void testOneRecord() { + metrics.recordResult(protocol, requestName, responseName, ResponseType.SUCCESS, 100); + + assertThat(MetricsCollector.responsesCounter) + .hasValueForLabels(1, protocol, requestName, responseName, ResponseType.SUCCESS.name()) + .and() + .hasNoOtherValues(); + + assertThat(MetricsCollector.latencyMs) + .hasDataSetForLabels( + ImmutableSet.of(100), protocol, requestName, responseName, ResponseType.SUCCESS.name()) + .and() + .hasNoOtherValues(); + } + + @Test + public void testMultipleRecords_sameStatus() { + metrics.recordResult(protocol, requestName, responseName, ResponseType.FAILURE, 100); + metrics.recordResult(protocol, requestName, responseName, ResponseType.FAILURE, 200); + + assertThat(MetricsCollector.responsesCounter) + .hasValueForLabels(2, protocol, requestName, responseName, ResponseType.FAILURE.name()) + .and() + .hasNoOtherValues(); + + assertThat(MetricsCollector.latencyMs) + .hasDataSetForLabels( + ImmutableSet.of(100, 200), + protocol, + requestName, + responseName, + ResponseType.FAILURE.name()) + .and() + .hasNoOtherValues(); + } + + @Test + public void testMultipleRecords_differentStatus() { + metrics.recordResult(protocol, requestName, responseName, ResponseType.SUCCESS, 100); + metrics.recordResult(protocol, requestName, responseName, ResponseType.FAILURE, 200); + + assertThat(MetricsCollector.responsesCounter) + .hasValueForLabels(1, protocol, requestName, responseName, ResponseType.SUCCESS.name()) + .and() + .hasValueForLabels(1, protocol, requestName, responseName, ResponseType.FAILURE.name()) + .and() + .hasNoOtherValues(); + + assertThat(MetricsCollector.latencyMs) + .hasDataSetForLabels( + ImmutableSet.of(100), protocol, requestName, responseName, ResponseType.SUCCESS.name()) + .and() + .hasDataSetForLabels( + ImmutableSet.of(200), protocol, requestName, responseName, ResponseType.FAILURE.name()) + .and() + .hasNoOtherValues(); + } +} diff --git a/prober/src/test/java/google/registry/monitoring/blackbox/util/EppUtils.java b/prober/src/test/java/google/registry/monitoring/blackbox/util/EppUtils.java index 266dcc723..4a3737da9 100644 --- a/prober/src/test/java/google/registry/monitoring/blackbox/util/EppUtils.java +++ b/prober/src/test/java/google/registry/monitoring/blackbox/util/EppUtils.java @@ -92,13 +92,14 @@ public class EppUtils { /** Returns standard hello request with supplied response. */ public static EppRequestMessage getHelloMessage(EppResponseMessage greetingResponse) { - return new EppRequestMessage(greetingResponse, null, (a, b) -> ImmutableMap.of()); + return new EppRequestMessage("hello", greetingResponse, null, (a, b) -> ImmutableMap.of()); } /** Returns standard login request with supplied userId, userPassword, and response. */ public static EppRequestMessage getLoginMessage( EppResponseMessage response, String userId, String userPassword) { return new EppRequestMessage( + "login", response, "login.xml", (clTrid, domain) -> @@ -111,6 +112,7 @@ public class EppUtils { /** Returns standard create request with supplied response. */ public static EppRequestMessage getCreateMessage(EppResponseMessage response) { return new EppRequestMessage( + "create", response, "create.xml", (clTrid, domain) -> @@ -122,6 +124,7 @@ public class EppUtils { /** Returns standard delete request with supplied response. */ public static EppRequestMessage getDeleteMessage(EppResponseMessage response) { return new EppRequestMessage( + "delete", response, "delete.xml", (clTrid, domain) -> @@ -133,6 +136,7 @@ public class EppUtils { /** Returns standard logout request with supplied response. */ public static EppRequestMessage getLogoutMessage(EppResponseMessage successResponse) { return new EppRequestMessage( + "logout", successResponse, "logout.xml", (clTrid, domain) -> ImmutableMap.of(CLIENT_TRID_KEY, clTrid)); @@ -141,6 +145,7 @@ public class EppUtils { /** Returns standard check request with supplied response. */ public static EppRequestMessage getCheckMessage(EppResponseMessage response) { return new EppRequestMessage( + "check", response, "check.xml", (clTrid, domain) ->