Addressing comments.

test/integration/otel_test.go

@@ -188,6 +188,14 @@ func httpSpan(endpoint string, children ...expectedSpans) expectedSpans {
 	}
 }
 
+func redisPipelineSpan(op, service string, children ...expectedSpans) expectedSpans {
+	return expectedSpans{
+		Operation: "redis.pipeline " + op,
+		Service:   service,
+		Children:  children,
+	}
+}
+
 // TestTraces tests that all the expected spans are present and properly connected
 func TestTraces(t *testing.T) {
 	t.Parallel()
@@ -210,17 +218,19 @@ func TestTraces(t *testing.T) {
 			{Operation: "/acme/new-nonce", Service: wfe, Children: []expectedSpans{
 				rpcSpan("nonce.NonceService/Nonce", wfe, "nonce-service")}},
 			httpSpan("/acme/new-acct",
+				redisPipelineSpan("get", wfe),
+				redisPipelineSpan("set", wfe),
 				rpcSpan("sa.StorageAuthorityReadOnly/KeyBlocked", wfe, sa),
 				rpcSpan("sa.StorageAuthorityReadOnly/GetRegistrationByKey", wfe, sa),
 				rpcSpan("ra.RegistrationAuthority/NewRegistration", wfe, ra,
 					rpcSpan("sa.StorageAuthority/KeyBlocked", ra, sa),
-					// 1 ra -> sa rate limit span omitted here
 					rpcSpan("sa.StorageAuthority/NewRegistration", ra, sa))),
 			httpSpan("/acme/new-order",
 				rpcSpan("sa.StorageAuthorityReadOnly/GetRegistration", wfe, sa),
+				redisPipelineSpan("get", wfe),
+				redisPipelineSpan("set", wfe),
 				rpcSpan("ra.RegistrationAuthority/NewOrder", wfe, ra,
 					rpcSpan("sa.StorageAuthority/GetOrderForNames", ra, sa),
-					// 8 ra -> sa rate limit spans omitted here
 					rpcSpan("sa.StorageAuthority/NewOrderAndAuthzs", ra, sa))),
 			httpSpan("/acme/authz-v3/",
 				rpcSpan("sa.StorageAuthorityReadOnly/GetAuthorization2", wfe, sa)),
@@ -236,8 +246,10 @@ func TestTraces(t *testing.T) {
 					rpcSpan("sa.StorageAuthority/GetValidOrderAuthorizations2", ra, sa),
 					rpcSpan("sa.StorageAuthority/SetOrderProcessing", ra, sa),
 					rpcSpan("ca.CertificateAuthority/IssuePrecertificate", ra, ca),
+					redisPipelineSpan("get", ra),
 					rpcSpan("Publisher/SubmitToSingleCTWithResult", ra, "boulder-publisher"),
 					rpcSpan("ca.CertificateAuthority/IssueCertificateForPrecertificate", ra, ca),
+					redisPipelineSpan("set", ra),
 					rpcSpan("sa.StorageAuthority/FinalizeOrder", ra, sa))),
 			httpSpan("/acme/order/", rpcSpan("sa.StorageAuthorityReadOnly/GetOrder", wfe, sa)),
 			httpSpan("/acme/cert/", rpcSpan("sa.StorageAuthorityReadOnly/GetCertificate", wfe, sa)),

test/integration/ratelimit_test.go

@@ -46,7 +46,8 @@ func TestCertificatesPerDomain(t *testing.T) {
 		return fmt.Sprintf("%s.%s", hex.EncodeToString(bytes[:]), randomDomain)
 	}
 
-	_, err := authAndIssue(nil, nil, []string{randomSubDomain()}, true)
+	firstSubDomain := randomSubDomain()
+	_, err := authAndIssue(nil, nil, []string{firstSubDomain}, true)
 	test.AssertNotError(t, err, "Failed to issue first certificate")
 
 	_, err = authAndIssue(nil, nil, []string{randomSubDomain()}, true)
@@ -62,38 +63,9 @@ func TestCertificatesPerDomain(t *testing.T) {
 		// Error should be served from legacy rate limits implementation.
 		test.AssertContains(t, err.Error(), fmt.Sprintf("too many certificates already issued for %q", randomDomain))
 	}
-}
-
-func TestRenewalExemption(t *testing.T) {
-	t.Parallel()
-
-	// Issue two certificates for different subdomains under a single domain,
-	// then renew both. With the certificatesPerName limit at 2 per 90 days, and
-	// renewals not exempt, both issuances should succeed. Finally, issue a
-	// certificate for a third subdomain, which should fail due to the limit.
-
-	baseDomain := random_domain()
-
-	_, err := authAndIssue(nil, nil, []string{"www." + baseDomain}, true)
-	test.AssertNotError(t, err, "Failed to issue first certificate")
-
-	_, err = authAndIssue(nil, nil, []string{"www." + baseDomain}, true)
-	test.AssertNotError(t, err, "Failed to issue first renewal")
-
-	_, err = authAndIssue(nil, nil, []string{"blog." + baseDomain}, true)
-	test.AssertNotError(t, err, "Failed to issue second certificate")
-
-	_, err = authAndIssue(nil, nil, []string{"blog." + baseDomain}, true)
-	test.AssertNotError(t, err, "Failed to issue second renewal")
 
-	_, err = authAndIssue(nil, nil, []string{"mail." + baseDomain}, true)
-	test.AssertError(t, err, "Somehow managed to issue third certificate")
-
-	if strings.Contains(os.Getenv("BOULDER_CONFIG_DIR"), "test/config-next") {
-		// Error should be served from key-value rate limits implementation.
-		test.AssertContains(t, err.Error(), fmt.Sprintf("too many certificates (2) already issued for %q in the last 2160h0m0s", baseDomain))
-	} else {
-		// Error should be served from legacy rate limits implementation.
-		test.AssertContains(t, err.Error(), fmt.Sprintf("too many certificates already issued for %q", baseDomain))
-	}
+	// Issue a certificate for the first subdomain, which should succeed because
+	// it's a renewal.
+	_, err = authAndIssue(nil, nil, []string{firstSubDomain}, true)
+	test.AssertNotError(t, err, "Failed to issue renewal certificate")
 }