Review comments

Author: aarongable

cmd/admin/dryrun.go

@@ -79,7 +79,7 @@ func (d dryRunSAC) EnableRateLimitOverride(ctx context.Context, req *sapb.Enable
 }
 
 func (d dryRunSAC) PauseIdentifiers(ctx context.Context, req *sapb.PauseRequest, _ ...grpc.CallOption) (*sapb.PauseIdentifiersResponse, error) {
-	d.log.Info(ctx, "dry-run: sa.EnableRateLimitOverride",
+	d.log.Info(ctx, "dry-run: sa.PauseIdentifiers",
 		blog.Acct(req.RegistrationID),
 		blog.Idents(identifier.FromProtoSlice(req.Identifiers)...),
 	)

cmd/admin/overrides_import.go

@@ -75,7 +75,7 @@ func (c *subcommandImportOverrides) Run(ctx context.Context, a *admin) error {
 	for range overrideCount {
 		result := <-results
 		if result.err != nil {
-			a.log.Error(ctx, "failed to add override", err,
+			a.log.Error(ctx, "failed to add override", result.err,
 				slog.Int64("limit", result.ov.LimitEnum),
 				slog.String("bucketKey", result.ov.BucketKey),
 			)

cmd/shell.go

@@ -228,7 +228,7 @@ func NewLogger(logConf blog.Config) blog.Logger {
 	go func() {
 		for {
 			time.Sleep(time.Hour)
-			logger.Info(context.Background(), "heartbeat", slog.Time("time", time.Now()))
+			logger.Info(context.Background(), "heartbeat", slog.Time("now", time.Now()))
 		}
 	}()
 	return logger

ra/ra.go

@@ -1594,8 +1594,9 @@ func (ra *RegistrationAuthorityImpl) RevokeCertByApplicant(ctx context.Context,
 	defer func() {
 		if err != nil {
 			ra.log.AuditError(ctx, "Revocation request", err)
+		} else {
+			ra.log.AuditInfo(ctx, "Revocation request")
 		}
-		ra.log.AuditInfo(ctx, "Revocation request")
 	}()
 
 	metadata, err := ra.SA.GetSerialMetadata(ctx, &sapb.Serial{Serial: serialString})
@@ -1736,8 +1737,9 @@ func (ra *RegistrationAuthorityImpl) RevokeCertByKey(ctx context.Context, req *r
 	defer func() {
 		if err != nil {
 			ra.log.AuditError(ctx, "Revocation request", err)
+		} else {
+			ra.log.AuditInfo(ctx, "Revocation request")
 		}
-		ra.log.AuditInfo(ctx, "Revocation request")
 	}()
 
 	// We revoke the cert before adding it to the blocked keys list, to avoid a
@@ -1824,8 +1826,9 @@ func (ra *RegistrationAuthorityImpl) AdministrativelyRevokeCertificate(ctx conte
 	defer func() {
 		if err != nil {
 			ra.log.AuditError(ctx, "Revocation request", err)
+		} else {
+			ra.log.AuditInfo(ctx, "Revocation request")
 		}
-		ra.log.AuditInfo(ctx, "Revocation request")
 	}()
 
 	var cert *x509.Certificate

va/caa.go

@@ -101,7 +101,6 @@ func (va *ValidationAuthorityImpl) DoCAA(ctx context.Context, req *vapb.IsCAAVal
 		if va.isPrimaryVA() {
 			// Observe total check latency (primary+remote).
 			va.observeLatency(opCAA, allPerspectives, string(challType), probType, outcome, va.clk.Since(start))
-			logAttrs = append(logAttrs, slog.String("status", string(core.StatusValid)))
 		}
 
 		va.log.AuditInfo(ctx, "CAA check result", logAttrs...)

va/va.go

@@ -610,6 +610,8 @@ func summarizeMPIC(passed, failed []string, passedRIRSet map[string]struct{}) *m
 // Internal logic errors are logged. If the number of operation failures exceeds
 // va.maxRemoteFailures, the first encountered problem is returned as a
 // *probs.ProblemDetails.
+//
+// It always returns a non-nil summary, whether or not there's also a prob.
 func (va *ValidationAuthorityImpl) doRemoteOperation(ctx context.Context, op remoteOperation, req proto.Message) (*mpicSummary, *probs.ProblemDetails) {
 	remoteVACount := len(va.remoteVAs)
 	//  - Mar 15, 2026: MUST implement using at least 3 perspectives
@@ -618,7 +620,7 @@ func (va *ValidationAuthorityImpl) doRemoteOperation(ctx context.Context, op rem
 	// See "Phased Implementation Timeline" in
 	// https://github.com/cabforum/servercert/blob/main/docs/BR.md#3229-multi-perspective-issuance-corroboration
 	if remoteVACount < 3 {
-		return nil, probs.ServerInternal("Insufficient remote perspectives: need at least 3")
+		return summarizeMPIC(nil, nil, nil), probs.ServerInternal("Insufficient remote perspectives: need at least 3")
 	}
 
 	type response struct {
@@ -883,7 +885,12 @@ func (va *ValidationAuthorityImpl) DoDCV(ctx context.Context, req *vapb.PerformV
 		}
 		var summary *mpicSummary
 		summary, prob = va.doRemoteOperation(ctx, op, req)
-		logAttrs = append(logAttrs, slog.Any("mpicSummary", summary))
+		logAttrs = append(logAttrs, slog.Group("mpic",
+			slog.Any("passed", summary.Passed),
+			slog.Any("failed", summary.Failed),
+			slog.Any("passedRIRs", summary.PassedRIRs),
+			slog.String("quorum", summary.QuorumResult),
+		))
 	}
 
 	return bgrpc.ValidationResultToPB(records, filterProblemDetails(prob), va.perspective, va.rir)