zydronium/google-nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus.git synced 2025-05-16 09:27:16 +02:00
Code Issues Projects Releases Packages Wiki Activity

Remove TLD @Parameter from inside the DnsMetrics

Browse source 
Currently, the TLD is a "hidden" part of the metric - we record it, but by looking at the call site you can't see that we record it.

Also, it's injected from the query / POST parameter, so we might not even be aware of what the value is and it might not be the value we wanted.

Instead, making it explicit in the Metric call. That way it's also more similar to the "logging" statements that record the same data but have to explicitly output the TLD.

It also makes the tests better, as we test that we indeed record the correct TLD

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=229550115
This commit is contained in:
guyben 2019-01-16 06:57:01 -08:00 • committed by jianglai
parent bc798f33e9
commit 8cfb748316
3 changed files with 52 additions and 43 deletions
Download patch file Download diff file Expand all files Collapse all files

16
java/google/registry/dns/DnsMetrics.java
View file

@ -14,8 +14,6 @@
package google.registry.dns; package google.registry.dns;
import static google.registry.request.RequestParameters.PARAM_TLD;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.monitoring.metrics.DistributionFitter; import com.google.monitoring.metrics.DistributionFitter;
import com.google.monitoring.metrics.EventMetric; import com.google.monitoring.metrics.EventMetric;
@ -25,7 +23,6 @@ import com.google.monitoring.metrics.IncrementableMetric;
import com.google.monitoring.metrics.LabelDescriptor; import com.google.monitoring.metrics.LabelDescriptor;
import com.google.monitoring.metrics.MetricRegistryImpl; import com.google.monitoring.metrics.MetricRegistryImpl;
import google.registry.config.RegistryEnvironment; import google.registry.config.RegistryEnvironment;
import google.registry.request.Parameter;
import javax.inject.Inject; import javax.inject.Inject;
import org.joda.time.Duration; import org.joda.time.Duration;
@ -184,7 +181,6 @@ public class DnsMetrics {
EXPONENTIAL_FITTER); EXPONENTIAL_FITTER);
@Inject RegistryEnvironment registryEnvironment; @Inject RegistryEnvironment registryEnvironment;
@Inject @Parameter(PARAM_TLD) String tld;
@Inject @Inject
DnsMetrics() {} DnsMetrics() {}
@ -193,7 +189,7 @@ public class DnsMetrics {
* Increment a monotonic counter that tracks calls to {@link * Increment a monotonic counter that tracks calls to {@link
* google.registry.dns.writer.DnsWriter#publishDomain(String)}, per TLD. * google.registry.dns.writer.DnsWriter#publishDomain(String)}, per TLD.
*/ */
public void incrementPublishDomainRequests(long numRequests, PublishStatus status) { public void incrementPublishDomainRequests(String tld, long numRequests, PublishStatus status) {
if (numRequests > 0) { if (numRequests > 0) {
publishDomainRequests.incrementBy(numRequests, tld, status.name()); publishDomainRequests.incrementBy(numRequests, tld, status.name());
} }
@ -203,7 +199,7 @@ public class DnsMetrics {
* Increment a monotonic counter that tracks calls to {@link * Increment a monotonic counter that tracks calls to {@link
* google.registry.dns.writer.DnsWriter#publishHost(String)}, per TLD. * google.registry.dns.writer.DnsWriter#publishHost(String)}, per TLD.
*/ */
public void incrementPublishHostRequests(long numRequests, PublishStatus status) { public void incrementPublishHostRequests(String tld, long numRequests, PublishStatus status) {
if (numRequests > 0) { if (numRequests > 0) {
publishHostRequests.incrementBy(numRequests, tld, status.name()); publishHostRequests.incrementBy(numRequests, tld, status.name());
} }
@ -212,12 +208,13 @@ public class DnsMetrics {
/** /**
* Measures information about the entire batched commit, per TLD. * Measures information about the entire batched commit, per TLD.
* *
* The information includes running times (per item and per commit), and batch sizes (per item and * <p>The information includes running times (per item and per commit), and batch sizes (per item
* per commit) * and per commit)
* *
* This is to be used for load testing the system, and will not measure anything in prod. * <p>This is to be used for load testing the system, and will not measure anything in prod.
*/ */
void recordCommit( void recordCommit(
String tld,
String dnsWriter, String dnsWriter,
CommitStatus status, CommitStatus status,
Duration processingDuration, Duration processingDuration,
@ -254,6 +251,7 @@ public class DnsMetrics {
} }
void recordActionResult( void recordActionResult(
String tld,
String dnsWriter, String dnsWriter,
ActionStatus status, ActionStatus status,
int numberOfItems, int numberOfItems,

15
java/google/registry/dns/PublishDnsUpdatesAction.java
View file

@ -96,6 +96,7 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable<Void> {
DateTime now = clock.nowUtc(); DateTime now = clock.nowUtc();
dnsMetrics.recordActionResult( dnsMetrics.recordActionResult(
tld,
dnsWriter, dnsWriter,
status, status,
nullToEmpty(domains).size() + nullToEmpty(hosts).size(), nullToEmpty(domains).size() + nullToEmpty(hosts).size(),
@ -197,8 +198,8 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable<Void> {
domainsPublished += 1; domainsPublished += 1;
} }
} }
dnsMetrics.incrementPublishDomainRequests(domainsPublished, PublishStatus.ACCEPTED); dnsMetrics.incrementPublishDomainRequests(tld, domainsPublished, PublishStatus.ACCEPTED);
dnsMetrics.incrementPublishDomainRequests(domainsRejected, PublishStatus.REJECTED); dnsMetrics.incrementPublishDomainRequests(tld, domainsRejected, PublishStatus.REJECTED);
int hostsPublished = 0; int hostsPublished = 0;
int hostsRejected = 0; int hostsRejected = 0;
@ -213,8 +214,8 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable<Void> {
hostsPublished += 1; hostsPublished += 1;
} }
} }
dnsMetrics.incrementPublishHostRequests(hostsPublished, PublishStatus.ACCEPTED); dnsMetrics.incrementPublishHostRequests(tld, hostsPublished, PublishStatus.ACCEPTED);
dnsMetrics.incrementPublishHostRequests(hostsRejected, PublishStatus.REJECTED); dnsMetrics.incrementPublishHostRequests(tld, hostsRejected, PublishStatus.REJECTED);
// If we got here it means we managed to stage the entire batch without any errors. // If we got here it means we managed to stage the entire batch without any errors.
// Next we will commit the batch. // Next we will commit the batch.
@ -229,11 +230,7 @@ public final class PublishDnsUpdatesAction implements Runnable, Callable<Void> {
recordActionResult(actionStatus); recordActionResult(actionStatus);
Duration duration = new Duration(timeAtStart, clock.nowUtc()); Duration duration = new Duration(timeAtStart, clock.nowUtc());
dnsMetrics.recordCommit( dnsMetrics.recordCommit(
dnsWriter, tld, dnsWriter, commitStatus, duration, domainsPublished, hostsPublished);
commitStatus,
duration,
domainsPublished,
hostsPublished);
logger.atInfo().log( logger.atInfo().log(
"writer.commit() statistics: TLD: %s, dnsWriter: %s, commitStatus: %s, duration: %s, " "writer.commit() statistics: TLD: %s, dnsWriter: %s, commitStatus: %s, duration: %s, "
+ "domainsPublished: %d, domainsRejected: %d, hostsPublished: %d, hostsRejected: %d", + "domainsPublished: %d, domainsRejected: %d, hostsPublished: %d, hostsRejected: %d",

64
javatests/google/registry/dns/PublishDnsUpdatesActionTest.java
View file

@ -115,13 +115,15 @@ public class PublishDnsUpdatesActionTest {
verify(dnsWriter).publishHost("ns1.example.xn--q9jyb4c"); verify(dnsWriter).publishHost("ns1.example.xn--q9jyb4c");
verify(dnsWriter).commit(); verify(dnsWriter).commit();
verifyNoMoreInteractions(dnsWriter); verifyNoMoreInteractions(dnsWriter);
verify(dnsMetrics).incrementPublishDomainRequests(0, PublishStatus.ACCEPTED); verify(dnsMetrics).incrementPublishDomainRequests("xn--q9jyb4c", 0, PublishStatus.ACCEPTED);
verify(dnsMetrics).incrementPublishDomainRequests(0, PublishStatus.REJECTED); verify(dnsMetrics).incrementPublishDomainRequests("xn--q9jyb4c", 0, PublishStatus.REJECTED);
verify(dnsMetrics).incrementPublishHostRequests(1, PublishStatus.ACCEPTED); verify(dnsMetrics).incrementPublishHostRequests("xn--q9jyb4c", 1, PublishStatus.ACCEPTED);
verify(dnsMetrics).incrementPublishHostRequests(0, PublishStatus.REJECTED); verify(dnsMetrics).incrementPublishHostRequests("xn--q9jyb4c", 0, PublishStatus.REJECTED);
verify(dnsMetrics).recordCommit("correctWriter", CommitStatus.SUCCESS, Duration.ZERO, 0, 1); verify(dnsMetrics)
.recordCommit("xn--q9jyb4c", "correctWriter", CommitStatus.SUCCESS, Duration.ZERO, 0, 1);
verify(dnsMetrics) verify(dnsMetrics)
.recordActionResult( .recordActionResult(
"xn--q9jyb4c",
"correctWriter", "correctWriter",
ActionStatus.SUCCESS, ActionStatus.SUCCESS,
1, 1,
@ -141,13 +143,15 @@ public class PublishDnsUpdatesActionTest {
verify(dnsWriter).publishDomain("example.xn--q9jyb4c"); verify(dnsWriter).publishDomain("example.xn--q9jyb4c");
verify(dnsWriter).commit(); verify(dnsWriter).commit();
verifyNoMoreInteractions(dnsWriter); verifyNoMoreInteractions(dnsWriter);
verify(dnsMetrics).incrementPublishDomainRequests(1, PublishStatus.ACCEPTED); verify(dnsMetrics).incrementPublishDomainRequests("xn--q9jyb4c", 1, PublishStatus.ACCEPTED);
verify(dnsMetrics).incrementPublishDomainRequests(0, PublishStatus.REJECTED); verify(dnsMetrics).incrementPublishDomainRequests("xn--q9jyb4c", 0, PublishStatus.REJECTED);
verify(dnsMetrics).incrementPublishHostRequests(0, PublishStatus.ACCEPTED); verify(dnsMetrics).incrementPublishHostRequests("xn--q9jyb4c", 0, PublishStatus.ACCEPTED);
verify(dnsMetrics).incrementPublishHostRequests(0, PublishStatus.REJECTED); verify(dnsMetrics).incrementPublishHostRequests("xn--q9jyb4c", 0, PublishStatus.REJECTED);
verify(dnsMetrics).recordCommit("correctWriter", CommitStatus.SUCCESS, Duration.ZERO, 1, 0); verify(dnsMetrics)
.recordCommit("xn--q9jyb4c", "correctWriter", CommitStatus.SUCCESS, Duration.ZERO, 1, 0);
verify(dnsMetrics) verify(dnsMetrics)
.recordActionResult( .recordActionResult(
"xn--q9jyb4c",
"correctWriter", "correctWriter",
ActionStatus.SUCCESS, ActionStatus.SUCCESS,
1, 1,
@ -186,13 +190,15 @@ public class PublishDnsUpdatesActionTest {
assertThrows(RuntimeException.class, action::run); assertThrows(RuntimeException.class, action::run);
verify(dnsMetrics).incrementPublishDomainRequests(2, PublishStatus.ACCEPTED); verify(dnsMetrics).incrementPublishDomainRequests("xn--q9jyb4c", 2, PublishStatus.ACCEPTED);
verify(dnsMetrics).incrementPublishDomainRequests(0, PublishStatus.REJECTED); verify(dnsMetrics).incrementPublishDomainRequests("xn--q9jyb4c", 0, PublishStatus.REJECTED);
verify(dnsMetrics).incrementPublishHostRequests(3, PublishStatus.ACCEPTED); verify(dnsMetrics).incrementPublishHostRequests("xn--q9jyb4c", 3, PublishStatus.ACCEPTED);
verify(dnsMetrics).incrementPublishHostRequests(0, PublishStatus.REJECTED); verify(dnsMetrics).incrementPublishHostRequests("xn--q9jyb4c", 0, PublishStatus.REJECTED);
verify(dnsMetrics).recordCommit("correctWriter", CommitStatus.FAILURE, Duration.ZERO, 2, 3); verify(dnsMetrics)
.recordCommit("xn--q9jyb4c", "correctWriter", CommitStatus.FAILURE, Duration.ZERO, 2, 3);
verify(dnsMetrics) verify(dnsMetrics)
.recordActionResult( .recordActionResult(
"xn--q9jyb4c",
"correctWriter", "correctWriter",
ActionStatus.COMMIT_FAILURE, ActionStatus.COMMIT_FAILURE,
5, 5,
@ -218,13 +224,15 @@ public class PublishDnsUpdatesActionTest {
verify(dnsWriter).publishHost("ns1.example2.xn--q9jyb4c"); verify(dnsWriter).publishHost("ns1.example2.xn--q9jyb4c");
verify(dnsWriter).commit(); verify(dnsWriter).commit();
verifyNoMoreInteractions(dnsWriter); verifyNoMoreInteractions(dnsWriter);
verify(dnsMetrics).incrementPublishDomainRequests(2, PublishStatus.ACCEPTED); verify(dnsMetrics).incrementPublishDomainRequests("xn--q9jyb4c", 2, PublishStatus.ACCEPTED);
verify(dnsMetrics).incrementPublishDomainRequests(0, PublishStatus.REJECTED); verify(dnsMetrics).incrementPublishDomainRequests("xn--q9jyb4c", 0, PublishStatus.REJECTED);
verify(dnsMetrics).incrementPublishHostRequests(3, PublishStatus.ACCEPTED); verify(dnsMetrics).incrementPublishHostRequests("xn--q9jyb4c", 3, PublishStatus.ACCEPTED);
verify(dnsMetrics).incrementPublishHostRequests(0, PublishStatus.REJECTED); verify(dnsMetrics).incrementPublishHostRequests("xn--q9jyb4c", 0, PublishStatus.REJECTED);
verify(dnsMetrics).recordCommit("correctWriter", CommitStatus.SUCCESS, Duration.ZERO, 2, 3); verify(dnsMetrics)
.recordCommit("xn--q9jyb4c", "correctWriter", CommitStatus.SUCCESS, Duration.ZERO, 2, 3);
verify(dnsMetrics) verify(dnsMetrics)
.recordActionResult( .recordActionResult(
"xn--q9jyb4c",
"correctWriter", "correctWriter",
ActionStatus.SUCCESS, ActionStatus.SUCCESS,
5, 5,
@ -244,13 +252,15 @@ public class PublishDnsUpdatesActionTest {
verify(dnsWriter).commit(); verify(dnsWriter).commit();
verifyNoMoreInteractions(dnsWriter); verifyNoMoreInteractions(dnsWriter);
verify(dnsMetrics).incrementPublishDomainRequests(0, PublishStatus.ACCEPTED); verify(dnsMetrics).incrementPublishDomainRequests("xn--q9jyb4c", 0, PublishStatus.ACCEPTED);
verify(dnsMetrics).incrementPublishDomainRequests(2, PublishStatus.REJECTED); verify(dnsMetrics).incrementPublishDomainRequests("xn--q9jyb4c", 2, PublishStatus.REJECTED);
verify(dnsMetrics).incrementPublishHostRequests(0, PublishStatus.ACCEPTED); verify(dnsMetrics).incrementPublishHostRequests("xn--q9jyb4c", 0, PublishStatus.ACCEPTED);
verify(dnsMetrics).incrementPublishHostRequests(3, PublishStatus.REJECTED); verify(dnsMetrics).incrementPublishHostRequests("xn--q9jyb4c", 3, PublishStatus.REJECTED);
verify(dnsMetrics).recordCommit("correctWriter", CommitStatus.SUCCESS, Duration.ZERO, 0, 0); verify(dnsMetrics)
.recordCommit("xn--q9jyb4c", "correctWriter", CommitStatus.SUCCESS, Duration.ZERO, 0, 0);
verify(dnsMetrics) verify(dnsMetrics)
.recordActionResult( .recordActionResult(
"xn--q9jyb4c",
"correctWriter", "correctWriter",
ActionStatus.SUCCESS, ActionStatus.SUCCESS,
5, 5,
@ -274,6 +284,7 @@ public class PublishDnsUpdatesActionTest {
verifyNoMoreInteractions(dnsWriter); verifyNoMoreInteractions(dnsWriter);
verify(dnsMetrics) verify(dnsMetrics)
.recordActionResult( .recordActionResult(
"xn--q9jyb4c",
"correctWriter", "correctWriter",
ActionStatus.LOCK_FAILURE, ActionStatus.LOCK_FAILURE,
5, 5,
@ -297,6 +308,7 @@ public class PublishDnsUpdatesActionTest {
verifyNoMoreInteractions(dnsWriter); verifyNoMoreInteractions(dnsWriter);
verify(dnsMetrics) verify(dnsMetrics)
.recordActionResult( .recordActionResult(
"xn--q9jyb4c",
"correctWriter", "correctWriter",
ActionStatus.BAD_LOCK_INDEX, ActionStatus.BAD_LOCK_INDEX,
2, 2,
@ -322,6 +334,7 @@ public class PublishDnsUpdatesActionTest {
verifyNoMoreInteractions(dnsWriter); verifyNoMoreInteractions(dnsWriter);
verify(dnsMetrics) verify(dnsMetrics)
.recordActionResult( .recordActionResult(
"xn--q9jyb4c",
"correctWriter", "correctWriter",
ActionStatus.BAD_LOCK_INDEX, ActionStatus.BAD_LOCK_INDEX,
2, 2,
@ -345,6 +358,7 @@ public class PublishDnsUpdatesActionTest {
verifyNoMoreInteractions(dnsWriter); verifyNoMoreInteractions(dnsWriter);
verify(dnsMetrics) verify(dnsMetrics)
.recordActionResult( .recordActionResult(
"xn--q9jyb4c",
"wrongWriter", "wrongWriter",
ActionStatus.BAD_WRITER, ActionStatus.BAD_WRITER,
5, 5,