refactor: http logger option

http/http.go

@@ -66,6 +66,7 @@ func WithErrorReporter(errorReporter observability.ErrorReporter) OptionFunc {
 func WithLogger(logger *zerolog.Logger) OptionFunc {
 	return func(h *httpClient) {
 		h.logger = logger
+		h.errorReporter = observability.NewErrorReporter(logger)
 	}
 }
 
@@ -98,36 +99,40 @@ var retryErrorCodes = map[int]bool{
 	http.StatusInternalServerError: true,
 }
 
-func (s *httpClient) Do(req *http.Request) (response *http.Response, err error) {
+func (s *httpClient) Do(req *http.Request) (*http.Response, error) {
 	span := s.instrumentor.StartSpan(req.Context(), "http.Do")
 	defer s.instrumentor.Finish(span)
 
-	for i := 0; i < s.retryCount; i++ {
+	retryCount := s.retryCount
+	for {
 		requestId := span.GetTraceId()
 		req.Header.Set("snyk-request-id", requestId)
 
-		response, err = s.httpCall(req)
+		response, err := s.httpCall(req)
 		if err != nil {
 			return nil, err // no retries for errors
 		}
 
-		if retryErrorCodes[response.StatusCode] {
-			s.logger.Debug().Err(err).Str("method", req.Method).Int("attempts done", i+1).Msg("retrying")
-			if i < s.retryCount-1 {
-				time.Sleep(5 * time.Second)
-				continue
-			}
+		if retryCount > 0 && retryErrorCodes[response.StatusCode] {
+			s.logger.Debug().Err(err).Int("attempts left", retryCount).Msg("retrying")
+			retryCount--
+			time.Sleep(5 * time.Second)
+			continue
 		}
 
-		// no error, we can break the retry loop
-		break
+		// should return
+		return response, err
 	}
-	return response, err
 }
 
 func (s *httpClient) httpCall(req *http.Request) (*http.Response, error) {
-	log := s.logger.With().Str("method", "http.httpCall").Logger()
 	requestId := req.Header.Get("snyk-request-id")
+	log := s.logger.With().
+		Str("method", "http.httpCall").
+		Str("reqMethod", req.Method).
+		Str("url", req.URL.String()).
+		Str("snyk-request-id", requestId).
+		Logger()
 
 	// store the request body so that after retrying it can be read again
 	var copyReqBody io.ReadCloser
@@ -137,7 +142,7 @@ func (s *httpClient) httpCall(req *http.Request) (*http.Response, error) {
 		reqBody := io.NopCloser(bytes.NewBuffer(reqBuf))
 		copyReqBody = io.NopCloser(bytes.NewBuffer(reqBuf))
 		req.Body = reqBody
-		s.logger.Debug().Str("url", req.URL.String()).Str("snyk-request-id", requestId).Str("requestBody", string(reqBuf)).Msg("SEND TO REMOTE")
+		s.logger.Debug().Msg("SEND TO REMOTE")
 	}
 	response, err := s.httpClientFactory().Do(req)
 	req.Body = copyReqBody
@@ -147,9 +152,9 @@ func (s *httpClient) httpCall(req *http.Request) (*http.Response, error) {
 		resBuf, _ = io.ReadAll(response.Body)
 		copyResBody = io.NopCloser(bytes.NewBuffer(resBuf))
 		response.Body = copyResBody
-		s.logger.Debug().Str("url", req.URL.String()).Str("response.Status", response.Status).Str("snyk-request-id", requestId).Str("responseBody", string(resBuf)).Msg("RECEIVED FROM REMOTE")
+		s.logger.Debug().Str("response.Status", response.Status).Msg("RECEIVED FROM REMOTE")
 	} else {
-		s.logger.Debug().Str("url", req.URL.String()).Str("snyk-request-id", requestId).Msg("RECEIVED FROM REMOTE")
+		s.logger.Debug().Msg("RECEIVED FROM REMOTE")
 	}
 
 	if err != nil {

http/http_test.go

@@ -28,7 +28,6 @@ import (
 	"github.com/stretchr/testify/require"
 
 	codeClientHTTP "github.com/snyk/code-client-go/http"
-	"github.com/snyk/code-client-go/observability"
 	"github.com/snyk/code-client-go/observability/mocks"
 )
 
@@ -56,7 +55,7 @@ func (d *dummyTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 	}, nil
 }
 
-func TestSnykCodeBackendService_DoCall_shouldRetry(t *testing.T) {
+func TestSnykCodeBackendService_DoCall_shouldRetryWithARequestBody(t *testing.T) {
 	d := &dummyTransport{responseCode: 502, status: "502 Bad Gateway"}
 	dummyClientFactory := func() *http.Client {
 		return &http.Client{
@@ -72,7 +71,7 @@ func TestSnykCodeBackendService_DoCall_shouldRetry(t *testing.T) {
 	mockInstrumentor.EXPECT().Finish(gomock.Any()).Times(1)
 	mockErrorReporter := mocks.NewMockErrorReporter(ctrl)
 
-	req, err := http.NewRequest(http.MethodGet, "https://httpstat.us/500", nil)
+	req, err := http.NewRequest(http.MethodGet, "https://127.0.0.1", io.NopCloser(strings.NewReader("body")))
 	require.NoError(t, err)
 
 	s := codeClientHTTP.NewHTTPClient(
@@ -85,11 +84,11 @@ func TestSnykCodeBackendService_DoCall_shouldRetry(t *testing.T) {
 	res, err := s.Do(req)
 	assert.NoError(t, err)
 	assert.NotNil(t, res)
-	assert.Equal(t, 3, d.calls)
+	assert.Equal(t, 4, d.calls)
 }
 
-func TestSnykCodeBackendService_DoCall_shouldRetryWithARequestBody(t *testing.T) {
-	d := &dummyTransport{responseCode: 502, status: "502 Bad Gateway"}
+func TestSnykCodeBackendService_DoCall_shouldSucceed(t *testing.T) {
+	d := &dummyTransport{responseCode: 200}
 	dummyClientFactory := func() *http.Client {
 		return &http.Client{
 			Transport: d,
@@ -104,7 +103,7 @@ func TestSnykCodeBackendService_DoCall_shouldRetryWithARequestBody(t *testing.T)
 	mockInstrumentor.EXPECT().Finish(gomock.Any()).Times(1)
 	mockErrorReporter := mocks.NewMockErrorReporter(ctrl)
 
-	req, err := http.NewRequest(http.MethodGet, "https://httpstat.us/500", io.NopCloser(strings.NewReader("body")))
+	req, err := http.NewRequest(http.MethodGet, "https://httpstat.us/200", nil)
 	require.NoError(t, err)
 
 	s := codeClientHTTP.NewHTTPClient(
@@ -117,12 +116,15 @@ func TestSnykCodeBackendService_DoCall_shouldRetryWithARequestBody(t *testing.T)
 	res, err := s.Do(req)
 	assert.NoError(t, err)
 	assert.NotNil(t, res)
-	assert.Equal(t, 3, d.calls)
+	assert.Equal(t, 1, d.calls)
 }
 
-func TestSnykCodeBackendService_doCall_rejected(t *testing.T) {
+func TestSnykCodeBackendService_DoCall_shouldFail(t *testing.T) {
+	d := &dummyTransport{responseCode: 400, status: "400 Bad Request"}
 	dummyClientFactory := func() *http.Client {
-		return &http.Client{}
+		return &http.Client{
+			Transport: d,
+		}
 	}
 
 	ctrl := gomock.NewController(t)
@@ -132,20 +134,20 @@ func TestSnykCodeBackendService_doCall_rejected(t *testing.T) {
 	mockInstrumentor.EXPECT().StartSpan(gomock.Any(), gomock.Any()).Return(mockSpan).Times(1)
 	mockInstrumentor.EXPECT().Finish(gomock.Any()).Times(1)
 	mockErrorReporter := mocks.NewMockErrorReporter(ctrl)
-	mockErrorReporter.EXPECT().CaptureError(gomock.Any(), observability.ErrorReporterOptions{ErrorDiagnosticPath: ""})
 
 	req, err := http.NewRequest(http.MethodGet, "https://127.0.0.1", nil)
 	require.NoError(t, err)
 
 	s := codeClientHTTP.NewHTTPClient(
 		dummyClientFactory,
-		codeClientHTTP.WithRetryCount(3),
+		codeClientHTTP.WithRetryCount(1),
 		codeClientHTTP.WithInstrumentor(mockInstrumentor),
 		codeClientHTTP.WithErrorReporter(mockErrorReporter),
 		codeClientHTTP.WithLogger(newLogger(t)),
 	)
-	_, err = s.Do(req)
-	assert.Error(t, err)
+	response, err := s.Do(req)
+	assert.Equal(t, "400 Bad Request", response.Status)
+	assert.NoError(t, err)
 }
 
 func newLogger(t *testing.T) *zerolog.Logger {

internal/analysis/analysis_test.go

@@ -209,7 +209,7 @@ func TestAnalysis_CreateWorkspace_KnownErrors(t *testing.T) {
 
 			logger := zerolog.Nop()
 
-			analysisOrchestrator := analysis.NewAnalysisOrchestrator(&logger, mockHTTPClient, mockInstrumentor, mockErrorReporter, mockConfig)
+			analysisOrchestrator := analysis.NewAnalysisOrchestrator(mockConfig, &logger, mockHTTPClient, mockInstrumentor, mockErrorReporter)
 			_, err := analysisOrchestrator.CreateWorkspace(
 				context.Background(),
 				"4a72d1db-b465-4764-99e1-ecedad03b06a",