va: add challenge type to remote VA differentials. (letsencrypt#4410)

Daniel McCarney · jsha · commit fe23dabd69ce · 2019-08-29T14:41:14.000-07:00
This will make data analysis of the differentials easier. Along the way
I also added a unit test for `logRemoteValidationDifferentials`.
diff --git a/va/va.go b/va/va.go
@@ -476,7 +476,7 @@ 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, primaryResult, remoteProbs)
+	va.logRemoteValidationDifferentials(domain, challengeType, primaryResult, remoteProbs)
 
 	// Based on the threshold of good/bad return nil or a problem.
 	if good >= required {
@@ -496,6 +496,7 @@ func (va *ValidationAuthorityImpl) processRemoteResults(
 // that contains the primary VA result and the results each remote VA returned.
 func (va *ValidationAuthorityImpl) logRemoteValidationDifferentials(
 	domain string,
+	challengeType string,
 	primaryResult *probs.ProblemDetails,
 	remoteProbs []*probs.ProblemDetails) {
 
@@ -528,11 +529,13 @@ func (va *ValidationAuthorityImpl) logRemoteValidationDifferentials(
 
 	logOb := struct {
 		Domain          string
+		ChallengeType   string
 		PrimaryResult   *probs.ProblemDetails
 		RemoteSuccesses int
 		RemoteFailures  []*probs.ProblemDetails
 	}{
 		Domain:          domain,
+		ChallengeType:   challengeType,
 		PrimaryResult:   primaryResult,
 		RemoteSuccesses: len(successes),
 		RemoteFailures:  failures,
diff --git a/va/va_test.go b/va/va_test.go
@@ -611,3 +611,68 @@ func TestDetailedError(t *testing.T) {
 		}
 	}
 }
+
+func TestLogRemoteValidationDifferentials(t *testing.T) {
+	// Create some remote VAs
+	remoteVA1, _ := setup(nil, 0, "remote 1", nil)
+	remoteVA2, _ := setup(nil, 0, "remote 2", nil)
+	remoteVA3, _ := setup(nil, 0, "remote 3", nil)
+	remoteVAs := []RemoteVA{
+		{remoteVA1, "remote 1"},
+		{remoteVA2, "remote 2"},
+		{remoteVA3, "remote 3"},
+	}
+
+	// Set up a local VA that allows a max of 2 remote failures.
+	localVA, mockLog := setup(nil, 2, "local 1", remoteVAs)
+
+	egProbA := probs.DNS("root DNS servers closed at 4:30pm")
+	egProbB := probs.OrderNotReady("please take a number")
+
+	testCases := []struct {
+		name          string
+		primaryResult *probs.ProblemDetails
+		remoteProbs   []*probs.ProblemDetails
+		expectedLog   string
+	}{
+		{
+			name:          "remote and primary results equal (all nil)",
+			primaryResult: nil,
+			remoteProbs:   nil,
+		},
+		{
+			name:          "remote and primary results equal (not nil)",
+			primaryResult: egProbA,
+			remoteProbs:   []*probs.ProblemDetails{egProbA, egProbA, egProbA},
+		},
+		{
+			name:          "remote and primary differ (primary nil)",
+			primaryResult: nil,
+			remoteProbs:   []*probs.ProblemDetails{egProbA, nil, egProbB},
+			expectedLog:   `INFO: remoteVADifferentials JSON={"Domain":"example.com","ChallengeType":"blorpus-01","PrimaryResult":null,"RemoteSuccesses":1,"RemoteFailures":[{"type":"dns","detail":"root DNS servers closed at 4:30pm","status":400},{"type":"orderNotReady","detail":"please take a number","status":403}]}`,
+		},
+		{
+			name:          "remote and primary differ (primary not nil)",
+			primaryResult: egProbA,
+			remoteProbs:   []*probs.ProblemDetails{nil, egProbB, nil},
+			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":[{"type":"orderNotReady","detail":"please take a number","status":403}]}`,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			mockLog.Clear()
+
+			localVA.logRemoteValidationDifferentials(
+				"example.com", "blorpus-01", tc.primaryResult, tc.remoteProbs)
+
+			lines := mockLog.GetAllMatching("remoteVADifferentials JSON=.*")
+			if tc.expectedLog != "" {
+				test.AssertEquals(t, len(lines), 1)
+				test.AssertEquals(t, lines[0], tc.expectedLog)
+			} else {
+				test.AssertEquals(t, len(lines), 0)
+			}
+		})
+	}
+}
