parse-errormsgs-in-retryable-cases: In case of retyable scenario for http, only status code comes as a highlighter of failure, actual reason is very difficult to understand. This issue solves parsing of response body in case if we received msg

Author: Preeti Dewani

exporters/otlp/otlpmetric/otlpmetricgrpc/internal/otest/collector.go

@@ -177,7 +177,7 @@ type HTTPResponseError struct {
 }
 
 func (e *HTTPResponseError) Error() string {
-	return fmt.Sprintf("%d: %s", e.Status, e.Err)
+	return e.Err.Error()
 }
 
 func (e *HTTPResponseError) Unwrap() error { return e.Err }

exporters/otlp/otlpmetric/otlpmetrichttp/client.go

@@ -14,17 +14,19 @@ import (
 	"net/http"
 	"net/url"
 	"strconv"
+	"strings"
 	"sync"
 	"time"
 
 	"google.golang.org/protobuf/proto"
 
 	"go.opentelemetry.io/otel"
+	colmetricpb "go.opentelemetry.io/proto/otlp/collector/metrics/v1"
+	metricpb "go.opentelemetry.io/proto/otlp/metrics/v1"
+
 	"go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp/internal"
 	"go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf"
 	"go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp/internal/retry"
-	colmetricpb "go.opentelemetry.io/proto/otlp/collector/metrics/v1"
-	metricpb "go.opentelemetry.io/proto/otlp/metrics/v1"
 )
 
 type client struct {
@@ -189,11 +191,21 @@ func (c *client) UploadMetrics(ctx context.Context, protoMetrics *metricpb.Resou
 			// Retry-able failure.
 			rErr = newResponseError(resp.Header)
 
-			// Going to retry, drain the body to reuse the connection.
-			if _, err := io.Copy(io.Discard, resp.Body); err != nil {
-				_ = resp.Body.Close()
+			// In all these cases, since the response body is
+			// not getting parsed, actual reason of failure is
+			// very hard to find out for the user.
+			// If body is not empty, then we can use the msg
+			// of underlying service as error message.
+			var respData bytes.Buffer
+			if _, err := io.Copy(&respData, resp.Body); err != nil {
 				return err
 			}
+
+			respStr := strings.TrimSpace(respData.String())
+			if respStr != "" {
+				rErr = errors.New(respStr)
+			}
+
 		default:
 			rErr = fmt.Errorf("failed to send metrics to %s: %s", request.URL, resp.Status)
 		}

exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go

@@ -17,11 +17,12 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf"
-	"go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp/internal/otest"
 	"go.opentelemetry.io/otel/sdk/metric"
 	"go.opentelemetry.io/otel/sdk/metric/metricdata"
 	mpb "go.opentelemetry.io/proto/otlp/metrics/v1"
+
+	"go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp/internal/oconf"
+	"go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp/internal/otest"
 )
 
 type clientShim struct {
@@ -192,6 +193,33 @@ func TestConfig(t *testing.T) {
 		assert.Len(t, rCh, 0, "failed HTTP responses did not occur")
 	})
 
+	t.Run("WithRetryAndExporterErr", func(t *testing.T) {
+		exporterErr := errors.New("rpc error: code = Unavailable desc = service.name not found in resource attributes")
+		rCh := make(chan otest.ExportResult, 1)
+		header := http.Header{http.CanonicalHeaderKey("Retry-After"): {"10"}}
+		rCh <- otest.ExportResult{Err: &otest.HTTPResponseError{
+			Status: http.StatusServiceUnavailable,
+			Err:    exporterErr,
+			Header: header,
+		}}
+
+		exp, coll := factoryFunc("", rCh, WithRetry(RetryConfig{
+			Enabled:         true,
+			InitialInterval: time.Nanosecond,
+			MaxInterval:     time.Millisecond,
+			MaxElapsedTime:  time.Minute,
+		}))
+		ctx := context.Background()
+		t.Cleanup(func() { require.NoError(t, coll.Shutdown(ctx)) })
+		// Push this after Shutdown so the HTTP server doesn't hang.
+		t.Cleanup(func() { close(rCh) })
+		t.Cleanup(func() { require.NoError(t, exp.Shutdown(ctx)) })
+
+		target := exp.Export(ctx, &metricdata.ResourceMetrics{})
+		assert.ErrorAs(t, fmt.Errorf("failed to upload metrics: %s", exporterErr.Error()), &target)
+		assert.Len(t, rCh, 0, "failed HTTP responses did not occur")
+	})
+
 	t.Run("WithURLPath", func(t *testing.T) {
 		path := "/prefix/v2/metrics"
 		ePt := fmt.Sprintf("http://localhost:0%s", path)

exporters/otlp/otlpmetric/otlpmetrichttp/internal/otest/collector.go

@@ -177,7 +177,7 @@ type HTTPResponseError struct {
 }
 
 func (e *HTTPResponseError) Error() string {
-	return fmt.Sprintf("%d: %s", e.Status, e.Err)
+	return e.Err.Error()
 }
 
 func (e *HTTPResponseError) Unwrap() error { return e.Err }

internal/shared/otlp/otlpmetric/otest/collector.go.tmpl

@@ -177,7 +177,7 @@ type HTTPResponseError struct {
 }
 
 func (e *HTTPResponseError) Error() string {
-	return fmt.Sprintf("%d: %s", e.Status, e.Err)
+	return e.Err.Error()
 }
 
 func (e *HTTPResponseError) Unwrap() error { return e.Err }