From 014c15ba61843eef2a4f95ad7b8136d24f3f05f9 Mon Sep 17 00:00:00 2001 From: Aaron Gable Date: Mon, 3 Oct 2022 11:55:27 -0700 Subject: [PATCH] crl-storer: simplify S3 error handling (#6417) Currently, we refuse to error out when checking the error from S3, to ensure that we can update the metrics appropriately. This requires us to use an unconventional error-checking structure, and to check the error again when it comes time to return. Instead, move the metrics above the error check, and then make the error check a more traditional structure. --- crl/storer/storer.go | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/crl/storer/storer.go b/crl/storer/storer.go index dfc497daa..055c0f028 100644 --- a/crl/storer/storer.go +++ b/crl/storer/storer.go @@ -166,23 +166,21 @@ func (cs *crlStorer) UploadCRL(stream cspb.CRLStorer_UploadCRLServer) error { ContentType: &crlContentType, Metadata: map[string]string{"crlNumber": crlNumber.String()}, }) - if err != nil { - cs.uploadCount.WithLabelValues(issuer.Subject.CommonName, "failed").Inc() - cs.log.AuditErrf("CRL upload failed: id=[%s] err=[%s]", crlId, err) - } else { - cs.uploadCount.WithLabelValues(issuer.Subject.CommonName, "success").Inc() - cs.log.AuditInfof( - "CRL uploaded: id=[%s] issuerCN=[%s] thisUpdate=[%s] nextUpdate=[%s] numEntries=[%d]", - crlId, issuer.Subject.CommonName, crl.ThisUpdate, crl.NextUpdate, len(crl.RevokedCertificates), - ) - } latency := cs.clk.Now().Sub(start) cs.latencyHistogram.WithLabelValues(issuer.Subject.CommonName).Observe(latency.Seconds()) if err != nil { + cs.uploadCount.WithLabelValues(issuer.Subject.CommonName, "failed").Inc() + cs.log.AuditErrf("CRL upload failed: id=[%s] err=[%s]", crlId, err) return fmt.Errorf("uploading to S3: %w", err) } + cs.uploadCount.WithLabelValues(issuer.Subject.CommonName, "success").Inc() + cs.log.AuditInfof( + "CRL uploaded: id=[%s] issuerCN=[%s] thisUpdate=[%s] nextUpdate=[%s] numEntries=[%d]", + crlId, issuer.Subject.CommonName, crl.ThisUpdate, crl.NextUpdate, len(crl.RevokedCertificates), + ) + return stream.SendAndClose(&emptypb.Empty{}) }