Query-tee: Allow skipping comparison when preferred backend fails (#1…

…0612) * Query-tee: Allow skipping comparison when preferred backend fails My use case is that my secondary backend allows more requests through than the preferred backend (where they fail) * Add CHANGELOG * Use New() arg instead * Fix changelog * Apply suggestions from code review Co-authored-by: Charles Korn <charleskorn@users.noreply.github.com> Co-authored-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com> --------- Co-authored-by: Charles Korn <charleskorn@users.noreply.github.com> Co-authored-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
grafana · Feb 10, 2025 · cc085ad · cc085ad
1 parent 6da8a1f
commit cc085ad
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -103,6 +103,8 @@
 
 ### Query-tee
 
+* [ENHANCEMENT] Allow skipping comparisons when preferred backend fails. Disabled by default, enable with `-proxy.compare-skip-preferred-backend-failures=true`. #10612
+
 ### Documentation
 
 * [CHANGE] Add production tips related to cache size, heavy multi-tenancy and latency spikes. #9978

diff --git a/tools/querytee/proxy.go b/tools/querytee/proxy.go
@@ -51,6 +51,7 @@ type ProxyConfig struct {
 	BackendSkipTLSVerify                bool
 	AddMissingTimeParamToInstantQueries bool
 	SecondaryBackendsRequestProportion  float64
+	SkipPreferredBackendFailures        bool
 }
 
 type BackendConfig struct {
@@ -93,6 +94,7 @@ func (cfg *ProxyConfig) RegisterFlags(f *flag.FlagSet) {
 	f.DurationVar(&cfg.SkipRecentSamples, "proxy.compare-skip-recent-samples", 2*time.Minute, "The window from now to skip comparing samples. 0 to disable.")
 	f.Var(&cfg.SkipSamplesBefore, "proxy.compare-skip-samples-before", "Skip the samples before the given time for comparison. The time can be in RFC3339 format (or) RFC3339 without the timezone and seconds (or) date only.")
 	f.BoolVar(&cfg.RequireExactErrorMatch, "proxy.compare-exact-error-matching", false, "If true, errors will be considered the same only if they are exactly the same. If false, errors will be considered the same if they are considered equivalent.")
+	f.BoolVar(&cfg.SkipPreferredBackendFailures, "proxy.compare-skip-preferred-backend-failures", false, "If true, the proxy will 'skip' result comparisons where the preferred backend returned a 5xx status code. This is useful in the case where the secondary backend is more reliable.")
 	f.BoolVar(&cfg.PassThroughNonRegisteredRoutes, "proxy.passthrough-non-registered-routes", false, "Passthrough requests for non-registered routes to preferred backend.")
 	f.BoolVar(&cfg.AddMissingTimeParamToInstantQueries, "proxy.add-missing-time-parameter-to-instant-queries", true, "Add a 'time' parameter to proxied instant query requests if they do not have one.")
 	f.Float64Var(&cfg.SecondaryBackendsRequestProportion, "proxy.secondary-backends-request-proportion", 1.0, "Proportion of requests to send to secondary backends. Must be between 0 and 1 (inclusive), and if not 1, then -backend.preferred must be set.")
@@ -305,7 +307,7 @@ func (p *Proxy) Start() error {
 		if p.cfg.CompareResponses {
 			comparator = route.ResponseComparator
 		}
-		router.Path(route.Path).Methods(route.Methods...).Handler(NewProxyEndpoint(p.backends, route, p.metrics, p.logger, comparator, p.cfg.LogSlowQueryResponseThreshold, p.cfg.SecondaryBackendsRequestProportion))
+		router.Path(route.Path).Methods(route.Methods...).Handler(NewProxyEndpoint(p.backends, route, p.metrics, p.logger, comparator, p.cfg.LogSlowQueryResponseThreshold, p.cfg.SecondaryBackendsRequestProportion, p.cfg.SkipPreferredBackendFailures))
 	}
 
 	if p.cfg.PassThroughNonRegisteredRoutes {

diff --git a/tools/querytee/proxy_endpoint.go b/tools/querytee/proxy_endpoint.go
@@ -34,14 +34,15 @@ type ProxyEndpoint struct {
 	comparator                        ResponsesComparator
 	slowResponseThreshold             time.Duration
 	secondaryBackendRequestProportion float64
+	skipPreferredBackendFailures      bool
 
 	// The preferred backend, if any.
 	preferredBackend ProxyBackendInterface
 
 	route Route
 }
 
-func NewProxyEndpoint(backends []ProxyBackendInterface, route Route, metrics *ProxyMetrics, logger log.Logger, comparator ResponsesComparator, slowResponseThreshold time.Duration, secondaryBackendRequestProportion float64) *ProxyEndpoint {
+func NewProxyEndpoint(backends []ProxyBackendInterface, route Route, metrics *ProxyMetrics, logger log.Logger, comparator ResponsesComparator, slowResponseThreshold time.Duration, secondaryBackendRequestProportion float64, skipPreferredBackendFailures bool) *ProxyEndpoint {
 	var preferredBackend ProxyBackendInterface
 	for _, backend := range backends {
 		if backend.Preferred() {
@@ -59,6 +60,7 @@ func NewProxyEndpoint(backends []ProxyBackendInterface, route Route, metrics *Pr
 		slowResponseThreshold:             slowResponseThreshold,
 		secondaryBackendRequestProportion: secondaryBackendRequestProportion,
 		preferredBackend:                  preferredBackend,
+		skipPreferredBackendFailures:      skipPreferredBackendFailures,
 	}
 }
 
@@ -316,6 +318,10 @@ func (p *ProxyEndpoint) waitBackendResponseForDownstream(resCh chan *backendResp
 }
 
 func (p *ProxyEndpoint) compareResponses(expectedResponse, actualResponse *backendResponse, queryEvaluationTime time.Time) (ComparisonResult, error) {
+	if !expectedResponse.succeeded() && p.skipPreferredBackendFailures {
+		return ComparisonSkipped, fmt.Errorf("skipped comparison of response because the request to the preferred backend failed")
+	}
+
 	if expectedResponse.err != nil {
 		return ComparisonFailed, fmt.Errorf("skipped comparison of response because the request to the preferred backend failed: %w", expectedResponse.err)
 	}

diff --git a/tools/querytee/proxy_endpoint_test.go b/tools/querytee/proxy_endpoint_test.go
@@ -115,7 +115,7 @@ func Test_ProxyEndpoint_waitBackendResponseForDownstream(t *testing.T) {
 		testData := testData
 
 		t.Run(testName, func(t *testing.T) {
-			endpoint := NewProxyEndpoint(testData.backends, testRoute, NewProxyMetrics(nil), log.NewNopLogger(), nil, 0, 1.0)
+			endpoint := NewProxyEndpoint(testData.backends, testRoute, NewProxyMetrics(nil), log.NewNopLogger(), nil, 0, 1.0, false)
 
 			// Send the responses from a dedicated goroutine.
 			resCh := make(chan *backendResponse)
@@ -160,7 +160,7 @@ func Test_ProxyEndpoint_Requests(t *testing.T) {
 		NewProxyBackend("backend-1", backendURL1, time.Second, true, false, defaultBackendConfig()),
 		NewProxyBackend("backend-2", backendURL2, time.Second, false, false, defaultBackendConfig()),
 	}
-	endpoint := NewProxyEndpoint(backends, testRoute, NewProxyMetrics(nil), log.NewNopLogger(), nil, 0, 1.0)
+	endpoint := NewProxyEndpoint(backends, testRoute, NewProxyMetrics(nil), log.NewNopLogger(), nil, 0, 1.0, false)
 
 	for _, tc := range []struct {
 		name    string
@@ -255,6 +255,7 @@ func Test_ProxyEndpoint_Comparison(t *testing.T) {
 		comparatorError              error
 		expectedComparisonResult     ComparisonResult
 		expectedComparisonError      string
+		skipPreferredBackendFailures bool
 	}{
 		"responses are the same": {
 			preferredResponseStatusCode:  http.StatusOK,
@@ -298,6 +299,23 @@ func Test_ProxyEndpoint_Comparison(t *testing.T) {
 			expectedComparisonError:      "skipped comparison of response because the response from the secondary backend contained an unexpected content type 'text/plain', expected 'application/json'",
 			expectedComparisonResult:     ComparisonSkipped,
 		},
+		"preferred backend failed but is set to skip": {
+			preferredResponseStatusCode:  http.StatusInternalServerError,
+			secondaryResponseStatusCode:  http.StatusOK,
+			preferredResponseContentType: "application/json",
+			secondaryResponseContentType: "application/json",
+			expectedComparisonResult:     ComparisonSkipped,
+			expectedComparisonError:      "skipped comparison of response because the request to the preferred backend failed",
+			skipPreferredBackendFailures: true,
+		},
+		"preferred backend failed, not set to skip": {
+			preferredResponseStatusCode:  http.StatusInternalServerError,
+			secondaryResponseStatusCode:  http.StatusOK,
+			preferredResponseContentType: "application/json",
+			secondaryResponseContentType: "application/json",
+			expectedComparisonResult:     ComparisonFailed,
+			expectedComparisonError:      "expected status code 500 (returned by preferred backend) but got 200 from secondary backend",
+		},
 	}
 
 	for name, scenario := range scenarios {
@@ -336,7 +354,7 @@ func Test_ProxyEndpoint_Comparison(t *testing.T) {
 				comparisonError:  scenario.comparatorError,
 			}
 
-			endpoint := NewProxyEndpoint(backends, testRoute, NewProxyMetrics(reg), logger, comparator, 0, 1.0)
+			endpoint := NewProxyEndpoint(backends, testRoute, NewProxyMetrics(reg), logger, comparator, 0, 1.0, scenario.skipPreferredBackendFailures)
 
 			resp := httptest.NewRecorder()
 			req, err := http.NewRequest("GET", "http://test/api/v1/test", nil)
@@ -438,7 +456,7 @@ func Test_ProxyEndpoint_LogSlowQueries(t *testing.T) {
 				comparisonResult: ComparisonSuccess,
 			}
 
-			endpoint := NewProxyEndpoint(backends, testRoute, NewProxyMetrics(reg), logger, comparator, scenario.slowResponseThreshold, 1.0)
+			endpoint := NewProxyEndpoint(backends, testRoute, NewProxyMetrics(reg), logger, comparator, scenario.slowResponseThreshold, 1.0, false)
 
 			resp := httptest.NewRecorder()
 			req, err := http.NewRequest("GET", "http://test/api/v1/test", nil)
@@ -507,7 +525,7 @@ func Test_ProxyEndpoint_RelativeDurationMetric(t *testing.T) {
 				comparisonResult: ComparisonSuccess,
 			}
 
-			endpoint := NewProxyEndpoint(backends, testRoute, NewProxyMetrics(reg), logger, comparator, 0, 1.0)
+			endpoint := NewProxyEndpoint(backends, testRoute, NewProxyMetrics(reg), logger, comparator, 0, 1.0, false)
 
 			resp := httptest.NewRecorder()
 			req, err := http.NewRequest("GET", "http://test/api/v1/test", nil)
@@ -842,7 +860,7 @@ func TestProxyEndpoint_BackendSelection(t *testing.T) {
 
 	for name, testCase := range testCases {
 		t.Run(name, func(t *testing.T) {
-			proxyEndpoint := NewProxyEndpoint(testCase.backends, Route{}, nil, nil, nil, 0, testCase.secondaryBackendRequestProportion)
+			proxyEndpoint := NewProxyEndpoint(testCase.backends, Route{}, nil, nil, nil, 0, testCase.secondaryBackendRequestProportion, false)
 			preferredOnlySelectionCount := 0
 
 			for i := 0; i < runCount; i++ {