Fix duplicate label set error in certMetaCollector for failed issuances

externalcas/metrics.go

@@ -90,16 +90,21 @@ func (c *certMetaCollector) Describe(ch chan<- *prometheus.Desc) {
 //	revocationDuration:  serial, common_name, status
 func (c *certMetaCollector) Collect(ch chan<- prometheus.Metric) {
 	for _, r := range c.store.allIssued() {
+		// Skip failure records: serial and issuer are empty, so all failures for
+		// the same CN/SAN collapse to identical label sets causing duplicate metric
+		// errors at scrape time. Failures are already counted by
+		// externalcas_certificates_issued_total{status="failure"}.
+		if r.Status != "success" {
+			continue
+		}
 		ch <- prometheus.MustNewConstMetric(c.issuedInfo, prometheus.GaugeValue, 1,
 			r.Serial, r.CommonName, r.Issuer, r.SANs, r.Status)
 		ch <- prometheus.MustNewConstMetric(c.requestDuration, prometheus.GaugeValue, r.DurationSeconds,
 			r.Serial, r.CommonName, r.Status)
-		if r.Status == "success" {
-			ch <- prometheus.MustNewConstMetric(c.issuedAt, prometheus.GaugeValue, float64(r.IssuedAt.Unix()),
-				r.Serial, r.CommonName)
-			ch <- prometheus.MustNewConstMetric(c.expiresAt, prometheus.GaugeValue, float64(r.ExpiresAt.Unix()),
-				r.Serial, r.CommonName)
-		}
+		ch <- prometheus.MustNewConstMetric(c.issuedAt, prometheus.GaugeValue, float64(r.IssuedAt.Unix()),
+			r.Serial, r.CommonName)
+		ch <- prometheus.MustNewConstMetric(c.expiresAt, prometheus.GaugeValue, float64(r.ExpiresAt.Unix()),
+			r.Serial, r.CommonName)
 	}
 	for _, r := range c.store.allRevoked() {
 		ch <- prometheus.MustNewConstMetric(c.revokedInfo, prometheus.GaugeValue, 1,

externalcas/metrics_test.go

@@ -50,34 +50,55 @@ func TestCertMetaCollector_Collect_SuccessRecord(t *testing.T) {
 
 func TestCertMetaCollector_Collect_FailureRecord(t *testing.T) {
 	s := newTestStore(t)
-	if err := s.recordIssued(CertRecord{
-		CommonName: "example.com", SANs: "example.com",
-		DurationSeconds: 0.3, Status: "failure",
-	}); err != nil {
-		t.Fatal(err)
+	// Store multiple failures for the same CN/SAN — the pre-fix behaviour would
+	// emit duplicate label sets and cause a Prometheus scrape error.
+	for i := 0; i < 3; i++ {
+		if err := s.recordIssued(CertRecord{
+			CommonName: "example.com", SANs: "example.com",
+			DurationSeconds: 0.3, Status: "failure",
+		}); err != nil {
+			t.Fatal(err)
+		}
 	}
 
 	c := newCertMetaCollector(s)
-	// Failures emit info and duration but NOT timestamp metrics — IssuedAt/ExpiresAt are
-	// zero values that would appear as 1970-01-01 in dashboards.
+	// Failure records must not be emitted by the collector — serial and issuer are
+	// empty for all failures, so multiple failures for the same CN/SAN produce
+	// identical label sets which Prometheus rejects with a duplicate metric error.
+	// Failures are counted by externalcas_certificates_issued_total{status="failure"}.
 	for _, name := range []string{
 		"externalcas_certificate_info",
 		"externalcas_certificate_signing_duration_seconds",
-	} {
-		if n := testutil.CollectAndCount(c, name); n != 1 {
-			t.Errorf("CollectAndCount(%q) = %d, want 1", name, n)
-		}
-	}
-	for _, name := range []string{
 		"externalcas_certificate_issued_timestamp_seconds",
 		"externalcas_certificate_expiry_timestamp_seconds",
 	} {
 		if n := testutil.CollectAndCount(c, name); n != 0 {
-			t.Errorf("CollectAndCount(%q) = %d, want 0 (timestamps must not be emitted for failures)", name, n)
+			t.Errorf("CollectAndCount(%q) = %d, want 0 (failure records must not be emitted)", name, n)
 		}
 	}
 }
 
+func TestCertMetaCollector_Collect_MultipleFailures_NoDuplicates(t *testing.T) {
+	// Regression test for: multiple failures for the same CN/SAN causing duplicate
+	// label set errors at scrape time when serial and issuer are both empty.
+	s := newTestStore(t)
+	for i := 0; i < 5; i++ {
+		if err := s.recordIssued(CertRecord{
+			CommonName: "example.com", SANs: "example.com",
+			DurationSeconds: float64(i), Status: "failure",
+		}); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	reg := prometheus.NewRegistry()
+	reg.MustRegister(newCertMetaCollector(s))
+	// Gather must succeed without duplicate metric errors.
+	if _, err := reg.Gather(); err != nil {
+		t.Errorf("Gather() returned error with multiple failure records: %v", err)
+	}
+}
+
 func TestCertMetaCollector_Collect_RevokedRecord(t *testing.T) {
 	s := newTestStore(t)
 	now := time.Now().Truncate(time.Second)