mirror of
https://github.com/google/nomulus.git
synced 2025-06-27 14:54:51 +02:00
Remove old json logging from flows
This was meant for log replay and has long been ignored/useless. As part of this, remove execution time from EppResponse since this was the only thing consuming it. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=137293734
This commit is contained in:
parent
5f8a95d853
commit
198558901d
7 changed files with 20 additions and 103 deletions
|
@ -28,7 +28,6 @@ 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.util.Clock;
|
||||
import google.registry.util.FormattingLogger;
|
||||
import javax.inject.Inject;
|
||||
|
||||
|
@ -41,7 +40,6 @@ public final class EppController {
|
|||
|
||||
private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass();
|
||||
|
||||
@Inject Clock clock;
|
||||
@Inject FlowComponent.Builder flowComponentBuilder;
|
||||
@Inject EppMetric.Builder metricBuilder;
|
||||
@Inject EppMetrics eppMetrics;
|
||||
|
@ -65,7 +63,7 @@ public final class EppController {
|
|||
} catch (EppException e) {
|
||||
// Send the client an error message, with no clTRID since we couldn't unmarshal it.
|
||||
metricBuilder.setStatus(e.getResult().getCode());
|
||||
return getErrorResponse(clock, e.getResult(), Trid.create(null));
|
||||
return getErrorResponse(e.getResult(), Trid.create(null));
|
||||
}
|
||||
metricBuilder.setCommandName(eppInput.getCommandName());
|
||||
if (!eppInput.getTargetIds().isEmpty()) {
|
||||
|
@ -101,21 +99,20 @@ public final class EppController {
|
|||
} catch (EppException | EppExceptionInProviderException e) {
|
||||
// The command failed. Send the client an error message.
|
||||
EppException eppEx = (EppException) (e instanceof EppException ? e : e.getCause());
|
||||
return getErrorResponse(clock, eppEx.getResult(), flowComponent.trid());
|
||||
return getErrorResponse(eppEx.getResult(), flowComponent.trid());
|
||||
} catch (Throwable e) {
|
||||
// Something bad and unexpected happened. Send the client a generic error, and log it.
|
||||
logger.severe(e, "Unexpected failure");
|
||||
return getErrorResponse(clock, Result.create(Code.COMMAND_FAILED), flowComponent.trid());
|
||||
return getErrorResponse(Result.create(Code.COMMAND_FAILED), flowComponent.trid());
|
||||
}
|
||||
}
|
||||
|
||||
/** Create a response indicating an EPP failure. */
|
||||
@VisibleForTesting
|
||||
static EppOutput getErrorResponse(Clock clock, Result result, Trid trid) {
|
||||
static EppOutput getErrorResponse(Result result, Trid trid) {
|
||||
return EppOutput.create(new EppResponse.Builder()
|
||||
.setResult(result)
|
||||
.setTrid(trid)
|
||||
.setExecutionTime(clock.nowUtc())
|
||||
.build());
|
||||
}
|
||||
}
|
||||
|
|
|
@ -86,7 +86,6 @@ public abstract class Flow {
|
|||
.setMessageQueueInfo(messageQueueInfo)
|
||||
.setResData(responseData)
|
||||
.setExtensions(responseExtensions)
|
||||
.setExecutionTime(now)
|
||||
.build());
|
||||
}
|
||||
|
||||
|
|
|
@ -101,18 +101,8 @@ public class FlowRunner {
|
|||
metric.incrementAttempts();
|
||||
return createAndInitFlow(clock.nowUtc()).run();
|
||||
}
|
||||
// We log the command in a structured format. Note that we do this before the transaction;
|
||||
// if we did it after, we might miss a transaction that committed successfully but then crashed
|
||||
// before it could log.
|
||||
logger.info("EPP_Mutation " + new JsonLogStatement(trid)
|
||||
.add("client", clientId)
|
||||
.add("privileges", isSuperuser ? "SUPERUSER" : "NORMAL")
|
||||
.add("xmlBytes", xmlBase64));
|
||||
try {
|
||||
EppOutput flowResult =
|
||||
ofy()
|
||||
.transact(
|
||||
new Work<EppOutput>() {
|
||||
return ofy().transact(new Work<EppOutput>() {
|
||||
@Override
|
||||
public EppOutput run() {
|
||||
metric.incrementAttempts();
|
||||
|
@ -125,15 +115,10 @@ public class FlowRunner {
|
|||
} catch (EppException e) {
|
||||
throw new RuntimeException(e);
|
||||
}
|
||||
}
|
||||
});
|
||||
logger.info("EPP_Mutation_Committed " + new JsonLogStatement(trid)
|
||||
.add("executionTime", flowResult.getResponse().getExecutionTime().getMillis()));
|
||||
return flowResult;
|
||||
}});
|
||||
} catch (DryRunException e) {
|
||||
return e.output;
|
||||
} catch (RuntimeException e) {
|
||||
logger.warning("EPP_Mutation_Failed " + new JsonLogStatement(trid));
|
||||
logger.warning(getStackTraceAsString(e));
|
||||
if (e.getCause() instanceof EppException) {
|
||||
throw (EppException) e.getCause();
|
||||
|
@ -151,43 +136,6 @@ public class FlowRunner {
|
|||
isSuperuser,
|
||||
now);
|
||||
}
|
||||
|
||||
/**
|
||||
* Helper for logging in json format.
|
||||
*
|
||||
* <p>This is needed because the usual json outputters perform normalizations that we don't want
|
||||
* or need, since we know that our values never need to be escaped - there are only strings and
|
||||
* numbers, and the strings are not allowed to contain quote characters.
|
||||
*
|
||||
* <p>An example output for an EPP_Mutation: {"trid":"abc-123", "client":"some_registrar",
|
||||
* "tld":"com", "xmlBytes":"abc123DEF"}
|
||||
*
|
||||
* <p>An example output for an EPP_Mutation_Committed that doesn't create a new resource:
|
||||
* {"trid":"abc-123", "executionTime":123456789}
|
||||
*/
|
||||
private static class JsonLogStatement {
|
||||
|
||||
StringBuilder message;
|
||||
|
||||
JsonLogStatement(Trid trid) {
|
||||
message =
|
||||
new StringBuilder("{\"trid\":\"").append(trid.getServerTransactionId()).append('\"');
|
||||
}
|
||||
|
||||
JsonLogStatement add(String key, Object value) {
|
||||
if (value != null) {
|
||||
String quote = value instanceof String ? "\"" : "";
|
||||
message.append(String.format(", \"%s\":%s%s%s", key, quote, value, quote));
|
||||
}
|
||||
return this;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String toString() {
|
||||
return message + "}";
|
||||
}
|
||||
}
|
||||
|
||||
/** Exception for canceling a transaction while capturing what the output would have been. */
|
||||
private static class DryRunException extends RuntimeException {
|
||||
final EppOutput output;
|
||||
|
|
|
@ -68,9 +68,7 @@ import javax.xml.bind.annotation.XmlElement;
|
|||
import javax.xml.bind.annotation.XmlElementRef;
|
||||
import javax.xml.bind.annotation.XmlElementRefs;
|
||||
import javax.xml.bind.annotation.XmlElementWrapper;
|
||||
import javax.xml.bind.annotation.XmlTransient;
|
||||
import javax.xml.bind.annotation.XmlType;
|
||||
import org.joda.time.DateTime;
|
||||
|
||||
/**
|
||||
* The EppResponse class represents an EPP response message.
|
||||
|
@ -91,14 +89,6 @@ public class EppResponse extends ImmutableObject implements ResponseOrGreeting {
|
|||
/** The command result. The RFC allows multiple failure results, but we always return one. */
|
||||
Result result;
|
||||
|
||||
/**
|
||||
* The time the command that created this response was executed.
|
||||
*
|
||||
* <p>This is for logging purposes only and is not returned to the user.
|
||||
*/
|
||||
@XmlTransient
|
||||
DateTime executionTime;
|
||||
|
||||
/**
|
||||
* Information about messages queued for retrieval. This may appear in response to any EPP message
|
||||
* (if messages are queued), but in practice this will only be set in response to a poll request.
|
||||
|
@ -158,10 +148,6 @@ public class EppResponse extends ImmutableObject implements ResponseOrGreeting {
|
|||
@XmlElementWrapper(name = "extension")
|
||||
ImmutableList<? extends ResponseExtension> extensions;
|
||||
|
||||
public DateTime getExecutionTime() {
|
||||
return executionTime;
|
||||
}
|
||||
|
||||
public ImmutableList<? extends ResponseData> getResponseData() {
|
||||
return resData;
|
||||
}
|
||||
|
@ -221,11 +207,6 @@ public class EppResponse extends ImmutableObject implements ResponseOrGreeting {
|
|||
return this;
|
||||
}
|
||||
|
||||
public Builder setExecutionTime(DateTime executionTime) {
|
||||
getInstance().executionTime = executionTime;
|
||||
return this;
|
||||
}
|
||||
|
||||
public Builder setResData(@Nullable ImmutableList<? extends ResponseData> resData) {
|
||||
getInstance().resData = resData;
|
||||
return this;
|
||||
|
|
|
@ -33,7 +33,6 @@ import google.registry.testing.AppEngineRule;
|
|||
import google.registry.testing.FakeClock;
|
||||
import google.registry.testing.ShardableTestCase;
|
||||
import google.registry.util.Clock;
|
||||
import google.registry.util.SystemClock;
|
||||
import google.registry.xml.ValidationMode;
|
||||
import org.joda.time.DateTime;
|
||||
import org.junit.Before;
|
||||
|
@ -85,7 +84,6 @@ public class EppControllerTest extends ShardableTestCase {
|
|||
eppController = new EppController();
|
||||
eppController.metricBuilder = EppMetric.builderForRequest("request-id-1", clock);
|
||||
eppController.bigQueryMetricsEnqueuer = metricsEnqueuer;
|
||||
eppController.clock = clock;
|
||||
eppController.flowComponentBuilder = flowComponentBuilder;
|
||||
eppController.eppMetrics = eppMetrics;
|
||||
}
|
||||
|
@ -93,8 +91,7 @@ public class EppControllerTest extends ShardableTestCase {
|
|||
@Test
|
||||
public void testMarshallingUnknownError() throws Exception {
|
||||
marshal(
|
||||
EppController.getErrorResponse(
|
||||
new SystemClock(), Result.create(Code.COMMAND_FAILED), Trid.create(null)),
|
||||
EppController.getErrorResponse(Result.create(Code.COMMAND_FAILED), Trid.create(null)),
|
||||
ValidationMode.STRICT);
|
||||
}
|
||||
|
||||
|
|
|
@ -44,7 +44,6 @@ import java.util.List;
|
|||
import java.util.Map;
|
||||
import java.util.logging.LogRecord;
|
||||
import java.util.logging.Logger;
|
||||
import org.joda.time.DateTime;
|
||||
import org.json.simple.JSONValue;
|
||||
import org.junit.Before;
|
||||
import org.junit.Rule;
|
||||
|
@ -69,7 +68,6 @@ public class FlowRunnerTest extends ShardableTestCase {
|
|||
|
||||
final EppOutput eppOutput = mock(EppOutput.class);
|
||||
EppResponse eppResponse = mock(EppResponse.class);
|
||||
when(eppResponse.getExecutionTime()).thenReturn(new DateTime(1337));
|
||||
when(eppOutput.getResponse()).thenReturn(eppResponse);
|
||||
|
||||
flowRunner.clientId = "TheRegistrar";
|
||||
|
@ -83,8 +81,7 @@ public class FlowRunnerTest extends ShardableTestCase {
|
|||
@Override
|
||||
protected EppOutput run() {
|
||||
return eppOutput;
|
||||
}
|
||||
});
|
||||
}});
|
||||
flowRunner.inputXmlBytes = "<xml/>".getBytes(UTF_8);
|
||||
flowRunner.isDryRun = false;
|
||||
flowRunner.isSuperuser = false;
|
||||
|
|
|
@ -17,7 +17,6 @@ package google.registry.testing;
|
|||
import static com.google.common.truth.Truth.assertAbout;
|
||||
import static com.google.common.truth.Truth.assertThat;
|
||||
import static google.registry.flows.EppXmlTransformer.marshal;
|
||||
import static google.registry.util.DateTimeUtils.START_OF_TIME;
|
||||
|
||||
import com.google.common.truth.AbstractVerb.DelegatedVerb;
|
||||
import com.google.common.truth.FailureStrategy;
|
||||
|
@ -50,7 +49,6 @@ public class EppExceptionSubject extends Subject<EppExceptionSubject, EppExcepti
|
|||
EppOutput.create(new EppResponse.Builder()
|
||||
.setTrid(Trid.create(null))
|
||||
.setResult(actual().getResult())
|
||||
.setExecutionTime(START_OF_TIME)
|
||||
.build()),
|
||||
ValidationMode.STRICT);
|
||||
} catch (XmlException e) {
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue