diff --git a/java/google/registry/env/common/default/WEB-INF/queue.xml b/java/google/registry/env/common/default/WEB-INF/queue.xml index 3e14770b7..7b2cf9afb 100644 --- a/java/google/registry/env/common/default/WEB-INF/queue.xml +++ b/java/google/registry/env/common/default/WEB-INF/queue.xml @@ -192,17 +192,6 @@ - - - bigquery-streaming-metrics - 500/s - 500 - - 1 - 1m - - - retryable-cron-tasks diff --git a/java/google/registry/flows/EppController.java b/java/google/registry/flows/EppController.java index a98517a9f..7a43e3ff8 100644 --- a/java/google/registry/flows/EppController.java +++ b/java/google/registry/flows/EppController.java @@ -22,7 +22,6 @@ import static google.registry.flows.FlowReporter.extractTlds; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Joiner; import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; import com.google.common.flogger.FluentLogger; @@ -33,7 +32,6 @@ import google.registry.model.eppoutput.EppOutput; import google.registry.model.eppoutput.EppResponse; import google.registry.model.eppoutput.Result; import google.registry.model.eppoutput.Result.Code; -import google.registry.monitoring.whitebox.BigQueryMetricsEnqueuer; import google.registry.monitoring.whitebox.EppMetric; import java.util.Optional; import javax.inject.Inject; @@ -52,7 +50,6 @@ public final class EppController { @Inject FlowComponent.Builder flowComponentBuilder; @Inject EppMetric.Builder eppMetricBuilder; @Inject EppMetrics eppMetrics; - @Inject BigQueryMetricsEnqueuer bigQueryMetricsEnqueuer; @Inject ServerTridProvider serverTridProvider; @Inject EppController() {} @@ -65,7 +62,6 @@ public final class EppController { boolean isSuperuser, byte[] inputXmlBytes) { eppMetricBuilder.setClientId(Optional.ofNullable(sessionMetadata.getClientId())); - eppMetricBuilder.setPrivilegeLevel(isSuperuser ? "SUPERUSER" : "NORMAL"); try { EppInput eppInput; try { @@ -99,7 +95,6 @@ public final class EppController { e.getResult(), Trid.create(null, serverTridProvider.createServerTrid())); } if (!eppInput.getTargetIds().isEmpty()) { - eppMetricBuilder.setEppTarget(Joiner.on(',').join(eppInput.getTargetIds())); if (eppInput.isDomainResourceType()) { eppMetricBuilder.setTlds(extractTlds(eppInput.getTargetIds())); } @@ -122,7 +117,6 @@ public final class EppController { } finally { if (!isDryRun) { EppMetric metric = eppMetricBuilder.build(); - bigQueryMetricsEnqueuer.export(metric); eppMetrics.incrementEppRequests(metric); eppMetrics.recordProcessingTime(metric); } diff --git a/java/google/registry/flows/FlowRunner.java b/java/google/registry/flows/FlowRunner.java index b830e4e23..8df7f0083 100644 --- a/java/google/registry/flows/FlowRunner.java +++ b/java/google/registry/flows/FlowRunner.java @@ -72,7 +72,6 @@ public class FlowRunner { } eppMetricBuilder.setCommandNameFromFlow(flowClass.getSimpleName()); if (!isTransactional) { - eppMetricBuilder.incrementAttempts(); EppOutput eppOutput = EppOutput.create(flowProvider.get().run()); if (flowClass.equals(LoginFlow.class)) { // In LoginFlow, clientId isn't known until after the flow executes, so save it then. @@ -84,7 +83,6 @@ public class FlowRunner { return ofy() .transact( () -> { - eppMetricBuilder.incrementAttempts(); try { EppOutput output = EppOutput.create(flowProvider.get().run()); if (isDryRun) { diff --git a/java/google/registry/module/backend/BackendRequestComponent.java b/java/google/registry/module/backend/BackendRequestComponent.java index 0b5df3532..09c7fa8ab 100644 --- a/java/google/registry/module/backend/BackendRequestComponent.java +++ b/java/google/registry/module/backend/BackendRequestComponent.java @@ -53,7 +53,6 @@ import google.registry.export.sheet.SheetModule; import google.registry.export.sheet.SyncRegistrarsSheetAction; import google.registry.flows.async.AsyncFlowsModule; import google.registry.mapreduce.MapreduceModule; -import google.registry.monitoring.whitebox.MetricsExportAction; import google.registry.monitoring.whitebox.WhiteboxModule; import google.registry.rde.BrdaCopyAction; import google.registry.rde.RdeModule; @@ -136,7 +135,6 @@ interface BackendRequestComponent { IcannReportingStagingAction icannReportingStagingAction(); IcannReportingUploadAction icannReportingUploadAction(); LoadSnapshotAction loadSnapshotAction(); - MetricsExportAction metricsExportAction(); NordnUploadAction nordnUploadAction(); NordnVerifyAction nordnVerifyAction(); PublishDnsUpdatesAction publishDnsUpdatesAction(); diff --git a/java/google/registry/monitoring/whitebox/BigQueryMetric.java b/java/google/registry/monitoring/whitebox/BigQueryMetric.java deleted file mode 100644 index 70c9373ee..000000000 --- a/java/google/registry/monitoring/whitebox/BigQueryMetric.java +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright 2017 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.whitebox; - -import com.google.api.services.bigquery.model.TableFieldSchema; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; - -/** - * A metric which can be encoded into a BigQuery row. - * - * @see BigQueryMetricsEnqueuer - */ -public interface BigQueryMetric { - - /** Get the BigQuery table name for this metric. */ - String getTableId(); - - /** Get the schema description for the BigQuery table. */ - ImmutableList getSchemaFields(); - - /** Get a map of the row values for this metric instance. */ - ImmutableMap getBigQueryRowEncoding(); -} diff --git a/java/google/registry/monitoring/whitebox/BigQueryMetricsEnqueuer.java b/java/google/registry/monitoring/whitebox/BigQueryMetricsEnqueuer.java deleted file mode 100644 index 54905098c..000000000 --- a/java/google/registry/monitoring/whitebox/BigQueryMetricsEnqueuer.java +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright 2017 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.whitebox; - -import static com.google.appengine.api.taskqueue.TaskOptions.Builder.withUrl; - -import com.google.appengine.api.taskqueue.Queue; -import com.google.appengine.api.taskqueue.TaskOptions; -import com.google.appengine.api.taskqueue.TransientFailureException; -import com.google.common.base.Supplier; -import com.google.common.flogger.FluentLogger; -import google.registry.util.AppEngineServiceUtils; -import java.util.Map.Entry; -import javax.inject.Inject; -import javax.inject.Named; - -/** - * A collector of metric information. Enqueues collected metrics to a task queue to be written to - * BigQuery asynchronously. - * - * @see MetricsExportAction - */ -public class BigQueryMetricsEnqueuer { - - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - - public static final String QUEUE_BIGQUERY_STREAMING_METRICS = "bigquery-streaming-metrics"; - - @Inject AppEngineServiceUtils appEngineServiceUtils; - @Inject @Named("insertIdGenerator") Supplier idGenerator; - @Inject @Named(QUEUE_BIGQUERY_STREAMING_METRICS) Queue queue; - - @Inject BigQueryMetricsEnqueuer() {} - - public void export(BigQueryMetric metric) { - try { - String hostname = appEngineServiceUtils.getCurrentVersionHostname("backend"); - TaskOptions opts = - withUrl(MetricsExportAction.PATH) - .header("Host", hostname) - .param("insertId", idGenerator.get()); - for (Entry entry : metric.getBigQueryRowEncoding().entrySet()) { - opts.param(entry.getKey(), entry.getValue()); - } - opts.param("tableId", metric.getTableId()); - queue.add(opts); - } catch (TransientFailureException e) { - // Log and swallow. We may drop some metrics here but this should be rare. - logger.atInfo().withCause(e).log( - "Transient error occurred while recording metric; metric dropped."); - } - } -} diff --git a/java/google/registry/monitoring/whitebox/EppMetric.java b/java/google/registry/monitoring/whitebox/EppMetric.java index 26e298a42..d328e1d56 100644 --- a/java/google/registry/monitoring/whitebox/EppMetric.java +++ b/java/google/registry/monitoring/whitebox/EppMetric.java @@ -15,44 +15,19 @@ package google.registry.monitoring.whitebox; import static com.google.common.base.Preconditions.checkArgument; -import static google.registry.bigquery.BigqueryUtils.toBigqueryTimestamp; -import com.google.api.services.bigquery.model.TableFieldSchema; import com.google.auto.value.AutoValue; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; -import google.registry.bigquery.BigqueryUtils.FieldType; import google.registry.model.eppoutput.Result.Code; import google.registry.model.registry.Registries; import google.registry.util.Clock; import java.util.Optional; import org.joda.time.DateTime; -/** - * A value class for recording attributes of an EPP metric. - * - * @see BigQueryMetricsEnqueuer - */ +/** A value class for recording attributes of an EPP metric. */ @AutoValue -public abstract class EppMetric implements BigQueryMetric { - - static final String TABLE_ID = "eppMetrics"; - static final ImmutableList SCHEMA_FIELDS = - ImmutableList.of( - new TableFieldSchema().setName("requestId").setType(FieldType.STRING.name()), - new TableFieldSchema().setName("startTime").setType(FieldType.TIMESTAMP.name()), - new TableFieldSchema().setName("endTime").setType(FieldType.TIMESTAMP.name()), - new TableFieldSchema().setName("commandName").setType(FieldType.STRING.name()), - new TableFieldSchema().setName("clientId").setType(FieldType.STRING.name()), - new TableFieldSchema().setName("tld").setType(FieldType.STRING.name()), - new TableFieldSchema().setName("privilegeLevel").setType(FieldType.STRING.name()), - new TableFieldSchema().setName("eppTarget").setType(FieldType.STRING.name()), - new TableFieldSchema().setName("eppStatus").setType(FieldType.INTEGER.name()), - new TableFieldSchema().setName("attempts").setType(FieldType.INTEGER.name())); - - public abstract String getRequestId(); +public abstract class EppMetric { public abstract DateTime getStartTimestamp(); @@ -64,55 +39,8 @@ public abstract class EppMetric implements BigQueryMetric { public abstract Optional getTld(); - public abstract Optional getPrivilegeLevel(); - - public abstract Optional getEppTarget(); - public abstract Optional getStatus(); - public abstract Integer getAttempts(); - - @Override - public String getTableId() { - return TABLE_ID; - } - - @Override - public ImmutableList getSchemaFields() { - return SCHEMA_FIELDS; - } - - @Override - public ImmutableMap getBigQueryRowEncoding() { - // Create map builder, start with required values - ImmutableMap.Builder map = - new ImmutableMap.Builder() - .put("requestId", getRequestId()) - .put("startTime", toBigqueryTimestamp(getStartTimestamp())) - .put("endTime", toBigqueryTimestamp(getEndTimestamp())) - .put("attempts", getAttempts().toString()); - // Populate optional values, if present - addOptional("commandName", getCommandName(), map); - addOptional("clientId", getClientId(), map); - addOptional("tld", getTld(), map); - addOptional("privilegeLevel", getPrivilegeLevel(), map); - addOptional("eppTarget", getEppTarget(), map); - if (getStatus().isPresent()) { - map.put("eppStatus", Integer.toString(getStatus().get().code)); - } - - return map.build(); - } - - /** - * Helper method to populate an {@link com.google.common.collect.ImmutableMap.Builder} with an - * {@link Optional} value if the value is {@link Optional#isPresent()}. - */ - private static void addOptional( - String key, Optional value, ImmutableMap.Builder map) { - value.ifPresent(t -> map.put(key, t.toString())); - } - /** Create an {@link EppMetric.Builder}. */ public static Builder builder() { return new AutoValue_EppMetric.Builder(); @@ -124,9 +52,8 @@ public abstract class EppMetric implements BigQueryMetric { * *

The start timestamp is recorded now, and the end timestamp at {@code build()}. */ - public static Builder builderForRequest(String requestId, Clock clock) { + public static Builder builderForRequest(Clock clock) { return builder() - .setRequestId(requestId) .setStartTimestamp(clock.nowUtc()) .setClock(clock); } @@ -135,14 +62,9 @@ public abstract class EppMetric implements BigQueryMetric { @AutoValue.Builder public abstract static class Builder { - /** Builder-only counter of the number of attempts, to support {@link #incrementAttempts()}. */ - private int attempts = 0; - /** Builder-only clock to support automatic recording of endTimestamp on {@link #build()}. */ private Clock clock = null; - abstract Builder setRequestId(String requestId); - abstract Builder setStartTimestamp(DateTime startTimestamp); abstract Builder setEndTimestamp(DateTime endTimestamp); @@ -191,19 +113,8 @@ public abstract class EppMetric implements BigQueryMetric { return this; } - public abstract Builder setPrivilegeLevel(String privilegeLevel); - - public abstract Builder setEppTarget(String eppTarget); - public abstract Builder setStatus(Code code); - abstract Builder setAttempts(Integer attempts); - - public Builder incrementAttempts() { - attempts++; - return this; - } - Builder setClock(Clock clock) { this.clock = clock; return this; @@ -216,7 +127,6 @@ public abstract class EppMetric implements BigQueryMetric { * current timestamp of the clock; otherwise end timestamp must have been previously set. */ public EppMetric build() { - setAttempts(attempts); if (clock != null) { setEndTimestamp(clock.nowUtc()); } diff --git a/java/google/registry/monitoring/whitebox/MetricsExportAction.java b/java/google/registry/monitoring/whitebox/MetricsExportAction.java deleted file mode 100644 index f5a802ff1..000000000 --- a/java/google/registry/monitoring/whitebox/MetricsExportAction.java +++ /dev/null @@ -1,104 +0,0 @@ -// Copyright 2017 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.whitebox; - -import static com.google.common.base.Predicates.in; -import static com.google.common.base.Predicates.not; -import static com.google.common.collect.Multimaps.filterKeys; -import static google.registry.request.Action.Method.POST; -import static java.util.stream.Collectors.joining; - -import com.google.api.services.bigquery.Bigquery; -import com.google.api.services.bigquery.model.TableDataInsertAllRequest; -import com.google.api.services.bigquery.model.TableDataInsertAllResponse; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableListMultimap; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; -import com.google.common.flogger.FluentLogger; -import google.registry.bigquery.CheckedBigquery; -import google.registry.config.RegistryConfig.Config; -import google.registry.request.Action; -import google.registry.request.Parameter; -import google.registry.request.ParameterMap; -import google.registry.request.auth.Auth; -import java.io.IOException; -import java.util.Map; -import javax.inject.Inject; - -/** Action for exporting metrics to BigQuery. */ -@Action( - path = MetricsExportAction.PATH, - method = POST, - auth = Auth.AUTH_INTERNAL_ONLY -) -public class MetricsExportAction implements Runnable { - - public static final String PATH = "/_dr/task/metrics"; - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - private static final String DATASET_ID = "metrics"; - private static final ImmutableSet SPECIAL_PARAMS = ImmutableSet.of("tableId", "insertId"); - - @Inject @Parameter("tableId") String tableId; - @Inject @Parameter("insertId") String insertId; - @Inject @Config("projectId") String projectId; - - @Inject CheckedBigquery checkedBigquery; - @Inject @ParameterMap ImmutableListMultimap parameters; - @Inject MetricsExportAction() {} - - /** Exports metrics to BigQuery. */ - @Override - public void run() { - try { - Bigquery bigquery = - checkedBigquery.ensureDataSetAndTableExist(projectId, DATASET_ID, tableId); - // Filter out the special parameters that the Action is called with. Everything that's left - // is returned in a Map that is suitable to pass to Bigquery as row data. - Map jsonRows = - ImmutableMap.copyOf( - filterKeys(parameters, not(in(SPECIAL_PARAMS))).entries()); - TableDataInsertAllResponse response = bigquery.tabledata() - .insertAll( - projectId, - DATASET_ID, - tableId, - new TableDataInsertAllRequest() - .setRows( - ImmutableList.of(new TableDataInsertAllRequest.Rows() - .setInsertId(insertId) - .setJson(jsonRows)))) - .execute(); - - if (response.getInsertErrors() != null && !response.getInsertErrors().isEmpty()) { - throw new RuntimeException( - response - .getInsertErrors() - .stream() - .map( - error -> { - try { - return error.toPrettyString(); - } catch (IOException e) { - return error.toString(); - } - }) - .collect(joining("\n"))); - } - } catch (Throwable e) { - logger.atWarning().withCause(e).log("Unknown error while exporting metrics to BigQuery."); - } - } -} diff --git a/java/google/registry/monitoring/whitebox/WhiteboxModule.java b/java/google/registry/monitoring/whitebox/WhiteboxModule.java index 2203964b4..1c67aa483 100644 --- a/java/google/registry/monitoring/whitebox/WhiteboxModule.java +++ b/java/google/registry/monitoring/whitebox/WhiteboxModule.java @@ -14,24 +14,9 @@ package google.registry.monitoring.whitebox; -import static com.google.appengine.api.taskqueue.QueueFactory.getQueue; -import static google.registry.monitoring.whitebox.BigQueryMetricsEnqueuer.QUEUE_BIGQUERY_STREAMING_METRICS; -import static google.registry.request.RequestParameters.extractRequiredParameter; - -import com.google.api.services.bigquery.model.TableFieldSchema; -import com.google.appengine.api.taskqueue.Queue; -import com.google.common.base.Supplier; -import com.google.common.collect.ImmutableList; import dagger.Module; import dagger.Provides; -import dagger.multibindings.IntoMap; -import dagger.multibindings.StringKey; -import google.registry.request.Parameter; -import google.registry.request.RequestLogId; import google.registry.util.Clock; -import java.util.UUID; -import javax.inject.Named; -import javax.servlet.http.HttpServletRequest; /** * Dagger module for injecting common settings for Whitebox tasks. @@ -39,46 +24,14 @@ import javax.servlet.http.HttpServletRequest; @Module public class WhiteboxModule { - @Provides - @IntoMap - @StringKey(EppMetric.TABLE_ID) - static ImmutableList provideEppMetricsSchema() { - return EppMetric.SCHEMA_FIELDS; - } - - @Provides - @Parameter("tableId") - static String provideTableId(HttpServletRequest req) { - return extractRequiredParameter(req, "tableId"); - } - - @Provides - @Parameter("insertId") - static String provideInsertId(HttpServletRequest req) { - return extractRequiredParameter(req, "insertId"); - } - - @Provides - @Named("insertIdGenerator") - static Supplier provideInsertIdGenerator() { - return () -> UUID.randomUUID().toString(); - } - /** Provides an EppMetric builder with the request ID and startTimestamp already initialized. */ @Provides - static EppMetric.Builder provideEppMetricBuilder( - @RequestLogId String requestLogId, Clock clock) { - return EppMetric.builderForRequest(requestLogId, clock); + static EppMetric.Builder provideEppMetricBuilder(Clock clock) { + return EppMetric.builderForRequest(clock); } @Provides static CheckApiMetric.Builder provideCheckApiMetricBuilder(Clock clock) { return CheckApiMetric.builder(clock); } - - @Provides - @Named(QUEUE_BIGQUERY_STREAMING_METRICS) - static Queue provideBigQueryStreamingMetricsQueue() { - return getQueue(QUEUE_BIGQUERY_STREAMING_METRICS); - } } diff --git a/javatests/google/registry/flows/EppCommitLogsTest.java b/javatests/google/registry/flows/EppCommitLogsTest.java index aa07fa2ff..0ed3dddd0 100644 --- a/javatests/google/registry/flows/EppCommitLogsTest.java +++ b/javatests/google/registry/flows/EppCommitLogsTest.java @@ -70,7 +70,7 @@ public class EppCommitLogsTest extends ShardableTestCase { sessionMetadata.setClientId("TheRegistrar"); DaggerEppTestComponent.builder() .fakesAndMocksModule( - FakesAndMocksModule.create(clock, EppMetric.builderForRequest("request-id-1", clock))) + FakesAndMocksModule.create(clock, EppMetric.builderForRequest(clock))) .build() .startRequest() .flowComponentBuilder() diff --git a/javatests/google/registry/flows/EppControllerTest.java b/javatests/google/registry/flows/EppControllerTest.java index 7c062e5df..1e882aca6 100644 --- a/javatests/google/registry/flows/EppControllerTest.java +++ b/javatests/google/registry/flows/EppControllerTest.java @@ -16,7 +16,6 @@ package google.registry.flows; import static com.google.common.io.BaseEncoding.base64; import static com.google.common.truth.Truth.assertThat; -import static com.google.common.truth.Truth8.assertThat; import static google.registry.flows.EppXmlTransformer.marshal; import static google.registry.testing.DatastoreHelper.createTld; import static google.registry.testing.LogsSubject.assertAboutLogs; @@ -40,7 +39,6 @@ import google.registry.model.eppoutput.EppOutput; import google.registry.model.eppoutput.EppResponse; import google.registry.model.eppoutput.Result; import google.registry.model.eppoutput.Result.Code; -import google.registry.monitoring.whitebox.BigQueryMetricsEnqueuer; import google.registry.monitoring.whitebox.EppMetric; import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; @@ -60,7 +58,6 @@ import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -import org.mockito.ArgumentCaptor; import org.mockito.Matchers; import org.mockito.Mock; @@ -74,7 +71,6 @@ public class EppControllerTest extends ShardableTestCase { @Mock SessionMetadata sessionMetadata; @Mock TransportCredentials transportCredentials; @Mock EppMetrics eppMetrics; - @Mock BigQueryMetricsEnqueuer bigQueryMetricsEnqueuer; @Mock FlowComponent.Builder flowComponentBuilder; @Mock FlowComponent flowComponent; @Mock FlowRunner flowRunner; @@ -111,9 +107,8 @@ public class EppControllerTest extends ShardableTestCase { when(result.getCode()).thenReturn(Code.SUCCESS_WITH_NO_MESSAGES); eppController = new EppController(); - eppController.eppMetricBuilder = EppMetric.builderForRequest("request-id-1", clock); + eppController.eppMetricBuilder = EppMetric.builderForRequest(clock); when(flowRunner.run(eppController.eppMetricBuilder)).thenReturn(eppOutput); - eppController.bigQueryMetricsEnqueuer = bigQueryMetricsEnqueuer; eppController.flowComponentBuilder = flowComponentBuilder; eppController.eppMetrics = eppMetrics; eppController.serverTridProvider = new FakeServerTridProvider(); @@ -132,49 +127,6 @@ public class EppControllerTest extends ShardableTestCase { ValidationMode.STRICT); } - @Test - public void testHandleEppCommand_unmarshallableData_exportsMetric() { - eppController.handleEppCommand( - sessionMetadata, - transportCredentials, - EppRequestSource.UNIT_TEST, - false, - false, - new byte[0]); - - ArgumentCaptor metricCaptor = ArgumentCaptor.forClass(EppMetric.class); - verify(bigQueryMetricsEnqueuer).export(metricCaptor.capture()); - EppMetric metric = metricCaptor.getValue(); - assertThat(metric.getRequestId()).isEqualTo("request-id-1"); - assertThat(metric.getStartTimestamp()).isEqualTo(START_TIME); - assertThat(metric.getEndTimestamp()).isEqualTo(clock.nowUtc()); - assertThat(metric.getClientId()).hasValue("some-client"); - assertThat(metric.getPrivilegeLevel()).hasValue("NORMAL"); - assertThat(metric.getStatus()).hasValue(Code.SYNTAX_ERROR); - } - - @Test - public void testHandleEppCommand_regularEppCommand_exportsBigQueryMetric() { - eppController.handleEppCommand( - sessionMetadata, - transportCredentials, - EppRequestSource.UNIT_TEST, - false, - true, - domainCreateXml.getBytes(UTF_8)); - - ArgumentCaptor metricCaptor = ArgumentCaptor.forClass(EppMetric.class); - verify(bigQueryMetricsEnqueuer).export(metricCaptor.capture()); - EppMetric metric = metricCaptor.getValue(); - assertThat(metric.getRequestId()).isEqualTo("request-id-1"); - assertThat(metric.getStartTimestamp()).isEqualTo(START_TIME); - assertThat(metric.getEndTimestamp()).isEqualTo(clock.nowUtc()); - assertThat(metric.getClientId()).hasValue("some-client"); - assertThat(metric.getPrivilegeLevel()).hasValue("SUPERUSER"); - assertThat(metric.getStatus()).hasValue(Code.SUCCESS_WITH_NO_MESSAGES); - assertThat(metric.getEppTarget()).hasValue("example.tld"); - } - @Test public void testHandleEppCommand_regularEppCommand_exportsEppMetrics() { createTld("tld"); @@ -182,12 +134,10 @@ public class EppControllerTest extends ShardableTestCase { // FlowRunner, not EppController, and since FlowRunner is mocked out for these tests they won't // actually get values. EppMetric.Builder metricBuilder = - EppMetric.builderForRequest("request-id-1", clock) + EppMetric.builderForRequest(clock) .setClientId("some-client") - .setEppTarget("example.tld") .setStatus(Code.SUCCESS_WITH_NO_MESSAGES) - .setTld("tld") - .setPrivilegeLevel("SUPERUSER"); + .setTld("tld"); eppController.handleEppCommand( sessionMetadata, transportCredentials, @@ -210,8 +160,6 @@ public class EppControllerTest extends ShardableTestCase { true, true, domainCreateXml.getBytes(UTF_8)); - - verifyZeroInteractions(bigQueryMetricsEnqueuer); verifyZeroInteractions(eppMetrics); } diff --git a/javatests/google/registry/flows/EppLifecycleContactTest.java b/javatests/google/registry/flows/EppLifecycleContactTest.java index aa6f91c73..bdb33f4d3 100644 --- a/javatests/google/registry/flows/EppLifecycleContactTest.java +++ b/javatests/google/registry/flows/EppLifecycleContactTest.java @@ -52,8 +52,6 @@ public class EppLifecycleContactTest extends EppTestCase { .and() .hasCommandName("ContactCreate") .and() - .hasEppTarget("sh8013") - .and() .hasStatus(SUCCESS); assertThatCommand("contact_info.xml") .atTime("2000-06-01T00:01:00Z") @@ -63,8 +61,6 @@ public class EppLifecycleContactTest extends EppTestCase { .and() .hasCommandName("ContactInfo") .and() - .hasEppTarget("sh8013") - .and() .hasStatus(SUCCESS); assertThatCommand("contact_delete_sh8013.xml") .hasResponse("contact_delete_response_sh8013.xml"); @@ -73,8 +69,6 @@ public class EppLifecycleContactTest extends EppTestCase { .and() .hasCommandName("ContactDelete") .and() - .hasEppTarget("sh8013") - .and() .hasStatus(SUCCESS_WITH_ACTION_PENDING); assertThatLogoutSucceeds(); } diff --git a/javatests/google/registry/flows/EppLifecycleDomainTest.java b/javatests/google/registry/flows/EppLifecycleDomainTest.java index b4274518b..edd705e77 100644 --- a/javatests/google/registry/flows/EppLifecycleDomainTest.java +++ b/javatests/google/registry/flows/EppLifecycleDomainTest.java @@ -440,8 +440,6 @@ public class EppLifecycleDomainTest extends EppTestCase { .and() .hasCommandName("HostUpdate") .and() - .hasEppTarget("ns3.fakesite.example") - .and() .hasStatus(SUCCESS); // Delete the fakesite.example domain (which should succeed since it no longer has subords). assertThatCommand("domain_delete.xml", ImmutableMap.of("DOMAIN", "fakesite.example")) @@ -454,8 +452,6 @@ public class EppLifecycleDomainTest extends EppTestCase { .and() .hasCommandName("DomainDelete") .and() - .hasEppTarget("fakesite.example") - .and() .hasStatus(SUCCESS_WITH_ACTION_PENDING); // Check info on the renamed host and verify that it's still around and wasn't deleted. assertThatCommand("host_info_ns9000_example.xml") @@ -466,8 +462,6 @@ public class EppLifecycleDomainTest extends EppTestCase { .and() .hasCommandName("HostInfo") .and() - .hasEppTarget("ns9000.example.external") - .and() .hasStatus(SUCCESS); assertThatLogoutSucceeds(); assertThat(getRecordedEppMetric()) @@ -575,8 +569,6 @@ public class EppLifecycleDomainTest extends EppTestCase { .and() .hasCommandName("DomainCheck") .and() - .hasEppTarget("rich.example") - .and() .hasTld("example") .and() .hasStatus(SUCCESS); diff --git a/javatests/google/registry/flows/EppLifecycleHostTest.java b/javatests/google/registry/flows/EppLifecycleHostTest.java index fb918c292..2328424c3 100644 --- a/javatests/google/registry/flows/EppLifecycleHostTest.java +++ b/javatests/google/registry/flows/EppLifecycleHostTest.java @@ -67,8 +67,6 @@ public class EppLifecycleHostTest extends EppTestCase { .and() .hasCommandName("HostCreate") .and() - .hasEppTarget("ns1.example.tld") - .and() .hasStatus(SUCCESS); assertThatCommand("host_info.xml", ImmutableMap.of("HOSTNAME", "ns1.example.tld")) .atTime("2000-06-02T00:02:00Z") @@ -81,8 +79,6 @@ public class EppLifecycleHostTest extends EppTestCase { .and() .hasCommandName("HostInfo") .and() - .hasEppTarget("ns1.example.tld") - .and() .hasStatus(SUCCESS); assertThatCommand("host_delete.xml", ImmutableMap.of("HOSTNAME", "ns1.example.tld")) .atTime("2000-06-02T00:03:00Z") @@ -92,8 +88,6 @@ public class EppLifecycleHostTest extends EppTestCase { .and() .hasCommandName("HostDelete") .and() - .hasEppTarget("ns1.example.tld") - .and() .hasStatus(SUCCESS_WITH_ACTION_PENDING); assertThatLogoutSucceeds(); } diff --git a/javatests/google/registry/flows/EppTestCase.java b/javatests/google/registry/flows/EppTestCase.java index d76e602cf..9385849f4 100644 --- a/javatests/google/registry/flows/EppTestCase.java +++ b/javatests/google/registry/flows/EppTestCase.java @@ -167,7 +167,7 @@ public class EppTestCase extends ShardableTestCase { EppRequestHandler handler = new EppRequestHandler(); FakeResponse response = new FakeResponse(); handler.response = response; - eppMetricBuilder = EppMetric.builderForRequest("request-id-1", clock); + eppMetricBuilder = EppMetric.builderForRequest(clock); handler.eppController = DaggerEppTestComponent.builder() .fakesAndMocksModule(FakesAndMocksModule.create(clock, eppMetricBuilder)) .build() diff --git a/javatests/google/registry/flows/EppTestComponent.java b/javatests/google/registry/flows/EppTestComponent.java index d2e86259b..cfe140422 100644 --- a/javatests/google/registry/flows/EppTestComponent.java +++ b/javatests/google/registry/flows/EppTestComponent.java @@ -32,7 +32,6 @@ import google.registry.flows.async.AsyncFlowEnqueuer; import google.registry.flows.custom.CustomLogicFactory; import google.registry.flows.custom.TestCustomLogicFactory; import google.registry.flows.domain.DomainFlowTmchUtils; -import google.registry.monitoring.whitebox.BigQueryMetricsEnqueuer; import google.registry.monitoring.whitebox.EppMetric; import google.registry.request.RequestScope; import google.registry.request.lock.LockHandler; @@ -64,7 +63,6 @@ interface EppTestComponent { class FakesAndMocksModule { private AsyncFlowEnqueuer asyncFlowEnqueuer; - private BigQueryMetricsEnqueuer metricsEnqueuer; private DnsQueue dnsQueue; private DomainFlowTmchUtils domainFlowTmchUtils; private EppMetric.Builder metricBuilder; @@ -75,7 +73,7 @@ interface EppTestComponent { public static FakesAndMocksModule create() { FakeClock clock = new FakeClock(); - return create(clock, EppMetric.builderForRequest("request-id-1", clock)); + return create(clock, EppMetric.builderForRequest(clock)); } public static FakesAndMocksModule create(FakeClock clock, EppMetric.Builder metricBuilder) { @@ -106,7 +104,6 @@ interface EppTestComponent { instance.dnsQueue = DnsQueue.create(); instance.metricBuilder = eppMetricBuilder; instance.appEngineServiceUtils = appEngineServiceUtils; - instance.metricsEnqueuer = mock(BigQueryMetricsEnqueuer.class); instance.lockHandler = new FakeLockHandler(true); return instance; } @@ -116,11 +113,6 @@ interface EppTestComponent { return asyncFlowEnqueuer; } - @Provides - BigQueryMetricsEnqueuer provideBigQueryMetricsEnqueuer() { - return metricsEnqueuer; - } - @Provides Clock provideClock() { return clock; diff --git a/javatests/google/registry/flows/FlowRunnerTest.java b/javatests/google/registry/flows/FlowRunnerTest.java index 82a3362fa..c828425ba 100644 --- a/javatests/google/registry/flows/FlowRunnerTest.java +++ b/javatests/google/registry/flows/FlowRunnerTest.java @@ -54,8 +54,7 @@ public class FlowRunnerTest extends ShardableTestCase { public final AppEngineRule appEngineRule = new AppEngineRule.Builder().build(); private final FlowRunner flowRunner = new FlowRunner(); - private final EppMetric.Builder eppMetricBuilder = - EppMetric.builderForRequest("request-id-1", new FakeClock()); + private final EppMetric.Builder eppMetricBuilder = EppMetric.builderForRequest(new FakeClock()); private final TestLogHandler handler = new TestLogHandler(); @@ -84,19 +83,6 @@ public class FlowRunnerTest extends ShardableTestCase { flowRunner.flowReporter = Mockito.mock(FlowReporter.class); } - @Test - public void testRun_nonTransactionalCommand_incrementsMetricAttempts() throws Exception { - flowRunner.run(eppMetricBuilder); - assertThat(eppMetricBuilder.build().getAttempts()).isEqualTo(1); - } - - @Test - public void testRun_transactionalCommand_incrementsMetricAttempts() throws Exception { - flowRunner.isTransactional = true; - flowRunner.run(eppMetricBuilder); - assertThat(eppMetricBuilder.build().getAttempts()).isEqualTo(1); - } - @Test public void testRun_nonTransactionalCommand_setsCommandNameOnMetric() throws Exception { flowRunner.isTransactional = true; diff --git a/javatests/google/registry/flows/FlowTestCase.java b/javatests/google/registry/flows/FlowTestCase.java index 7db1baca2..145730833 100644 --- a/javatests/google/registry/flows/FlowTestCase.java +++ b/javatests/google/registry/flows/FlowTestCase.java @@ -278,7 +278,7 @@ public abstract class FlowTestCase extends ShardableTestCase { private EppOutput runFlowInternal(CommitMode commitMode, UserPrivileges userPrivileges) throws Exception { - eppMetricBuilder = EppMetric.builderForRequest("request-id-1", clock); + eppMetricBuilder = EppMetric.builderForRequest(clock); // Assert that the xml triggers the flow we expect. assertThat(FlowPicker.getFlowClass(eppLoader.getEpp())) .isEqualTo(new TypeInstantiator(getClass()){}.getExactType()); diff --git a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java index 1698920c9..da4f3a562 100644 --- a/javatests/google/registry/flows/domain/DomainCreateFlowTest.java +++ b/javatests/google/registry/flows/domain/DomainCreateFlowTest.java @@ -2541,6 +2541,5 @@ public class DomainCreateFlowTest extends ResourceFlowTestCase getSchemaFields() { - return null; - } - - @Override - public ImmutableMap getBigQueryRowEncoding() { - return ImmutableMap.of( - "startTime", toBigqueryTimestamp(getStartTimestamp()), - "endTime", toBigqueryTimestamp(getEndTimestamp())); - } - - abstract DateTime getStartTimestamp(); - - abstract DateTime getEndTimestamp(); - } -} diff --git a/javatests/google/registry/monitoring/whitebox/EppMetricTest.java b/javatests/google/registry/monitoring/whitebox/EppMetricTest.java index 6c21ee1ac..7fd47d227 100644 --- a/javatests/google/registry/monitoring/whitebox/EppMetricTest.java +++ b/javatests/google/registry/monitoring/whitebox/EppMetricTest.java @@ -14,18 +14,13 @@ package google.registry.monitoring.whitebox; -import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; import static google.registry.testing.DatastoreHelper.createTld; import static google.registry.testing.DatastoreHelper.createTlds; -import com.google.api.services.bigquery.model.TableFieldSchema; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import google.registry.model.eppoutput.Result.Code; import google.registry.testing.AppEngineRule; import google.registry.testing.FakeClock; -import org.joda.time.DateTime; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -40,7 +35,7 @@ public class EppMetricTest { @Test public void test_invalidTld_isRecordedAsInvalid() { EppMetric metric = - EppMetric.builderForRequest("request-id-1", new FakeClock()) + EppMetric.builderForRequest(new FakeClock()) .setTlds(ImmutableSet.of("notarealtld")) .build(); assertThat(metric.getTld()).hasValue("_invalid"); @@ -50,9 +45,7 @@ public class EppMetricTest { public void test_validTld_isRecorded() { createTld("example"); EppMetric metric = - EppMetric.builderForRequest("request-id-1", new FakeClock()) - .setTlds(ImmutableSet.of("example")) - .build(); + EppMetric.builderForRequest(new FakeClock()).setTlds(ImmutableSet.of("example")).build(); assertThat(metric.getTld()).hasValue("example"); } @@ -60,7 +53,7 @@ public class EppMetricTest { public void test_multipleTlds_areRecordedAsVarious() { createTlds("foo", "bar"); EppMetric metric = - EppMetric.builderForRequest("request-id-1", new FakeClock()) + EppMetric.builderForRequest(new FakeClock()) .setTlds(ImmutableSet.of("foo", "bar", "baz")) .build(); assertThat(metric.getTld()).hasValue("_various"); @@ -69,64 +62,7 @@ public class EppMetricTest { @Test public void test_zeroTlds_areRecordedAsAbsent() { EppMetric metric = - EppMetric.builderForRequest("request-id-1", new FakeClock()) - .setTlds(ImmutableSet.of()) - .build(); + EppMetric.builderForRequest(new FakeClock()).setTlds(ImmutableSet.of()).build(); assertThat(metric.getTld()).isEmpty(); } - - @Test - public void testGetBigQueryRowEncoding_encodesCorrectly() { - EppMetric metric = - EppMetric.builder() - .setRequestId("request-id-1") - .setStartTimestamp(new DateTime(1337)) - .setEndTimestamp(new DateTime(1338)) - .setCommandName("command") - .setClientId("client") - .setTld("example") - .setPrivilegeLevel("level") - .setEppTarget("target") - .setStatus(Code.COMMAND_USE_ERROR) - .incrementAttempts() - .build(); - - assertThat(metric.getBigQueryRowEncoding()) - .containsExactlyEntriesIn( - new ImmutableMap.Builder() - .put("requestId", "request-id-1") - .put("startTime", "1.337000") - .put("endTime", "1.338000") - .put("commandName", "command") - .put("clientId", "client") - .put("tld", "example") - .put("privilegeLevel", "level") - .put("eppTarget", "target") - .put("eppStatus", "2002") - .put("attempts", "1") - .build()); - } - - @Test - public void testGetBigQueryRowEncoding_hasAllSchemaFields() { - EppMetric metric = - EppMetric.builder() - .setRequestId("request-id-1") - .setStartTimestamp(new DateTime(1337)) - .setEndTimestamp(new DateTime(1338)) - .setCommandName("command") - .setClientId("client") - .setTld("example") - .setPrivilegeLevel("level") - .setEppTarget("target") - .setStatus(Code.COMMAND_USE_ERROR) - .incrementAttempts() - .build(); - ImmutableSet.Builder schemaFieldNames = new ImmutableSet.Builder<>(); - for (TableFieldSchema schemaField : metric.getSchemaFields()) { - schemaFieldNames.add(schemaField.getName()); - } - - assertThat(metric.getBigQueryRowEncoding().keySet()).isEqualTo(schemaFieldNames.build()); - } } diff --git a/javatests/google/registry/monitoring/whitebox/MetricsExportActionTest.java b/javatests/google/registry/monitoring/whitebox/MetricsExportActionTest.java deleted file mode 100644 index 5e54adb64..000000000 --- a/javatests/google/registry/monitoring/whitebox/MetricsExportActionTest.java +++ /dev/null @@ -1,108 +0,0 @@ -// Copyright 2017 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.whitebox; - -import static java.util.concurrent.TimeUnit.MILLISECONDS; -import static org.mockito.Matchers.anyString; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import com.google.api.services.bigquery.Bigquery; -import com.google.api.services.bigquery.Bigquery.Tabledata; -import com.google.api.services.bigquery.Bigquery.Tabledata.InsertAll; -import com.google.api.services.bigquery.model.TableDataInsertAllRequest; -import com.google.api.services.bigquery.model.TableDataInsertAllResponse; -import com.google.api.services.bigquery.model.TableDataInsertAllResponse.InsertErrors; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableListMultimap; -import google.registry.bigquery.CheckedBigquery; -import google.registry.testing.AppEngineRule; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; -import org.mockito.Matchers; - -/** Unit tests for {@link MetricsExportAction}. */ -@RunWith(JUnit4.class) -public class MetricsExportActionTest { - - @Rule - public final AppEngineRule appEngine = - AppEngineRule.builder().withDatastore().withTaskQueue().build(); - - private final CheckedBigquery checkedBigquery = mock(CheckedBigquery.class); - private final Bigquery bigquery = mock(Bigquery.class); - private final Tabledata tabledata = mock(Tabledata.class); - private final InsertAll insertAll = mock(InsertAll.class); - - private TableDataInsertAllResponse response = new TableDataInsertAllResponse(); - private long currentTimeMillis = 1000000000000L; - - private ImmutableListMultimap parameters = - new ImmutableListMultimap.Builder() - .put("startTime", String.valueOf(MILLISECONDS.toSeconds(currentTimeMillis - 100))) - .put("endTime", String.valueOf(MILLISECONDS.toSeconds(currentTimeMillis))) - .put("jobname", "test job") - .put("status", "success") - .put("tld", "test") - .build(); - - MetricsExportAction action; - - @Before - public void setup() throws Exception { - when(checkedBigquery.ensureDataSetAndTableExist(anyString(), anyString(), anyString())) - .thenReturn(bigquery); - - when(bigquery.tabledata()).thenReturn(tabledata); - when(tabledata.insertAll( - anyString(), - anyString(), - anyString(), - Matchers.any(TableDataInsertAllRequest.class))).thenReturn(insertAll); - action = new MetricsExportAction(); - action.checkedBigquery = checkedBigquery; - action.insertId = "insert id"; - action.parameters = parameters; - action.projectId = "project id"; - action.tableId = "eppMetrics"; - } - - @Test - public void testSuccess_nullErrors() throws Exception { - when(insertAll.execute()).thenReturn(response); - response.setInsertErrors(null); - action.run(); - verify(insertAll).execute(); - } - - @Test - public void testSuccess_emptyErrors() throws Exception { - when(insertAll.execute()).thenReturn(response); - response.setInsertErrors(ImmutableList.of()); - action.run(); - verify(insertAll).execute(); - } - - @Test - public void testFailure_errors() throws Exception { - when(insertAll.execute()).thenReturn(response); - response.setInsertErrors(ImmutableList.of(new InsertErrors())); - action.run(); - } -} diff --git a/javatests/google/registry/testing/EppMetricSubject.java b/javatests/google/registry/testing/EppMetricSubject.java index b80ba0e2e..c8187d2f8 100644 --- a/javatests/google/registry/testing/EppMetricSubject.java +++ b/javatests/google/registry/testing/EppMetricSubject.java @@ -44,10 +44,6 @@ public class EppMetricSubject extends Subject { return hasValue(commandName, actual().getCommandName(), "has commandName"); } - public And hasEppTarget(String eppTarget) { - return hasValue(eppTarget, actual().getEppTarget(), "has eppTarget"); - } - public And hasStatus(Code status) { return hasValue(status, actual().getStatus(), "has status"); }