feat: compare legacy and unified search results via histogram (#109022)

* feat: compare legacy and unified search results via histogram Signed-off-by: Bruno Abrantes <bruno@brunoabrantes.com> * fix: handle cases where request type is not set Signed-off-by: Bruno Abrantes <bruno@brunoabrantes.com> * fix: use struct instead of bool because it's more memory efficient Signed-off-by: Bruno Abrantes <bruno@brunoabrantes.com> * fix: calculate recall percentage rather than union between legacy and unified Signed-off-by: Bruno Abrantes <bruno@brunoabrantes.com> --------- Signed-off-by: Bruno Abrantes <bruno@brunoabrantes.com>
2025-08-05 11:17:32 +02:00
parent 6408e3acaa
commit cb921dc47a
2 changed files with 225 additions and 3 deletions
@@ -2,8 +2,11 @@ package resource

 import (
 	"context"
+	"fmt"
 	"time"

+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/promauto"
 	"google.golang.org/grpc"
 	"k8s.io/apimachinery/pkg/runtime/schema"

@@ -17,6 +20,20 @@ const (
 	backgroundRequestTimeout = 500 * time.Millisecond
 )

+var (
+	// searchResultsMatchHistogram tracks the percentage match between legacy and unified search results
+	searchResultsMatchHistogram = promauto.NewHistogramVec(
+		prometheus.HistogramOpts{
+			Namespace: "grafana",
+			Subsystem: "unified_storage",
+			Name:      "search_results_match_percentage",
+			Help:      "Histogram of percentage match between legacy and unified search results",
+			Buckets:   []float64{0, 10, 20, 30, 40, 50, 60, 70, 80, 90, 100},
+		},
+		[]string{"resource_type"},
+	)
+)
+
 type DualWriter interface {
 	IsEnabled(schema.GroupResource) bool
 	ReadFromUnified(context.Context, schema.GroupResource) (bool, error)
@@ -44,6 +61,43 @@ type searchWrapper struct {
 	logger        log.Logger
 }

+// extractUIDs extracts unique UIDs from search response results
+func extractUIDs(response *resourcepb.ResourceSearchResponse) map[string]struct{} {
+	uids := make(map[string]struct{})
+	if response == nil || response.Results == nil || response.Results.Rows == nil {
+		return uids
+	}
+
+	for _, row := range response.Results.Rows {
+		if row.Key != nil && row.Key.Name != "" {
+			uids[row.Key.Name] = struct{}{}
+		}
+	}
+	return uids
+}
+
+// calculateMatchPercentage calculates recall: what percentage of legacy results were also found by unified search
+func calculateMatchPercentage(legacyUIDs, unifiedUIDs map[string]struct{}) float64 {
+	if len(legacyUIDs) == 0 && len(unifiedUIDs) == 0 {
+		return 100.0 // Both empty, consider as 100% match
+	}
+	if len(legacyUIDs) == 0 || len(unifiedUIDs) == 0 {
+		return 0.0 // One empty, other not
+	}
+
+	// Count matches: how many legacy results did unified also return?
+	matches := 0
+	for uid := range legacyUIDs {
+		if _, exists := unifiedUIDs[uid]; exists {
+			matches++
+		}
+	}
+
+	// Calculate recall: percentage of legacy results that unified also returned
+	// Legacy is the source of truth, so we use it as the denominator
+	return float64(matches) / float64(len(legacyUIDs)) * 100.0
+}
+
 func (s *searchWrapper) GetStats(ctx context.Context, in *resourcepb.ResourceStatsRequest,
 	opts ...grpc.CallOption) (*resourcepb.ResourceStatsResponse, error) {
 	client := s.legacyClient
@@ -89,23 +143,67 @@ func (s *searchWrapper) Search(ctx context.Context, in *resourcepb.ResourceSearc
 	}

 	// If dual reader feature flag is enabled, and legacy is the main storage,
-	// make a background call to unified
+	// make a background call to unified and compare results
 	if s.features != nil && s.features.IsEnabledGlobally(featuremgmt.FlagUnifiedStorageSearchDualReaderEnabled) && !unified {
+		// Get the legacy result first
+		legacyResponse, legacyErr := s.legacyClient.Search(ctx, in, opts...)
+		if legacyErr != nil {
+			return nil, legacyErr
+		}
+
 		// Create background context with timeout but ignore parent cancelation
 		ctxBg := context.WithoutCancel(ctx)
 		ctxBgWithTimeout, cancel := context.WithTimeout(ctxBg, backgroundRequestTimeout)

-		// Make background call without blocking the main request
+		// Make background call and compare results
 		go func() {
 			defer cancel() // Ensure we clean up the context
-			_, bgErr := s.unifiedClient.Search(ctxBgWithTimeout, in, opts...)
+			unifiedResponse, bgErr := s.unifiedClient.Search(ctxBgWithTimeout, in, opts...)
 			if bgErr != nil {
 				s.logger.Error("Background Search call to unified failed", "error", bgErr, "timeout", backgroundRequestTimeout)
 			} else {
 				s.logger.Debug("Background Search call to unified succeeded")
+
+				// Compare results when both are successful
+				var requestKey *resourcepb.ResourceKey
+				if in.Options != nil {
+					requestKey = in.Options.Key
+				}
+				s.compareSearchResults(legacyResponse, unifiedResponse, requestKey)
 			}
 		}()
+
+		return legacyResponse, nil
 	}

 	return client.Search(ctx, in, opts...)
 }
+
+// compareSearchResults compares legacy and unified search results and logs/metrics the outcome
+func (s *searchWrapper) compareSearchResults(legacyResponse, unifiedResponse *resourcepb.ResourceSearchResponse, requestKey *resourcepb.ResourceKey) {
+	if legacyResponse == nil || unifiedResponse == nil {
+		return
+	}
+
+	legacyUIDs := extractUIDs(legacyResponse)
+	unifiedUIDs := extractUIDs(unifiedResponse)
+
+	matchPercentage := calculateMatchPercentage(legacyUIDs, unifiedUIDs)
+
+	// Determine resource type for labeling - handle nil safely
+	resourceType := "unknown"
+	if requestKey != nil && requestKey.Resource != "" {
+		resourceType = requestKey.Resource
+	}
+
+	s.logger.Debug("Search results comparison completed",
+		"resource_type", resourceType,
+		"legacy_count", len(legacyUIDs),
+		"unified_count", len(unifiedUIDs),
+		"match_percentage", fmt.Sprintf("%.1f%%", matchPercentage),
+		"legacy_total_hits", legacyResponse.TotalHits,
+		"unified_total_hits", unifiedResponse.TotalHits,
+	)
+
+	searchResultsMatchHistogram.WithLabelValues(resourceType).Observe(matchPercentage)
+}
@@ -418,3 +418,127 @@ func TestSearchWrapper_GetStats(t *testing.T) {
 		unifiedClient.AssertExpectations(t)
 	})
 }
+
+func TestExtractUIDs(t *testing.T) {
+	tests := []struct {
+		name     string
+		response *resourcepb.ResourceSearchResponse
+		expected map[string]struct{}
+	}{
+		{
+			name:     "nil response",
+			response: nil,
+			expected: map[string]struct{}{},
+		},
+		{
+			name: "empty results",
+			response: &resourcepb.ResourceSearchResponse{
+				Results: &resourcepb.ResourceTable{
+					Rows: []*resourcepb.ResourceTableRow{},
+				},
+			},
+			expected: map[string]struct{}{},
+		},
+		{
+			name: "single result",
+			response: &resourcepb.ResourceSearchResponse{
+				Results: &resourcepb.ResourceTable{
+					Rows: []*resourcepb.ResourceTableRow{
+						{
+							Key: &resourcepb.ResourceKey{
+								Name: "test-uid-1",
+							},
+						},
+					},
+				},
+			},
+			expected: map[string]struct{}{"test-uid-1": {}},
+		},
+		{
+			name: "multiple results",
+			response: &resourcepb.ResourceSearchResponse{
+				Results: &resourcepb.ResourceTable{
+					Rows: []*resourcepb.ResourceTableRow{
+						{
+							Key: &resourcepb.ResourceKey{
+								Name: "test-uid-1",
+							},
+						},
+						{
+							Key: &resourcepb.ResourceKey{
+								Name: "test-uid-2",
+							},
+						},
+					},
+				},
+			},
+			expected: map[string]struct{}{"test-uid-1": {}, "test-uid-2": {}},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := extractUIDs(tt.response)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestCalculateMatchPercentage(t *testing.T) {
+	tests := []struct {
+		name        string
+		legacyUIDs  map[string]struct{}
+		unifiedUIDs map[string]struct{}
+		expected    float64
+	}{
+		{
+			name:        "both empty",
+			legacyUIDs:  map[string]struct{}{},
+			unifiedUIDs: map[string]struct{}{},
+			expected:    100.0,
+		},
+		{
+			name:        "legacy empty, unified has results",
+			legacyUIDs:  map[string]struct{}{},
+			unifiedUIDs: map[string]struct{}{"uid1": {}},
+			expected:    0.0,
+		},
+		{
+			name:        "legacy has results, unified empty",
+			legacyUIDs:  map[string]struct{}{"uid1": {}},
+			unifiedUIDs: map[string]struct{}{},
+			expected:    0.0,
+		},
+		{
+			name:        "perfect match",
+			legacyUIDs:  map[string]struct{}{"uid1": {}, "uid2": {}},
+			unifiedUIDs: map[string]struct{}{"uid1": {}, "uid2": {}},
+			expected:    100.0,
+		},
+		{
+			name:        "partial match",
+			legacyUIDs:  map[string]struct{}{"uid1": {}, "uid2": {}},
+			unifiedUIDs: map[string]struct{}{"uid1": {}, "uid3": {}},
+			expected:    50.0, // 1 match out of 2 legacy UIDs (recall)
+		},
+		{
+			name:        "no match",
+			legacyUIDs:  map[string]struct{}{"uid1": {}, "uid2": {}},
+			unifiedUIDs: map[string]struct{}{"uid3": {}, "uid4": {}},
+			expected:    0.0,
+		},
+		{
+			name:        "legacy subset of unified",
+			legacyUIDs:  map[string]struct{}{"uid1": {}},
+			unifiedUIDs: map[string]struct{}{"uid1": {}, "uid2": {}},
+			expected:    100.0, // 1 match out of 1 legacy UID (perfect recall)
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := calculateMatchPercentage(tt.legacyUIDs, tt.unifiedUIDs)
+			assert.InDelta(t, tt.expected, result, 0.001)
+		})
+	}
+}