Only register grpc TLS metrics on successful handshake (#1338)

In the current gRPC implementation, TLS metrics are registered before the handshake is complete. In cases where the probe fails, probeSSLEarliestCertExpiryGauge is set to the default time.Time value (far in the past), resulting in any alerts using probe_ssl_earliest_cert_expiry metric firing. In the case of the TLS handshake failing, it makes more sense for TLS metrics not to be set. This is the way the http probe is currently configured. This should also solve #1120. * only register grpc TLS metrics on success * adds tests --------- Signed-off-by: mattb18 <[email protected]>
prometheus · Dec 31, 2024 · 9cc59c3 · 9cc59c3
1 parent 9f9d483
commit 9cc59c3
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 3 deletions.
diff --git a/prober/grpc.go b/prober/grpc.go
@@ -121,9 +121,6 @@ func ProbeGRPC(ctx context.Context, target string, module config.Module, registr
 	registry.MustRegister(isSSLGauge)
 	registry.MustRegister(statusCodeGauge)
 	registry.MustRegister(healthCheckResponseGaugeVec)
-	registry.MustRegister(probeSSLEarliestCertExpiryGauge)
-	registry.MustRegister(probeTLSVersion)
-	registry.MustRegister(probeSSLLastInformation)
 
 	if !strings.HasPrefix(target, "http://") && !strings.HasPrefix(target, "https://") {
 		target = "http://" + target
@@ -203,6 +200,7 @@ func ProbeGRPC(ctx context.Context, target string, module config.Module, registr
 	if serverPeer != nil {
 		tlsInfo, tlsOk := serverPeer.AuthInfo.(credentials.TLSInfo)
 		if tlsOk {
+			registry.MustRegister(probeSSLEarliestCertExpiryGauge, probeTLSVersion, probeSSLLastInformation)
 			isSSLGauge.Set(float64(1))
 			probeSSLEarliestCertExpiryGauge.Set(float64(getEarliestCertExpiry(&tlsInfo.State).Unix()))
 			probeTLSVersion.WithLabelValues(getTLSVersion(&tlsInfo.State)).Set(1)

diff --git a/prober/grpc_test.go b/prober/grpc_test.go
@@ -432,3 +432,34 @@ func TestGRPCHealthCheckUnimplemented(t *testing.T) {
 
 	checkRegistryResults(expectedResults, mfs, t)
 }
+
+func TestGRPCAbsentFailedTLS(t *testing.T) {
+	testCTX, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+	registry := prometheus.NewRegistry()
+
+	// probe and invalid port to trigger TCP/TLS error
+	result := ProbeGRPC(testCTX, "localhost:0",
+		config.Module{Timeout: time.Second, GRPC: config.GRPCProbe{
+			IPProtocolFallback: false,
+			Service:            "NonExistingService",
+		},
+		}, registry, promslog.NewNopLogger())
+
+	if result {
+		t.Fatalf("GRPC probe succeeded, should have failed")
+	}
+
+	mfs, err := registry.Gather()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	absentMetrics := []string{
+		"probe_ssl_earliest_cert_expiry",
+		"probe_tls_version_info",
+		"probe_ssl_last_chain_info",
+	}
+
+	checkAbsentMetrics(absentMetrics, mfs, t)
+}
diff --git a/prober/utils_test.go b/prober/utils_test.go
@@ -23,6 +23,7 @@ import (
 	"fmt"
 	"math/big"
 	"net"
+	"slices"
 	"testing"
 	"time"
 
@@ -249,3 +250,12 @@ func checkMetrics(expected map[string]map[string]map[string]struct{}, mfs []*dto
 		}
 	}
 }
+
+func checkAbsentMetrics(absent []string, mfs []*dto.MetricFamily, t *testing.T) {
+	for _, v := range mfs {
+		name := v.GetName()
+		if slices.Contains(absent, name) {
+			t.Fatalf("metric %s was found but should be absent", name)
+		}
+	}
+}