Improve distributor_test (#6479)

* Fix distributor_test error bug Signed-off-by: Yuri Nikolic <[email protected]> * Fixing review findings Co-authored-by: Nick Pillitteri <[email protected]> --------- Signed-off-by: Yuri Nikolic <[email protected]> Co-authored-by: Nick Pillitteri <[email protected]>
grafana · Oct 25, 2023 · 9bea6d6 · 9bea6d6
1 parent d6b6853
commit 9bea6d6
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 30 deletions.
diff --git a/pkg/distributor/distributor_test.go b/pkg/distributor/distributor_test.go
@@ -4785,8 +4785,6 @@ func TestSeriesAreShardedToCorrectIngesters(t *testing.T) {
 func TestHandleIngesterPushError(t *testing.T) {
 	testErrorMsg := "this is a test error message"
 	outputErrorMsgPrefix := "failed pushing to ingester"
-	userID := "test"
-	errWithUserID := fmt.Errorf("user=%s: %s", userID, testErrorMsg)
 
 	// Ensure that no error gets translated into no error.
 	t.Run("no error gives no error", func(t *testing.T) {
@@ -4826,7 +4824,7 @@ func TestHandleIngesterPushError(t *testing.T) {
 		})
 	}
 
-	// Ensure that the errors created by gogo/status package  get translated
+	// Ensure that the errors created by gogo/status package get translated
 	// into ingesterPushError messages.
 	statusTests := map[string]struct {
 		ingesterPushError         error
@@ -4836,6 +4834,11 @@ func TestHandleIngesterPushError(t *testing.T) {
 			ingesterPushError:         createStatusWithDetails(t, codes.Unavailable, testErrorMsg, mimirpb.INVALID).Err(),
 			expectedIngesterPushError: newIngesterPushError(createStatusWithDetails(t, codes.Unavailable, testErrorMsg, mimirpb.INVALID)),
 		},
+		"a DeadlineExceeded gRPC ingester error an ingesterPushError with INVALID cause": {
+			// This is how context.DeadlineExceeded error is translated into a gRPC error.
+			ingesterPushError:         status.Error(codes.DeadlineExceeded, context.DeadlineExceeded.Error()),
+			expectedIngesterPushError: newIngesterPushError(createStatusWithDetails(t, codes.Unavailable, context.DeadlineExceeded.Error(), mimirpb.INVALID)),
+		},
 		"an Unavailable gRPC error without details gives an ingesterPushError with INVALID cause": {
 			ingesterPushError:         status.Error(codes.Unavailable, testErrorMsg),
 			expectedIngesterPushError: newIngesterPushError(createStatusWithDetails(t, codes.Unavailable, testErrorMsg, mimirpb.INVALID)),
@@ -4859,33 +4862,6 @@ func TestHandleIngesterPushError(t *testing.T) {
 			require.Equal(t, testData.expectedIngesterPushError.errorCause(), ingesterPushErr.errorCause())
 		})
 	}
-
-	// Ensure that the errors with no status (e.g., context.Canceled,
-	// context.DeadLine, etc) are correctly wrapped.
-	otherTests := map[string]struct {
-		ingesterPushError error
-		expectedError     error
-	}{
-		"a random ingester error without status gives the same wrapped error": {
-			ingesterPushError: errWithUserID,
-			expectedError:     errors.Wrap(errWithUserID, outputErrorMsgPrefix),
-		},
-		"a context canceled error gives the same wrapped error": {
-			ingesterPushError: context.Canceled,
-			expectedError:     errors.Wrap(errWithUserID, outputErrorMsgPrefix),
-		},
-		"a context deadline exceeded error gives the same wrapped error": {
-			ingesterPushError: context.DeadlineExceeded,
-			expectedError:     errors.Wrap(errWithUserID, outputErrorMsgPrefix),
-		},
-	}
-	for testName, testData := range otherTests {
-		t.Run(testName, func(t *testing.T) {
-			err := handleIngesterPushError(testData.ingesterPushError)
-			require.Errorf(t, err, testData.expectedError.Error())
-			require.ErrorIs(t, err, testData.ingesterPushError)
-		})
-	}
 }
 
 func TestHandlePushError(t *testing.T) {

diff --git a/pkg/distributor/push_test.go b/pkg/distributor/push_test.go
@@ -944,6 +944,11 @@ func TestHandler_ToHTTPStatus(t *testing.T) {
 			expectedHTTPStatus: http.StatusInternalServerError,
 			expectedErrorMsg:   fmt.Sprintf("%s: %s", failedPushingToIngesterMessage, originalMsg),
 		},
+		"an ingesterPushError obtained from a DeadlineExceeded coming from the ingester gets translated into an HTTP 500": {
+			err:                newIngesterPushError(createStatusWithDetails(t, codes.Internal, context.DeadlineExceeded.Error(), mimirpb.INVALID)),
+			expectedHTTPStatus: http.StatusInternalServerError,
+			expectedErrorMsg:   fmt.Sprintf("%s: %s", failedPushingToIngesterMessage, context.DeadlineExceeded),
+		},
 	}
 	for name, tc := range testCases {
 		t.Run(name, func(t *testing.T) {