From 957e2814459f995d7e9684ce0fd15d0c43f96c8c Mon Sep 17 00:00:00 2001 From: Arthur Pitman Date: Wed, 21 Aug 2024 16:38:08 +0200 Subject: [PATCH] chore: Remove SendWithRetry --- pkg/client/dtclient/config_client.go | 44 +++++----- pkg/client/dtclient/retry.go | 46 +--------- pkg/client/dtclient/retry_test.go | 113 ------------------------- pkg/client/dtclient/settings_client.go | 8 +- 4 files changed, 34 insertions(+), 177 deletions(-) delete mode 100644 pkg/client/dtclient/retry_test.go diff --git a/pkg/client/dtclient/config_client.go b/pkg/client/dtclient/config_client.go index 3cecbdbba..48e5f1d9c 100644 --- a/pkg/client/dtclient/config_client.go +++ b/pkg/client/dtclient/config_client.go @@ -275,43 +275,49 @@ func (d *DynatraceClient) callWithRetryOnKnowTimingIssue(ctx context.Context, re return resp, nil } - apiError := coreapi.APIError{} - if !errors.As(err, &apiError) { + apiErr := coreapi.APIError{} + if !errors.As(err, &apiErr) { return nil, err } - var rs RetrySetting + rs := selectRetrySettingByAPIAndAPIError(d.retrySettings, theApi, apiErr) + if rs.MaxRetries == 0 { + return resp, err + } + + return coreapi.AsResponseOrError(restCall(ctx, endpoint, bytes.NewReader(requestBody), corerest.RequestOptions{ /* WITH RETRY SETTINGS HERE */ })) +} + +func selectRetrySettingByAPIAndAPIError(retrySettings RetrySettings, theAPI api.API, apiErr coreapi.APIError) RetrySetting { + rs := RetrySetting{} + // It can take longer until calculated service metrics are ready to be used in SLOs - if isCalculatedMetricNotReadyYet(apiError) || + if isCalculatedMetricNotReadyYet(apiErr) || // It can take longer until management zones are ready to be used in SLOs - isManagementZoneNotReadyYet(apiError) || + isManagementZoneNotReadyYet(apiErr) || // It can take longer until Credentials are ready to be used in Synthetic Monitors - isCredentialNotReadyYet(apiError) || + isCredentialNotReadyYet(apiErr) || // It can take some time for configurations to propagate to all cluster nodes - indicated by an incorrect constraint violation error - isGeneralDependencyNotReadyYet(apiError) || + isGeneralDependencyNotReadyYet(apiErr) || // Synthetic and related APIs sometimes run into issues of finding objects quickly after creation - isGeneralSyntheticAPIError(apiError, theApi) || + isGeneralSyntheticAPIError(apiErr, theAPI) || // Network zone deployments can fail due to fact that the feature not effectively enabled yet - isNetworkZoneFeatureNotEnabledYet(apiError, theApi) { + isNetworkZoneFeatureNotEnabledYet(apiErr, theAPI) { - rs = d.retrySettings.Normal + rs = retrySettings.Normal } // It can take even longer until request attributes are ready to be used - if isRequestAttributeNotYetReady(apiError) { - rs = d.retrySettings.Long + if isRequestAttributeNotYetReady(apiErr) { + rs = retrySettings.Long } // It can take even longer until applications are ready to be used in synthetic tests and calculated metrics - if isApplicationNotReadyYet(apiError, theApi) { - rs = d.retrySettings.VeryLong - } - - if rs.MaxRetries > 0 { - return SendWithRetry(ctx, restCall, endpoint, corerest.RequestOptions{}, requestBody, rs) + if isApplicationNotReadyYet(apiErr, theAPI) { + rs = retrySettings.VeryLong } - return resp, err + return rs } func isGeneralDependencyNotReadyYet(apiError coreapi.APIError) bool { diff --git a/pkg/client/dtclient/retry.go b/pkg/client/dtclient/retry.go index cfef376b3..860e6b96c 100644 --- a/pkg/client/dtclient/retry.go +++ b/pkg/client/dtclient/retry.go @@ -17,17 +17,12 @@ package dtclient import ( - "bytes" "context" - "errors" - "fmt" + + corerest "github.com/dynatrace/dynatrace-configuration-as-code-core/api/rest" "io" "net/http" "time" - - coreapi "github.com/dynatrace/dynatrace-configuration-as-code-core/api" - corerest "github.com/dynatrace/dynatrace-configuration-as-code-core/api/rest" - "github.com/dynatrace/dynatrace-configuration-as-code/v2/internal/log" ) type RetrySetting struct { @@ -58,40 +53,3 @@ var DefaultRetrySettings = RetrySettings{ // SendRequestWithBody is a function doing a PUT or POST HTTP request type SendRequestWithBody func(ctx context.Context, endpoint string, body io.Reader, options corerest.RequestOptions) (*http.Response, error) - -// SendWithRetry will retry to call sendWithBody for a given number of times, waiting a give duration between calls -func SendWithRetry(ctx context.Context, sendWithBody SendRequestWithBody, endpoint string, requestOptions corerest.RequestOptions, body []byte, setting RetrySetting) (*coreapi.Response, error) { - var err error - var resp *coreapi.Response - - for i := 0; i < setting.MaxRetries; i++ { - log.WithCtxFields(ctx).Warn("Failed to send HTTP request. Waiting for %s before retrying...", setting.WaitTime) - time.Sleep(setting.WaitTime) - resp, err = coreapi.AsResponseOrError(sendWithBody(ctx, endpoint, bytes.NewReader(body), requestOptions)) - if err == nil { - return resp, nil - } - - apierror := coreapi.APIError{} - if !errors.As(err, &apierror) { - return nil, err - } - } - - return nil, fmt.Errorf("HTTP send request %s failed after %d retries: %w", endpoint, setting.MaxRetries, err) -} - -// SendWithRetryWithInitialTry will try to call sendWithBody and if it didn't succeed call [SendWithRetry] -func SendWithRetryWithInitialTry(ctx context.Context, sendWithBody SendRequestWithBody, endpoint string, requestOptions corerest.RequestOptions, body []byte, setting RetrySetting) (*coreapi.Response, error) { - resp, err := coreapi.AsResponseOrError(sendWithBody(ctx, endpoint, bytes.NewReader(body), requestOptions)) - if err == nil { - return resp, nil - } - - apierror := coreapi.APIError{} - if !errors.As(err, &apierror) { - return nil, err - } - - return SendWithRetry(ctx, sendWithBody, endpoint, requestOptions, body, setting) -} diff --git a/pkg/client/dtclient/retry_test.go b/pkg/client/dtclient/retry_test.go deleted file mode 100644 index e8f6c144a..000000000 --- a/pkg/client/dtclient/retry_test.go +++ /dev/null @@ -1,113 +0,0 @@ -//go:build unit - -/* - * @license - * Copyright 2023 Dynatrace LLC - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package dtclient - -import ( - "context" - "fmt" - "io" - "net/http" - "strings" - "testing" - - coreapi "github.com/dynatrace/dynatrace-configuration-as-code-core/api" - corerest "github.com/dynatrace/dynatrace-configuration-as-code-core/api/rest" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func Test_sendWithsendWithRetryReturnsFirstSuccessfulResponse(t *testing.T) { - i := 0 - mockCall := SendRequestWithBody(func(ctx context.Context, endpoint string, data io.Reader, options corerest.RequestOptions) (*http.Response, error) { - if i < 3 { - i++ - return nil, coreapi.APIError{StatusCode: 400} - } - return &http.Response{ - StatusCode: 200, - Body: io.NopCloser(strings.NewReader("Success")), - }, nil - }) - - gotResp, err := SendWithRetry(context.TODO(), mockCall, "some/path", corerest.RequestOptions{}, []byte("Success"), RetrySetting{MaxRetries: 5}) - require.NoError(t, err) - assert.Equal(t, 200, gotResp.StatusCode) - assert.Equal(t, "Success", string(gotResp.Data)) -} - -func Test_sendWithRetryFailsAfterDefinedTries(t *testing.T) { - maxRetries := 2 - i := 0 - mockCall := SendRequestWithBody(func(ctx context.Context, url string, data io.Reader, options corerest.RequestOptions) (*http.Response, error) { - if i < maxRetries+1 { - i++ - return nil, coreapi.APIError{StatusCode: 400} - } - return &http.Response{ - StatusCode: 200, - Body: io.NopCloser(strings.NewReader("Success")), - }, nil - }) - - _, err := SendWithRetry(context.TODO(), mockCall, "some/path", corerest.RequestOptions{}, []byte("body"), RetrySetting{MaxRetries: maxRetries}) - require.Error(t, err) - assert.Equal(t, 2, i) -} - -func Test_sendWithRetryReturnContainsOriginalApiError(t *testing.T) { - maxRetries := 2 - i := 0 - mockCall := SendRequestWithBody(func(ctx context.Context, url string, data io.Reader, options corerest.RequestOptions) (*http.Response, error) { - if i < maxRetries+1 { - i++ - return nil, fmt.Errorf("Something wrong") - } - return &http.Response{ - StatusCode: 200, - Body: io.NopCloser(strings.NewReader("Success")), - }, nil - }) - - _, err := SendWithRetry(context.TODO(), mockCall, "some/path", corerest.RequestOptions{}, []byte("body"), RetrySetting{MaxRetries: maxRetries}) - require.Error(t, err) - assert.ErrorContains(t, err, "Something wrong") -} - -func Test_sendWithRetryReturnsIfNotSuccess(t *testing.T) { - maxRetries := 2 - i := 0 - mockCall := SendRequestWithBody(func(ctx context.Context, url string, data io.Reader, options corerest.RequestOptions) (*http.Response, error) { - if i < maxRetries+1 { - i++ - return &http.Response{ - StatusCode: 400, - Body: io.NopCloser(strings.NewReader("{ err: 'failed to create thing'}")), - }, nil - } - return &http.Response{ - StatusCode: 200, - Body: io.NopCloser(strings.NewReader("Success")), - }, nil - }) - - _, err := SendWithRetry(context.TODO(), mockCall, "some/path", corerest.RequestOptions{}, []byte("body"), RetrySetting{MaxRetries: maxRetries}) - apiError := coreapi.APIError{} - require.ErrorAs(t, err, &apiError) - assert.Equal(t, 400, apiError.StatusCode) -} diff --git a/pkg/client/dtclient/settings_client.go b/pkg/client/dtclient/settings_client.go index e0c95cde3..4e553dbc8 100644 --- a/pkg/client/dtclient/settings_client.go +++ b/pkg/client/dtclient/settings_client.go @@ -261,7 +261,13 @@ func (d *DynatraceClient) UpsertSettings(ctx context.Context, obj SettingsObject retrySetting = d.retrySettings.Normal } - resp, err := SendWithRetryWithInitialTry(ctx, d.platformClient.POST, d.settingsObjectAPIPath, corerest.RequestOptions{}, payload, retrySetting) + requestRetrier := corerest.RequestRetrier{ + MaxRetries: retrySetting.MaxRetries, + DelayAfterRetry: retrySetting.WaitTime, + ShouldRetryFunc: corerest.RetryIfNotSuccess, + } + + resp, err := coreapi.AsResponseOrError(d.platformClient.POST(ctx, d.settingsObjectAPIPath, bytes.NewReader(payload), corerest.RequestOptions{CustomRetrier: &requestRetrier})) if err != nil { d.settingsCache.Delete(obj.SchemaId) return DynatraceEntity{}, fmt.Errorf("failed to create or update Settings object with externalId %s: %w", externalID, err)