From 7b60b57c33318834e880faa822b0415cda6809a5 Mon Sep 17 00:00:00 2001
From: Daniel McCarney <cpu@letsencrypt.org>
Date: Thu, 31 Oct 2019 13:12:28 -0400
Subject: [PATCH] va: log account ID in multi VA differential JSON. (#4521)

This will reduce the amount of analysis time required to identify
large integrators that aren't compatible with multi VA.
---
 va/va.go      | 27 ++++++++++++++++++++++++---
 va/va_test.go |  6 +++---
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/va/va.go b/va/va.go
index c65af68ba..25f2d6161 100644
--- a/va/va.go
+++ b/va/va.go
@@ -443,6 +443,7 @@ func (va *ValidationAuthorityImpl) performRemoteValidation(
 // remote VAs but is more performant.
 func (va *ValidationAuthorityImpl) processRemoteResults(
 	domain string,
+	acctID int64,
 	challengeType string,
 	primaryResult *probs.ProblemDetails,
 	remoteResultsChan chan *remoteValidationResult,
@@ -503,7 +504,12 @@ func (va *ValidationAuthorityImpl) processRemoteResults(
 	// If we are using `features.MultiVAFullResults` then we haven't returned
 	// early and can now log the differential between what the primary VA saw and
 	// what all of the remote VAs saw.
-	va.logRemoteValidationDifferentials(domain, challengeType, primaryResult, remoteResults)
+	va.logRemoteValidationDifferentials(
+		domain,
+		acctID,
+		challengeType,
+		primaryResult,
+		remoteResults)
 
 	// Based on the threshold of good/bad return nil or a problem.
 	if good >= required {
@@ -523,6 +529,7 @@ func (va *ValidationAuthorityImpl) processRemoteResults(
 // that contains the primary VA result and the results each remote VA returned.
 func (va *ValidationAuthorityImpl) logRemoteValidationDifferentials(
 	domain string,
+	acctID int64,
 	challengeType string,
 	primaryResult *probs.ProblemDetails,
 	remoteResults []*remoteValidationResult) {
@@ -556,12 +563,14 @@ func (va *ValidationAuthorityImpl) logRemoteValidationDifferentials(
 
 	logOb := struct {
 		Domain          string
+		AccountID       int64
 		ChallengeType   string
 		PrimaryResult   *probs.ProblemDetails
 		RemoteSuccesses int
 		RemoteFailures  []*remoteValidationResult
 	}{
 		Domain:          domain,
+		AccountID:       acctID,
 		ChallengeType:   challengeType,
 		PrimaryResult:   primaryResult,
 		RemoteSuccesses: len(successes),
@@ -624,14 +633,26 @@ func (va *ValidationAuthorityImpl) PerformValidation(ctx context.Context, domain
 			// differentials then collect and log the remote results in a separate go
 			// routine to avoid blocking the primary VA.
 			go func() {
-				_ = va.processRemoteResults(domain, string(challenge.Type), prob, remoteResults, len(va.remoteVAs))
+				_ = va.processRemoteResults(
+					domain,
+					authz.RegistrationID,
+					string(challenge.Type),
+					prob,
+					remoteResults,
+					len(va.remoteVAs))
 			}()
 			// Since prob was nil and we're not enforcing the results from
 			// `processRemoteResults` set the challenge status to valid so the
 			// validationTime metrics increment has the correct result label.
 			challenge.Status = core.StatusValid
 		} else if features.Enabled(features.EnforceMultiVA) {
-			remoteProb := va.processRemoteResults(domain, string(challenge.Type), prob, remoteResults, len(va.remoteVAs))
+			remoteProb := va.processRemoteResults(
+				domain,
+				authz.RegistrationID,
+				string(challenge.Type),
+				prob,
+				remoteResults,
+				len(va.remoteVAs))
 
 			// We consider the multi VA result skippable even though we are enforcing
 			// multi VA if the domain or the account has multi-VA disabled by policy.
diff --git a/va/va_test.go b/va/va_test.go
index 132fa321e..db3383b76 100644
--- a/va/va_test.go
+++ b/va/va_test.go
@@ -739,7 +739,7 @@ func TestLogRemoteValidationDifferentials(t *testing.T) {
 				&remoteValidationResult{Problem: nil, VAHostname: "remoteB"},
 				&remoteValidationResult{Problem: egProbB, VAHostname: "remoteC"},
 			},
-			expectedLog: `INFO: remoteVADifferentials JSON={"Domain":"example.com","ChallengeType":"blorpus-01","PrimaryResult":null,"RemoteSuccesses":1,"RemoteFailures":[{"VAHostname":"remoteA","Problem":{"type":"dns","detail":"root DNS servers closed at 4:30pm","status":400}},{"VAHostname":"remoteC","Problem":{"type":"orderNotReady","detail":"please take a number","status":403}}]}`,
+			expectedLog: `INFO: remoteVADifferentials JSON={"Domain":"example.com","AccountID":1999,"ChallengeType":"blorpus-01","PrimaryResult":null,"RemoteSuccesses":1,"RemoteFailures":[{"VAHostname":"remoteA","Problem":{"type":"dns","detail":"root DNS servers closed at 4:30pm","status":400}},{"VAHostname":"remoteC","Problem":{"type":"orderNotReady","detail":"please take a number","status":403}}]}`,
 		},
 		{
 			name:          "remote and primary differ (primary not nil)",
@@ -749,7 +749,7 @@ func TestLogRemoteValidationDifferentials(t *testing.T) {
 				&remoteValidationResult{Problem: egProbB, VAHostname: "remoteB"},
 				&remoteValidationResult{Problem: nil, VAHostname: "remoteC"},
 			},
-			expectedLog: `INFO: remoteVADifferentials JSON={"Domain":"example.com","ChallengeType":"blorpus-01","PrimaryResult":{"type":"dns","detail":"root DNS servers closed at 4:30pm","status":400},"RemoteSuccesses":2,"RemoteFailures":[{"VAHostname":"remoteB","Problem":{"type":"orderNotReady","detail":"please take a number","status":403}}]}`,
+			expectedLog: `INFO: remoteVADifferentials JSON={"Domain":"example.com","AccountID":1999,"ChallengeType":"blorpus-01","PrimaryResult":{"type":"dns","detail":"root DNS servers closed at 4:30pm","status":400},"RemoteSuccesses":2,"RemoteFailures":[{"VAHostname":"remoteB","Problem":{"type":"orderNotReady","detail":"please take a number","status":403}}]}`,
 		},
 	}
 
@@ -758,7 +758,7 @@ func TestLogRemoteValidationDifferentials(t *testing.T) {
 			mockLog.Clear()
 
 			localVA.logRemoteValidationDifferentials(
-				"example.com", "blorpus-01", tc.primaryResult, tc.remoteProbs)
+				"example.com", 1999, "blorpus-01", tc.primaryResult, tc.remoteProbs)
 
 			lines := mockLog.GetAllMatching("remoteVADifferentials JSON=.*")
 			if tc.expectedLog != "" {