From 170861deec4aa9ec6af48f29ddebeffe3a143e43 Mon Sep 17 00:00:00 2001
From: Preeti Dewani <preetidewani1990@gmail.com>
Date: Mon, 24 Jun 2024 23:18:43 +0530
Subject: [PATCH 01/11] parse-errormsgs-in-retry-able-statuscodes: 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

---
 .../internal/otest/collector.go               |  2 +-
 .../otlp/otlpmetric/otlpmetrichttp/client.go  | 23 +++++++++---
 .../otlpmetric/otlpmetrichttp/client_test.go  | 35 +++++++++++++++++--
 .../internal/otest/collector.go               |  2 +-
 .../otlp/otlpmetric/otest/collector.go.tmpl   |  2 +-
 5 files changed, 53 insertions(+), 11 deletions(-)

diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/otest/collector.go b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/otest/collector.go
index 6eea8d39a75..6125b8cf50e 100644
--- a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/otest/collector.go
+++ b/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 }
diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
index eeb39339d45..2c9a3baee91 100644
--- a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
+++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
@@ -14,17 +14,20 @@ import (
 	"net/http"
 	"net/url"
 	"strconv"
+	"strings"
 	"sync"
 	"time"
 
 	"google.golang.org/protobuf/proto"
 
+	colmetricpb "go.opentelemetry.io/proto/otlp/collector/metrics/v1"
+	metricpb "go.opentelemetry.io/proto/otlp/metrics/v1"
+
 	"go.opentelemetry.io/otel"
+
 	"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 +192,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)
 		}
diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
index 2838fd9d011..3dcd2301369 100644
--- a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
+++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
@@ -17,11 +17,13 @@ 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"
+	mpb "go.opentelemetry.io/proto/otlp/metrics/v1"
+
 	"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 +194,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)
diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/otest/collector.go b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/otest/collector.go
index 178dcde6c36..911fc69ef67 100644
--- a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/otest/collector.go
+++ b/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 }
diff --git a/internal/shared/otlp/otlpmetric/otest/collector.go.tmpl b/internal/shared/otlp/otlpmetric/otest/collector.go.tmpl
index 22ffd923790..83137aac764 100644
--- a/internal/shared/otlp/otlpmetric/otest/collector.go.tmpl
+++ b/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 }

From 6c740ac255f6a5948f8c2d76bc8d04afffaba0cb Mon Sep 17 00:00:00 2001
From: Preeti Dewani <preetidewani1990@gmail.com>
Date: Mon, 24 Jun 2024 23:25:52 +0530
Subject: [PATCH 02/11] parse-errormsgs-in-retry-able-statuscodes: add change
 log detail and PR ID

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 204226a4ac1..8975b4c0ba8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -20,6 +20,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ### Changed
 
+- `client.UploadMetrics` in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp` now returns error shared by collector instead of generic retry-able failure error for retry-able applicable code. (#5541)
 - `Tracer.Start` in `go.opentelemetry.io/otel/trace/noop` no longer allocates a span for empty span context. (#5457)
 - Upgrade `go.opentelemetry.io/otel/semconv/v1.25.0` to `go.opentelemetry.io/otel/semconv/v1.26.0` in `go.opentelemetry.io/otel/example/otel-collector`. (#5490)
 - Upgrade `go.opentelemetry.io/otel/semconv/v1.25.0` to `go.opentelemetry.io/otel/semconv/v1.26.0` in `go.opentelemetry.io/otel/example/zipkin`. (#5490)

From 17cc1e8a6ef1d3c667f409baa83acde93f6b7f4f Mon Sep 17 00:00:00 2001
From: Preeti Dewani <preetidewani1990@gmail.com>
Date: Wed, 26 Jun 2024 15:28:17 +0530
Subject: [PATCH 03/11] parse-errormsgs-in-retry-able-statuscodes: 1. Add
 capability to pass error message to responseerror so that it can capture
 error message and also retry. 2. revert the removal of status code from
 httpresponse

---
 CHANGELOG.md                                  |  2 +-
 .../internal/otest/collector.go               |  2 +-
 .../otlp/otlpmetric/otlpmetrichttp/client.go  | 38 ++++++++++++-------
 .../otlpmetric/otlpmetrichttp/client_test.go  | 26 +++++--------
 .../internal/otest/collector.go               |  2 +-
 .../otlp/otlpmetric/otest/collector.go.tmpl   |  2 +-
 6 files changed, 37 insertions(+), 35 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8975b4c0ba8..c3953f26f3e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -20,7 +20,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ### Changed
 
-- `client.UploadMetrics` in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp` now returns error shared by collector instead of generic retry-able failure error for retry-able applicable code. (#5541)
+- Pass the underlying error rather than a generic retry-able failure in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#5541)
 - `Tracer.Start` in `go.opentelemetry.io/otel/trace/noop` no longer allocates a span for empty span context. (#5457)
 - Upgrade `go.opentelemetry.io/otel/semconv/v1.25.0` to `go.opentelemetry.io/otel/semconv/v1.26.0` in `go.opentelemetry.io/otel/example/otel-collector`. (#5490)
 - Upgrade `go.opentelemetry.io/otel/semconv/v1.25.0` to `go.opentelemetry.io/otel/semconv/v1.26.0` in `go.opentelemetry.io/otel/example/zipkin`. (#5490)
diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/otest/collector.go b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/otest/collector.go
index 6125b8cf50e..6eea8d39a75 100644
--- a/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/otest/collector.go
+++ b/exporters/otlp/otlpmetric/otlpmetricgrpc/internal/otest/collector.go
@@ -177,7 +177,7 @@ type HTTPResponseError struct {
 }
 
 func (e *HTTPResponseError) Error() string {
-	return e.Err.Error()
+	return fmt.Sprintf("%d: %s", e.Status, e.Err)
 }
 
 func (e *HTTPResponseError) Unwrap() error { return e.Err }
diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
index 2c9a3baee91..ef095a5f3df 100644
--- a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
+++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
@@ -20,14 +20,12 @@ import (
 
 	"google.golang.org/protobuf/proto"
 
-	colmetricpb "go.opentelemetry.io/proto/otlp/collector/metrics/v1"
-	metricpb "go.opentelemetry.io/proto/otlp/metrics/v1"
-
 	"go.opentelemetry.io/otel"
-
 	"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 {
@@ -149,7 +147,7 @@ func (c *client) UploadMetrics(ctx context.Context, protoMetrics *metricpb.Resou
 		resp, err := c.httpClient.Do(request.Request)
 		var urlErr *url.Error
 		if errors.As(err, &urlErr) && urlErr.Temporary() {
-			return newResponseError(http.Header{})
+			return newResponseError(http.Header{}, err.Error())
 		}
 		if err != nil {
 			return err
@@ -190,21 +188,24 @@ func (c *client) UploadMetrics(ctx context.Context, protoMetrics *metricpb.Resou
 			sc == http.StatusServiceUnavailable,
 			sc == http.StatusGatewayTimeout:
 			// Retry-able failure.
-			rErr = newResponseError(resp.Header)
+			rErr = newResponseError(resp.Header, "")
 
-			// 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.
+			// server may return a message with the response
+			// body, so we read it to include in the error
+			// message to be returned. It will help in
+			// debugging the actual issue.
 			var respData bytes.Buffer
 			if _, err := io.Copy(&respData, resp.Body); err != nil {
 				return err
 			}
 
+			// overwrite the error message with the response body
+			// if it is not empty
 			respStr := strings.TrimSpace(respData.String())
 			if respStr != "" {
-				rErr = errors.New(respStr)
+				// pass the error message along with retry-able error,
+				// so that it can be retried and also passes the message
+				rErr = newResponseError(resp.Header, respStr)
 			}
 
 		default:
@@ -282,21 +283,30 @@ func (r *request) reset(ctx context.Context) {
 // retryableError represents a request failure that can be retried.
 type retryableError struct {
 	throttle int64
+	errMsg   string
 }
 
 // newResponseError returns a retryableError and will extract any explicit
-// throttle delay contained in headers.
-func newResponseError(header http.Header) error {
+// throttle delay contained in headers and if there is message in the response
+// body, it will be used as the error message. If errMsg is not empty, it will
+// be used as the error message instead of the standard "retry-able" failure.
+func newResponseError(header http.Header, errMsg string) error {
 	var rErr retryableError
 	if v := header.Get("Retry-After"); v != "" {
 		if t, err := strconv.ParseInt(v, 10, 64); err == nil {
 			rErr.throttle = t
 		}
 	}
+
+	rErr.errMsg = errMsg
 	return rErr
 }
 
 func (e retryableError) Error() string {
+	if e.errMsg != "" {
+		return e.errMsg
+	}
+
 	return "retry-able request failure"
 }
 
diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
index 3dcd2301369..c4f7577c220 100644
--- a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
+++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
@@ -17,13 +17,11 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	mpb "go.opentelemetry.io/proto/otlp/metrics/v1"
-
-	"go.opentelemetry.io/otel/sdk/metric"
-	"go.opentelemetry.io/otel/sdk/metric/metricdata"
-
 	"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"
 )
 
 type clientShim struct {
@@ -189,26 +187,21 @@ func TestConfig(t *testing.T) {
 		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)) })
+		t.Cleanup(func() { require.NoError(t, exp.Shutdown(ctx))})
 		assert.NoError(t, exp.Export(ctx, &metricdata.ResourceMetrics{}), "failed retry")
 		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,
+			Status: http.StatusTooManyRequests,
 			Err:    exporterErr,
-			Header: header,
 		}}
-
 		exp, coll := factoryFunc("", rCh, WithRetry(RetryConfig{
-			Enabled:         true,
-			InitialInterval: time.Nanosecond,
-			MaxInterval:     time.Millisecond,
-			MaxElapsedTime:  time.Minute,
+			Enabled: false,
 		}))
 		ctx := context.Background()
 		t.Cleanup(func() { require.NoError(t, coll.Shutdown(ctx)) })
@@ -216,9 +209,8 @@ func TestConfig(t *testing.T) {
 		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")
+		err := exp.Export(ctx, &metricdata.ResourceMetrics{})
+		assert.EqualError(t, err, fmt.Sprintf("failed to upload metrics: %d: %v", http.StatusTooManyRequests, exporterErr))
 	})
 
 	t.Run("WithURLPath", func(t *testing.T) {
diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/otest/collector.go b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/otest/collector.go
index 911fc69ef67..178dcde6c36 100644
--- a/exporters/otlp/otlpmetric/otlpmetrichttp/internal/otest/collector.go
+++ b/exporters/otlp/otlpmetric/otlpmetrichttp/internal/otest/collector.go
@@ -177,7 +177,7 @@ type HTTPResponseError struct {
 }
 
 func (e *HTTPResponseError) Error() string {
-	return e.Err.Error()
+	return fmt.Sprintf("%d: %s", e.Status, e.Err)
 }
 
 func (e *HTTPResponseError) Unwrap() error { return e.Err }
diff --git a/internal/shared/otlp/otlpmetric/otest/collector.go.tmpl b/internal/shared/otlp/otlpmetric/otest/collector.go.tmpl
index 83137aac764..265574c1733 100644
--- a/internal/shared/otlp/otlpmetric/otest/collector.go.tmpl
+++ b/internal/shared/otlp/otlpmetric/otest/collector.go.tmpl
@@ -177,7 +177,7 @@ type HTTPResponseError struct {
 }
 
 func (e *HTTPResponseError) Error() string {
-        return e.Err.Error()
+        return fmt.Sprintf("%d: %s", e.Status, e.Err)
 }
 
 func (e *HTTPResponseError) Unwrap() error { return e.Err }

From a2c49e632ff62f0ad164d1d2e8ac12513f7954b3 Mon Sep 17 00:00:00 2001
From: Preeti Dewani <preetidewani1990@gmail.com>
Date: Fri, 28 Jun 2024 09:37:05 +0530
Subject: [PATCH 04/11] parse-errormsgs-in-retry-able-statuscodes: fix lint
 issues

---
 exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go | 4 +---
 internal/shared/otlp/otlpmetric/otest/collector.go.tmpl | 2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
index c4f7577c220..aa000b1eebc 100644
--- a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
+++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
@@ -187,10 +187,9 @@ func TestConfig(t *testing.T) {
 		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))})
+		t.Cleanup(func() { require.NoError(t, exp.Shutdown(ctx)) })
 		assert.NoError(t, exp.Export(ctx, &metricdata.ResourceMetrics{}), "failed retry")
 		assert.Len(t, rCh, 0, "failed HTTP responses did not occur")
-
 	})
 
 	t.Run("WithRetryAndExporterErr", func(t *testing.T) {
@@ -208,7 +207,6 @@ func TestConfig(t *testing.T) {
 		// 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)) })
-
 		err := exp.Export(ctx, &metricdata.ResourceMetrics{})
 		assert.EqualError(t, err, fmt.Sprintf("failed to upload metrics: %d: %v", http.StatusTooManyRequests, exporterErr))
 	})
diff --git a/internal/shared/otlp/otlpmetric/otest/collector.go.tmpl b/internal/shared/otlp/otlpmetric/otest/collector.go.tmpl
index 265574c1733..22ffd923790 100644
--- a/internal/shared/otlp/otlpmetric/otest/collector.go.tmpl
+++ b/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 fmt.Sprintf("%d: %s", e.Status, e.Err)
 }
 
 func (e *HTTPResponseError) Unwrap() error { return e.Err }

From abe63e92fa63eaeecc74cfbdd4f495dc553ae8ca Mon Sep 17 00:00:00 2001
From: Preeti Dewani <preetidewani1990@gmail.com>
Date: Thu, 4 Jul 2024 14:40:59 +0530
Subject: [PATCH 05/11] parse-errormsgs-in-retry-able-statuscodes: 1. Include
 msg parsing from response body in log and traces. 2. Parse the response body
 using status.FromError to extract relevant information

---
 CHANGELOG.md                                  |  2 +-
 exporters/otlp/otlplog/otlploghttp/client.go  | 44 +++++++++++++---
 exporters/otlp/otlplog/otlploghttp/go.mod     |  2 +-
 .../otlp/otlpmetric/otlpmetrichttp/client.go  | 14 +++++-
 .../otlp/otlptrace/otlptracehttp/client.go    | 50 ++++++++++++++++---
 .../otlptrace/otlptracehttp/client_test.go    |  2 +-
 6 files changed, 96 insertions(+), 18 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c3953f26f3e..e9d03b0be7a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -20,7 +20,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ### Changed
 
-- Pass the underlying error rather than a generic retry-able failure in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#5541)
 - `Tracer.Start` in `go.opentelemetry.io/otel/trace/noop` no longer allocates a span for empty span context. (#5457)
 - Upgrade `go.opentelemetry.io/otel/semconv/v1.25.0` to `go.opentelemetry.io/otel/semconv/v1.26.0` in `go.opentelemetry.io/otel/example/otel-collector`. (#5490)
 - Upgrade `go.opentelemetry.io/otel/semconv/v1.25.0` to `go.opentelemetry.io/otel/semconv/v1.26.0` in `go.opentelemetry.io/otel/example/zipkin`. (#5490)
@@ -43,6 +42,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Fix panic in baggage creation when a member contains 0x80 char in key or value. (#5494)
 - Correct comments for the priority of the `WithEndpoint` and `WithEndpointURL` options and their coresponding environment variables in  in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc`. (#5508)
 - Fix stale timestamps reported by the lastvalue aggregation. (#5517)
+- Pass the underlying error rather than a generic retry-able failure in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#5541)
 
 ## [1.27.0/0.49.0/0.3.0] 2024-05-21
 
diff --git a/exporters/otlp/otlplog/otlploghttp/client.go b/exporters/otlp/otlplog/otlploghttp/client.go
index 09a950838c4..64f7a1d9555 100644
--- a/exporters/otlp/otlplog/otlploghttp/client.go
+++ b/exporters/otlp/otlplog/otlploghttp/client.go
@@ -14,9 +14,11 @@ import (
 	"net/http"
 	"net/url"
 	"strconv"
+	"strings"
 	"sync"
 	"time"
 
+	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/proto"
 
 	"go.opentelemetry.io/otel"
@@ -143,7 +145,7 @@ func (c *httpClient) uploadLogs(ctx context.Context, data []*logpb.ResourceLogs)
 		resp, err := c.client.Do(request.Request)
 		var urlErr *url.Error
 		if errors.As(err, &urlErr) && urlErr.Temporary() {
-			return newResponseError(http.Header{})
+			return newResponseError(http.Header{}, err.Error())
 		}
 		if err != nil {
 			return err
@@ -184,13 +186,26 @@ func (c *httpClient) uploadLogs(ctx context.Context, data []*logpb.ResourceLogs)
 			sc == http.StatusServiceUnavailable,
 			sc == http.StatusGatewayTimeout:
 			// Retry-able failure.
-			rErr = newResponseError(resp.Header)
+			rErr = newResponseError(resp.Header, "")
 
-			// Going to retry, drain the body to reuse the connection.
-			if _, err := io.Copy(io.Discard, resp.Body); err != nil {
+			// server may return a message with the response
+			// body, so we read it to include in the error
+			// message to be returned. It will help in
+			// debugging the actual issue.
+			var respData bytes.Buffer
+			if _, err := io.Copy(&respData, resp.Body); err != nil {
 				_ = resp.Body.Close()
 				return err
 			}
+
+			// overwrite the error message with the response body
+			// if it is not empty
+			respStr := strings.TrimSpace(respData.String())
+			if respStr != "" {
+				// pass the error message along with retry-able error,
+				// so that it can be retried and also passes the message
+				rErr = newResponseError(resp.Header, respStr)
+			}
 		default:
 			rErr = fmt.Errorf("failed to send logs to %s: %s", request.URL, resp.Status)
 		}
@@ -266,21 +281,38 @@ func (r *request) reset(ctx context.Context) {
 // retryableError represents a request failure that can be retried.
 type retryableError struct {
 	throttle int64
+	errMsg   string
 }
 
 // newResponseError returns a retryableError and will extract any explicit
-// throttle delay contained in headers.
-func newResponseError(header http.Header) error {
+// throttle delay contained in headers and if there is message in the response
+// body, it will be used as the error message. If errMsg is not empty, it will
+// be used as the error message instead of the standard "retry-able" failure.
+func newResponseError(header http.Header, body string) error {
 	var rErr retryableError
 	if v := header.Get("Retry-After"); v != "" {
 		if t, err := strconv.ParseInt(v, 10, 64); err == nil {
 			rErr.throttle = t
 		}
 	}
+
+	// Extract the error message from the response body.
+	st, ok := status.FromError(errors.New(body))
+	rErr.errMsg = fmt.Sprintf("rpc error: code = %s desc = %s", st.Code(), st.Message())
+
+	// if response body is not in expected format
+	if !ok {
+		rErr.errMsg = body
+	}
+
 	return rErr
 }
 
 func (e retryableError) Error() string {
+	if e.errMsg != "" {
+		return e.errMsg
+	}
+
 	return "retry-able request failure"
 }
 
diff --git a/exporters/otlp/otlplog/otlploghttp/go.mod b/exporters/otlp/otlplog/otlploghttp/go.mod
index 72acd1b7bd0..84ef83700cc 100644
--- a/exporters/otlp/otlplog/otlploghttp/go.mod
+++ b/exporters/otlp/otlplog/otlploghttp/go.mod
@@ -12,6 +12,7 @@ require (
 	go.opentelemetry.io/otel/sdk/log v0.3.0
 	go.opentelemetry.io/otel/trace v1.27.0
 	go.opentelemetry.io/proto/otlp v1.3.1
+	google.golang.org/grpc v1.64.0
 	google.golang.org/protobuf v1.34.2
 )
 
@@ -29,7 +30,6 @@ require (
 	golang.org/x/text v0.16.0 // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20240617180043-68d350f18fd4 // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20240617180043-68d350f18fd4 // indirect
-	google.golang.org/grpc v1.64.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
 
diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
index ef095a5f3df..627d639d701 100644
--- a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
+++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
@@ -18,6 +18,7 @@ import (
 	"sync"
 	"time"
 
+	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/proto"
 
 	"go.opentelemetry.io/otel"
@@ -196,6 +197,7 @@ func (c *client) UploadMetrics(ctx context.Context, protoMetrics *metricpb.Resou
 			// debugging the actual issue.
 			var respData bytes.Buffer
 			if _, err := io.Copy(&respData, resp.Body); err != nil {
+				_ = resp.Body.Close()
 				return err
 			}
 
@@ -290,7 +292,7 @@ type retryableError struct {
 // throttle delay contained in headers and if there is message in the response
 // body, it will be used as the error message. If errMsg is not empty, it will
 // be used as the error message instead of the standard "retry-able" failure.
-func newResponseError(header http.Header, errMsg string) error {
+func newResponseError(header http.Header, body string) error {
 	var rErr retryableError
 	if v := header.Get("Retry-After"); v != "" {
 		if t, err := strconv.ParseInt(v, 10, 64); err == nil {
@@ -298,7 +300,15 @@ func newResponseError(header http.Header, errMsg string) error {
 		}
 	}
 
-	rErr.errMsg = errMsg
+	// Extract the error message from the response body.
+	st, ok := status.FromError(errors.New(body))
+	rErr.errMsg = fmt.Sprintf("rpc error: code = %s desc = %s", st.Code(), st.Message())
+
+	// if response body is not in expected format
+	if !ok {
+		rErr.errMsg = body
+	}
+
 	return rErr
 }
 
diff --git a/exporters/otlp/otlptrace/otlptracehttp/client.go b/exporters/otlp/otlptrace/otlptracehttp/client.go
index 1c487a09630..7277c4ca49d 100644
--- a/exporters/otlp/otlptrace/otlptracehttp/client.go
+++ b/exporters/otlp/otlptrace/otlptracehttp/client.go
@@ -14,9 +14,11 @@ import (
 	"net/http"
 	"net/url"
 	"strconv"
+	"strings"
 	"sync"
 	"time"
 
+	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/proto"
 
 	"go.opentelemetry.io/otel"
@@ -151,7 +153,7 @@ func (d *client) UploadTraces(ctx context.Context, protoSpans []*tracepb.Resourc
 		resp, err := d.client.Do(request.Request)
 		var urlErr *url.Error
 		if errors.As(err, &urlErr) && urlErr.Temporary() {
-			return newResponseError(http.Header{})
+			return newResponseError(http.Header{}, err.Error())
 		}
 		if err != nil {
 			return err
@@ -198,11 +200,28 @@ func (d *client) UploadTraces(ctx context.Context, protoSpans []*tracepb.Resourc
 			sc == http.StatusBadGateway,
 			sc == http.StatusServiceUnavailable,
 			sc == http.StatusGatewayTimeout:
-			// Retry-able failures.  Drain the body to reuse the connection.
-			if _, err := io.Copy(io.Discard, resp.Body); err != nil {
-				otel.Handle(err)
+			// Retry-able failures.
+			rErr := newResponseError(resp.Header, "")
+
+			// server may return a message with the response
+			// body, so we read it to include in the error
+			// message to be returned. It will help in
+			// debugging the actual issue.
+			var respData bytes.Buffer
+			if _, err := io.Copy(&respData, resp.Body); err != nil {
+				_ = resp.Body.Close()
+				return err
 			}
-			return newResponseError(resp.Header)
+
+			// overwrite the error message with the response body
+			// if it is not empty
+			respStr := strings.TrimSpace(respData.String())
+			if respStr != "" {
+				// pass the error message along with retry-able error,
+				// so that it can be retried and also passes the message
+				rErr = newResponseError(resp.Header, respStr)
+			}
+			return rErr
 		default:
 			return fmt.Errorf("failed to send to %s: %s", request.URL, resp.Status)
 		}
@@ -291,21 +310,38 @@ func (r *request) reset(ctx context.Context) {
 // retryableError represents a request failure that can be retried.
 type retryableError struct {
 	throttle int64
+	errMsg   string
 }
 
 // newResponseError returns a retryableError and will extract any explicit
-// throttle delay contained in headers.
-func newResponseError(header http.Header) error {
+// throttle delay contained in headers and if there is message in the response
+// body, it will be used as the error message. If errMsg is not empty, it will
+// be used as the error message instead of the standard "retry-able" failure.
+func newResponseError(header http.Header, body string) error {
 	var rErr retryableError
 	if s, ok := header["Retry-After"]; ok {
 		if t, err := strconv.ParseInt(s[0], 10, 64); err == nil {
 			rErr.throttle = t
 		}
 	}
+
+	// Extract the error message from the response body.
+	st, ok := status.FromError(errors.New(body))
+	rErr.errMsg = fmt.Sprintf("rpc error: code = %s desc = %s", st.Code(), st.Message())
+
+	// if response body is not in expected format
+	if !ok {
+		rErr.errMsg = body
+	}
+
 	return rErr
 }
 
 func (e retryableError) Error() string {
+	if e.errMsg != "" {
+		return e.errMsg
+	}
+
 	return "retry-able request failure"
 }
 
diff --git a/exporters/otlp/otlptrace/otlptracehttp/client_test.go b/exporters/otlp/otlptrace/otlptracehttp/client_test.go
index 0494f82c9a7..2e260745e4f 100644
--- a/exporters/otlp/otlptrace/otlptracehttp/client_test.go
+++ b/exporters/otlp/otlptrace/otlptracehttp/client_test.go
@@ -238,7 +238,7 @@ func TestTimeout(t *testing.T) {
 		assert.NoError(t, exporter.Shutdown(ctx))
 	}()
 	err = exporter.ExportSpans(ctx, otlptracetest.SingleReadOnlySpan())
-	assert.ErrorContains(t, err, "retry-able request failure")
+	assert.ErrorContains(t, err, "context deadline exceeded")
 }
 
 func TestNoRetry(t *testing.T) {

From 788473120af099fa5b8687f946e9eb187e17a061 Mon Sep 17 00:00:00 2001
From: Preeti Dewani <preetidewani1990@gmail.com>
Date: Thu, 4 Jul 2024 15:13:41 +0530
Subject: [PATCH 06/11] parse-errormsgs-in-retry-able-statuscodes: change
 errors.New to fmt.Errorf to maintain consistency

---
 exporters/otlp/otlplog/otlploghttp/client.go       | 2 +-
 exporters/otlp/otlpmetric/otlpmetrichttp/client.go | 2 +-
 exporters/otlp/otlptrace/otlptracehttp/client.go   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/exporters/otlp/otlplog/otlploghttp/client.go b/exporters/otlp/otlplog/otlploghttp/client.go
index 64f7a1d9555..df1b22a76e5 100644
--- a/exporters/otlp/otlplog/otlploghttp/client.go
+++ b/exporters/otlp/otlplog/otlploghttp/client.go
@@ -297,7 +297,7 @@ func newResponseError(header http.Header, body string) error {
 	}
 
 	// Extract the error message from the response body.
-	st, ok := status.FromError(errors.New(body))
+	st, ok := status.FromError(fmt.Errorf(body))
 	rErr.errMsg = fmt.Sprintf("rpc error: code = %s desc = %s", st.Code(), st.Message())
 
 	// if response body is not in expected format
diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
index 627d639d701..511b1a190da 100644
--- a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
+++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
@@ -301,7 +301,7 @@ func newResponseError(header http.Header, body string) error {
 	}
 
 	// Extract the error message from the response body.
-	st, ok := status.FromError(errors.New(body))
+	st, ok := status.FromError(fmt.Errorf(body))
 	rErr.errMsg = fmt.Sprintf("rpc error: code = %s desc = %s", st.Code(), st.Message())
 
 	// if response body is not in expected format
diff --git a/exporters/otlp/otlptrace/otlptracehttp/client.go b/exporters/otlp/otlptrace/otlptracehttp/client.go
index 7277c4ca49d..c363b2b14c2 100644
--- a/exporters/otlp/otlptrace/otlptracehttp/client.go
+++ b/exporters/otlp/otlptrace/otlptracehttp/client.go
@@ -326,7 +326,7 @@ func newResponseError(header http.Header, body string) error {
 	}
 
 	// Extract the error message from the response body.
-	st, ok := status.FromError(errors.New(body))
+	st, ok := status.FromError(fmt.Errorf(body))
 	rErr.errMsg = fmt.Sprintf("rpc error: code = %s desc = %s", st.Code(), st.Message())
 
 	// if response body is not in expected format

From 2709f7cafa169335c35b0cf5079a7da4f2e2f92a Mon Sep 17 00:00:00 2001
From: Preeti Dewani <preetidewani1990@gmail.com>
Date: Thu, 4 Jul 2024 15:42:40 +0530
Subject: [PATCH 07/11] parse-errormsgs-in-retry-able-statuscodes: 1. Correct
 changlog and comments. 2. Add test case for logs

---
 CHANGELOG.md                                  |  2 +-
 exporters/otlp/otlplog/otlploghttp/client.go  | 14 ++++++--------
 .../otlp/otlplog/otlploghttp/client_test.go   | 19 +++++++++++++++++++
 .../otlp/otlpmetric/otlpmetrichttp/client.go  | 14 ++++++--------
 .../otlp/otlptrace/otlptracehttp/client.go    | 14 ++++++--------
 5 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e9d03b0be7a..29ed7280b6d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -42,7 +42,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Fix panic in baggage creation when a member contains 0x80 char in key or value. (#5494)
 - Correct comments for the priority of the `WithEndpoint` and `WithEndpointURL` options and their coresponding environment variables in  in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc`. (#5508)
 - Fix stale timestamps reported by the lastvalue aggregation. (#5517)
-- Pass the underlying error rather than a generic retry-able failure in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#5541)
+- Pass the underlying error rather than a generic retry-able failure in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`, `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp` and `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#5541)
 
 ## [1.27.0/0.49.0/0.3.0] 2024-05-21
 
diff --git a/exporters/otlp/otlplog/otlploghttp/client.go b/exporters/otlp/otlplog/otlploghttp/client.go
index df1b22a76e5..640411f4b5b 100644
--- a/exporters/otlp/otlplog/otlploghttp/client.go
+++ b/exporters/otlp/otlplog/otlploghttp/client.go
@@ -279,6 +279,8 @@ func (r *request) reset(ctx context.Context) {
 }
 
 // retryableError represents a request failure that can be retried.
+//
+// If the `errMsg` attribute is not empty, it will be used as the error message.
 type retryableError struct {
 	throttle int64
 	errMsg   string
@@ -286,8 +288,7 @@ type retryableError struct {
 
 // newResponseError returns a retryableError and will extract any explicit
 // throttle delay contained in headers and if there is message in the response
-// body, it will be used as the error message. If errMsg is not empty, it will
-// be used as the error message instead of the standard "retry-able" failure.
+// body, it will be used as the error message.
 func newResponseError(header http.Header, body string) error {
 	var rErr retryableError
 	if v := header.Get("Retry-After"); v != "" {
@@ -296,13 +297,10 @@ func newResponseError(header http.Header, body string) error {
 		}
 	}
 
+	rErr.errMsg = body
 	// Extract the error message from the response body.
-	st, ok := status.FromError(fmt.Errorf(body))
-	rErr.errMsg = fmt.Sprintf("rpc error: code = %s desc = %s", st.Code(), st.Message())
-
-	// if response body is not in expected format
-	if !ok {
-		rErr.errMsg = body
+	if st, ok := status.FromError(fmt.Errorf(body)); ok {
+		rErr.errMsg = fmt.Sprintf("rpc error: code = %s desc = %s", st.Code(), st.Message())
 	}
 
 	return rErr
diff --git a/exporters/otlp/otlplog/otlploghttp/client_test.go b/exporters/otlp/otlplog/otlploghttp/client_test.go
index 906a0d4e4ad..373f0c12e8b 100644
--- a/exporters/otlp/otlplog/otlploghttp/client_test.go
+++ b/exporters/otlp/otlplog/otlploghttp/client_test.go
@@ -700,6 +700,25 @@ 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 exportResult, 1)
+		rCh <- exportResult{Err: &httpResponseError{
+			Status: http.StatusTooManyRequests,
+			Err:    exporterErr,
+		}}
+		exp, coll := factoryFunc("", rCh, WithRetry(RetryConfig{
+			Enabled: false,
+		}))
+		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)) })
+		err := exp.Export(ctx, make([]log.Record, 1))
+		assert.EqualError(t, err, fmt.Sprintf("%d: %v", http.StatusTooManyRequests, exporterErr))
+	})
+
 	t.Run("WithURLPath", func(t *testing.T) {
 		path := "/prefix/v2/logs"
 		ePt := fmt.Sprintf("http://localhost:0%s", path)
diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
index 511b1a190da..c38a4d95f3b 100644
--- a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
+++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
@@ -283,6 +283,8 @@ func (r *request) reset(ctx context.Context) {
 }
 
 // retryableError represents a request failure that can be retried.
+//
+// If the `errMsg` attribute is not empty, it will be used as the error message.
 type retryableError struct {
 	throttle int64
 	errMsg   string
@@ -290,8 +292,7 @@ type retryableError struct {
 
 // newResponseError returns a retryableError and will extract any explicit
 // throttle delay contained in headers and if there is message in the response
-// body, it will be used as the error message. If errMsg is not empty, it will
-// be used as the error message instead of the standard "retry-able" failure.
+// body, it will be used as the error message.
 func newResponseError(header http.Header, body string) error {
 	var rErr retryableError
 	if v := header.Get("Retry-After"); v != "" {
@@ -300,13 +301,10 @@ func newResponseError(header http.Header, body string) error {
 		}
 	}
 
+	rErr.errMsg = body
 	// Extract the error message from the response body.
-	st, ok := status.FromError(fmt.Errorf(body))
-	rErr.errMsg = fmt.Sprintf("rpc error: code = %s desc = %s", st.Code(), st.Message())
-
-	// if response body is not in expected format
-	if !ok {
-		rErr.errMsg = body
+	if st, ok := status.FromError(fmt.Errorf(body)); ok {
+		rErr.errMsg = fmt.Sprintf("rpc error: code = %s desc = %s", st.Code(), st.Message())
 	}
 
 	return rErr
diff --git a/exporters/otlp/otlptrace/otlptracehttp/client.go b/exporters/otlp/otlptrace/otlptracehttp/client.go
index c363b2b14c2..817b659fdeb 100644
--- a/exporters/otlp/otlptrace/otlptracehttp/client.go
+++ b/exporters/otlp/otlptrace/otlptracehttp/client.go
@@ -308,6 +308,8 @@ func (r *request) reset(ctx context.Context) {
 }
 
 // retryableError represents a request failure that can be retried.
+//
+// If the `errMsg` attribute is not empty, it will be used as the error message.
 type retryableError struct {
 	throttle int64
 	errMsg   string
@@ -315,8 +317,7 @@ type retryableError struct {
 
 // newResponseError returns a retryableError and will extract any explicit
 // throttle delay contained in headers and if there is message in the response
-// body, it will be used as the error message. If errMsg is not empty, it will
-// be used as the error message instead of the standard "retry-able" failure.
+// body, it will be used as the error message.
 func newResponseError(header http.Header, body string) error {
 	var rErr retryableError
 	if s, ok := header["Retry-After"]; ok {
@@ -325,13 +326,10 @@ func newResponseError(header http.Header, body string) error {
 		}
 	}
 
+	rErr.errMsg = body
 	// Extract the error message from the response body.
-	st, ok := status.FromError(fmt.Errorf(body))
-	rErr.errMsg = fmt.Sprintf("rpc error: code = %s desc = %s", st.Code(), st.Message())
-
-	// if response body is not in expected format
-	if !ok {
-		rErr.errMsg = body
+	if st, ok := status.FromError(fmt.Errorf(body)); ok {
+		rErr.errMsg = fmt.Sprintf("rpc error: code = %s desc = %s", st.Code(), st.Message())
 	}
 
 	return rErr

From d1a5da655ee6bffcce107f24c6e15d4d804826a2 Mon Sep 17 00:00:00 2001
From: Preeti Dewani <preetidewani1990@gmail.com>
Date: Thu, 11 Jul 2024 22:26:17 +0530
Subject: [PATCH 08/11] parse-errormsgs-in-retry-able-statuscodes: 1. Provide
 Unwrap and As method for retryableError type. 2. Remove error handling via
 grpc status as it needs specific GRPCStatus method implementation which
 depends on collector behaviour to communicate the right code

---
 exporters/otlp/otlplog/otlploghttp/client.go  | 36 ++++++++++++-------
 .../otlp/otlplog/otlploghttp/client_test.go   | 10 +++++-
 exporters/otlp/otlplog/otlploghttp/go.mod     |  2 +-
 .../otlp/otlpmetric/otlpmetrichttp/client.go  | 36 ++++++++++++-------
 .../otlpmetric/otlpmetrichttp/client_test.go  | 10 +++++-
 .../otlp/otlptrace/otlptracehttp/client.go    | 36 ++++++++++++-------
 6 files changed, 88 insertions(+), 42 deletions(-)

diff --git a/exporters/otlp/otlplog/otlploghttp/client.go b/exporters/otlp/otlplog/otlploghttp/client.go
index 78b1efda110..794bba3fc68 100644
--- a/exporters/otlp/otlplog/otlploghttp/client.go
+++ b/exporters/otlp/otlplog/otlploghttp/client.go
@@ -18,7 +18,6 @@ import (
 	"sync"
 	"time"
 
-	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/proto"
 
 	"go.opentelemetry.io/otel"
@@ -279,16 +278,14 @@ func (r *request) reset(ctx context.Context) {
 }
 
 // retryableError represents a request failure that can be retried.
-//
-// If the `errMsg` attribute is not empty, it will be used as the error message.
 type retryableError struct {
 	throttle int64
-	errMsg   string
+	err      error
 }
 
 // newResponseError returns a retryableError and will extract any explicit
 // throttle delay contained in headers and if there is message in the response
-// body, it will be used as the error message.
+// body, it will be passed as error.
 func newResponseError(header http.Header, body string) error {
 	var rErr retryableError
 	if v := header.Get("Retry-After"); v != "" {
@@ -297,23 +294,36 @@ func newResponseError(header http.Header, body string) error {
 		}
 	}
 
-	rErr.errMsg = body
-	// Extract the error message from the response body.
-	if st, ok := status.FromError(fmt.Errorf(body)); ok {
-		rErr.errMsg = fmt.Sprintf("rpc error: code = %s desc = %s", st.Code(), st.Message())
-	}
-
+	rErr.err = fmt.Errorf("retry-able request failure: %s", body)
 	return rErr
 }
 
 func (e retryableError) Error() string {
-	if e.errMsg != "" {
-		return e.errMsg
+	if e.err != nil {
+		return e.err.Error()
 	}
 
 	return "retry-able request failure"
 }
 
+func (e retryableError) Unwrap() error {
+	return e.err
+}
+
+func (e retryableError) As(target interface{}) bool {
+	if e.err == nil {
+		return false
+	}
+
+	switch v := target.(type) {
+	case **retryableError:
+		*v = &e
+		return true
+	default:
+		return false
+	}
+}
+
 // evaluate returns if err is retry-able. If it is and it includes an explicit
 // throttling delay, that delay is also returned.
 func evaluate(err error) (bool, time.Duration) {
diff --git a/exporters/otlp/otlplog/otlploghttp/client_test.go b/exporters/otlp/otlplog/otlploghttp/client_test.go
index 373f0c12e8b..6a256a0d9d3 100644
--- a/exporters/otlp/otlplog/otlploghttp/client_test.go
+++ b/exporters/otlp/otlplog/otlploghttp/client_test.go
@@ -716,7 +716,15 @@ func TestConfig(t *testing.T) {
 		t.Cleanup(func() { close(rCh) })
 		t.Cleanup(func() { require.NoError(t, exp.Shutdown(ctx)) })
 		err := exp.Export(ctx, make([]log.Record, 1))
-		assert.EqualError(t, err, fmt.Sprintf("%d: %v", http.StatusTooManyRequests, exporterErr))
+		assert.Error(t, err, exporterErr)
+
+		var retryErr *retryableError
+		assert.True(t, errors.As(err, &retryErr))
+		assert.Equal(t, fmt.Sprintf("retry-able request failure: 429: %v", exporterErr), retryErr.Unwrap().Error())
+
+		var newErr *retryableError
+		assert.True(t, retryErr.As(&newErr))
+		assert.Equal(t, newErr.Unwrap().Error(), retryErr.Unwrap().Error())
 	})
 
 	t.Run("WithURLPath", func(t *testing.T) {
diff --git a/exporters/otlp/otlplog/otlploghttp/go.mod b/exporters/otlp/otlplog/otlploghttp/go.mod
index 418555a8eb4..53e8f02f2aa 100644
--- a/exporters/otlp/otlplog/otlploghttp/go.mod
+++ b/exporters/otlp/otlplog/otlploghttp/go.mod
@@ -12,7 +12,6 @@ require (
 	go.opentelemetry.io/otel/sdk/log v0.4.0
 	go.opentelemetry.io/otel/trace v1.28.0
 	go.opentelemetry.io/proto/otlp v1.3.1
-	google.golang.org/grpc v1.65.0
 	google.golang.org/protobuf v1.34.2
 )
 
@@ -31,6 +30,7 @@ require (
 	golang.org/x/text v0.16.0 // indirect
 	google.golang.org/genproto/googleapis/api v0.0.0-20240709173604-40e1e62336c5 // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20240709173604-40e1e62336c5 // indirect
+	google.golang.org/grpc v1.65.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
 
diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
index a25b360f408..3d9ad0f39da 100644
--- a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
+++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
@@ -18,7 +18,6 @@ import (
 	"sync"
 	"time"
 
-	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/proto"
 
 	"go.opentelemetry.io/otel"
@@ -283,16 +282,14 @@ func (r *request) reset(ctx context.Context) {
 }
 
 // retryableError represents a request failure that can be retried.
-//
-// If the `errMsg` attribute is not empty, it will be used as the error message.
 type retryableError struct {
 	throttle int64
-	errMsg   string
+	err      error
 }
 
 // newResponseError returns a retryableError and will extract any explicit
 // throttle delay contained in headers and if there is message in the response
-// body, it will be used as the error message.
+// body, it will be passed as error.
 func newResponseError(header http.Header, body string) error {
 	var rErr retryableError
 	if v := header.Get("Retry-After"); v != "" {
@@ -301,23 +298,36 @@ func newResponseError(header http.Header, body string) error {
 		}
 	}
 
-	rErr.errMsg = body
-	// Extract the error message from the response body.
-	if st, ok := status.FromError(fmt.Errorf(body)); ok {
-		rErr.errMsg = fmt.Sprintf("rpc error: code = %s desc = %s", st.Code(), st.Message())
-	}
-
+	rErr.err = fmt.Errorf("retry-able request failure: %s", body)
 	return rErr
 }
 
 func (e retryableError) Error() string {
-	if e.errMsg != "" {
-		return e.errMsg
+	if e.err != nil {
+		return e.err.Error()
 	}
 
 	return "retry-able request failure"
 }
 
+func (e retryableError) Unwrap() error {
+	return e.err
+}
+
+func (e retryableError) As(target interface{}) bool {
+	if e.err == nil {
+		return false
+	}
+
+	switch v := target.(type) {
+	case **retryableError:
+		*v = &e
+		return true
+	default:
+		return false
+	}
+}
+
 // evaluate returns if err is retry-able. If it is and it includes an explicit
 // throttling delay, that delay is also returned.
 func evaluate(err error) (bool, time.Duration) {
diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
index aa000b1eebc..e3efe797e71 100644
--- a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
+++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
@@ -208,7 +208,15 @@ func TestConfig(t *testing.T) {
 		t.Cleanup(func() { close(rCh) })
 		t.Cleanup(func() { require.NoError(t, exp.Shutdown(ctx)) })
 		err := exp.Export(ctx, &metricdata.ResourceMetrics{})
-		assert.EqualError(t, err, fmt.Sprintf("failed to upload metrics: %d: %v", http.StatusTooManyRequests, exporterErr))
+		assert.EqualError(t, err, fmt.Sprintf("failed to upload metrics: retry-able request failure: %d: %v", http.StatusTooManyRequests, exporterErr))
+
+		var retryErr *retryableError
+		assert.True(t, errors.As(err, &retryErr))
+		assert.Equal(t, fmt.Sprintf("retry-able request failure: 429: %v", exporterErr), retryErr.Unwrap().Error())
+
+		var newErr *retryableError
+		assert.True(t, retryErr.As(&newErr))
+		assert.Equal(t, newErr.Unwrap().Error(), retryErr.Unwrap().Error())
 	})
 
 	t.Run("WithURLPath", func(t *testing.T) {
diff --git a/exporters/otlp/otlptrace/otlptracehttp/client.go b/exporters/otlp/otlptrace/otlptracehttp/client.go
index df0e524c802..a0023f63ecb 100644
--- a/exporters/otlp/otlptrace/otlptracehttp/client.go
+++ b/exporters/otlp/otlptrace/otlptracehttp/client.go
@@ -18,7 +18,6 @@ import (
 	"sync"
 	"time"
 
-	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/proto"
 
 	"go.opentelemetry.io/otel"
@@ -308,16 +307,14 @@ func (r *request) reset(ctx context.Context) {
 }
 
 // retryableError represents a request failure that can be retried.
-//
-// If the `errMsg` attribute is not empty, it will be used as the error message.
 type retryableError struct {
 	throttle int64
-	errMsg   string
+	err      error
 }
 
 // newResponseError returns a retryableError and will extract any explicit
 // throttle delay contained in headers and if there is message in the response
-// body, it will be used as the error message.
+// body, it will be passed as error.
 func newResponseError(header http.Header, body string) error {
 	var rErr retryableError
 	if s, ok := header["Retry-After"]; ok {
@@ -326,23 +323,36 @@ func newResponseError(header http.Header, body string) error {
 		}
 	}
 
-	rErr.errMsg = body
-	// Extract the error message from the response body.
-	if st, ok := status.FromError(fmt.Errorf(body)); ok {
-		rErr.errMsg = fmt.Sprintf("rpc error: code = %s desc = %s", st.Code(), st.Message())
-	}
-
+	rErr.err = fmt.Errorf("retry-able request failure: %s", body)
 	return rErr
 }
 
 func (e retryableError) Error() string {
-	if e.errMsg != "" {
-		return e.errMsg
+	if e.err != nil {
+		return e.err.Error()
 	}
 
 	return "retry-able request failure"
 }
 
+func (e retryableError) Unwrap() error {
+	return e.err
+}
+
+func (e retryableError) As(target interface{}) bool {
+	if e.err == nil {
+		return false
+	}
+
+	switch v := target.(type) {
+	case **retryableError:
+		*v = &e
+		return true
+	default:
+		return false
+	}
+}
+
 // evaluate returns if err is retry-able. If it is and it includes an explicit
 // throttling delay, that delay is also returned.
 func evaluate(err error) (bool, time.Duration) {

From 999b4d53136b4110e6776af3f0a76a42ad1727e2 Mon Sep 17 00:00:00 2001
From: Preeti Dewani <preetidewani1990@gmail.com>
Date: Fri, 12 Jul 2024 01:01:59 +0530
Subject: [PATCH 09/11] parse-errormsgs-in-retry-able-statuscodes: 1. Don't
 pre-render the error message before it is required. 2. Send error from it's
 source instead of reconstructing it in custom error type

---
 exporters/otlp/otlplog/otlploghttp/client.go  | 19 +++++++++---------
 .../otlp/otlplog/otlploghttp/client_test.go   |  2 +-
 .../otlp/otlpmetric/otlpmetrichttp/client.go  | 20 +++++++++----------
 .../otlpmetric/otlpmetrichttp/client_test.go  |  2 +-
 .../otlp/otlptrace/otlptracehttp/client.go    | 19 +++++++++---------
 5 files changed, 29 insertions(+), 33 deletions(-)

diff --git a/exporters/otlp/otlplog/otlploghttp/client.go b/exporters/otlp/otlplog/otlploghttp/client.go
index 794bba3fc68..d31c3d68f8b 100644
--- a/exporters/otlp/otlplog/otlploghttp/client.go
+++ b/exporters/otlp/otlplog/otlploghttp/client.go
@@ -144,7 +144,7 @@ func (c *httpClient) uploadLogs(ctx context.Context, data []*logpb.ResourceLogs)
 		resp, err := c.client.Do(request.Request)
 		var urlErr *url.Error
 		if errors.As(err, &urlErr) && urlErr.Temporary() {
-			return newResponseError(http.Header{}, err.Error())
+			return newResponseError(http.Header{}, err)
 		}
 		if err != nil {
 			return err
@@ -185,7 +185,7 @@ func (c *httpClient) uploadLogs(ctx context.Context, data []*logpb.ResourceLogs)
 			sc == http.StatusServiceUnavailable,
 			sc == http.StatusGatewayTimeout:
 			// Retry-able failure.
-			rErr = newResponseError(resp.Header, "")
+			rErr = newResponseError(resp.Header, nil)
 
 			// server may return a message with the response
 			// body, so we read it to include in the error
@@ -199,11 +199,10 @@ func (c *httpClient) uploadLogs(ctx context.Context, data []*logpb.ResourceLogs)
 
 			// overwrite the error message with the response body
 			// if it is not empty
-			respStr := strings.TrimSpace(respData.String())
-			if respStr != "" {
-				// pass the error message along with retry-able error,
-				// so that it can be retried and also passes the message
-				rErr = newResponseError(resp.Header, respStr)
+			if respStr := strings.TrimSpace(respData.String()); respStr != "" {
+				// Include response for context.
+				e := errors.New(respStr)
+				rErr = newResponseError(resp.Header, e)
 			}
 		default:
 			rErr = fmt.Errorf("failed to send logs to %s: %s", request.URL, resp.Status)
@@ -286,7 +285,7 @@ type retryableError struct {
 // newResponseError returns a retryableError and will extract any explicit
 // throttle delay contained in headers and if there is message in the response
 // body, it will be passed as error.
-func newResponseError(header http.Header, body string) error {
+func newResponseError(header http.Header, wrapped error) error {
 	var rErr retryableError
 	if v := header.Get("Retry-After"); v != "" {
 		if t, err := strconv.ParseInt(v, 10, 64); err == nil {
@@ -294,13 +293,13 @@ func newResponseError(header http.Header, body string) error {
 		}
 	}
 
-	rErr.err = fmt.Errorf("retry-able request failure: %s", body)
+	rErr.err = wrapped
 	return rErr
 }
 
 func (e retryableError) Error() string {
 	if e.err != nil {
-		return e.err.Error()
+		return fmt.Sprintf("retry-able request failure: %v", e.err.Error())
 	}
 
 	return "retry-able request failure"
diff --git a/exporters/otlp/otlplog/otlploghttp/client_test.go b/exporters/otlp/otlplog/otlploghttp/client_test.go
index 6a256a0d9d3..78e66feb62e 100644
--- a/exporters/otlp/otlplog/otlploghttp/client_test.go
+++ b/exporters/otlp/otlplog/otlploghttp/client_test.go
@@ -720,7 +720,7 @@ func TestConfig(t *testing.T) {
 
 		var retryErr *retryableError
 		assert.True(t, errors.As(err, &retryErr))
-		assert.Equal(t, fmt.Sprintf("retry-able request failure: 429: %v", exporterErr), retryErr.Unwrap().Error())
+		assert.Equal(t, fmt.Sprintf("429: %v", exporterErr), retryErr.Unwrap().Error())
 
 		var newErr *retryableError
 		assert.True(t, retryErr.As(&newErr))
diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
index 3d9ad0f39da..f9fb3f864d0 100644
--- a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
+++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
@@ -147,7 +147,7 @@ func (c *client) UploadMetrics(ctx context.Context, protoMetrics *metricpb.Resou
 		resp, err := c.httpClient.Do(request.Request)
 		var urlErr *url.Error
 		if errors.As(err, &urlErr) && urlErr.Temporary() {
-			return newResponseError(http.Header{}, err.Error())
+			return newResponseError(http.Header{}, err)
 		}
 		if err != nil {
 			return err
@@ -188,7 +188,7 @@ func (c *client) UploadMetrics(ctx context.Context, protoMetrics *metricpb.Resou
 			sc == http.StatusServiceUnavailable,
 			sc == http.StatusGatewayTimeout:
 			// Retry-able failure.
-			rErr = newResponseError(resp.Header, "")
+			rErr = newResponseError(resp.Header, nil)
 
 			// server may return a message with the response
 			// body, so we read it to include in the error
@@ -202,13 +202,11 @@ func (c *client) UploadMetrics(ctx context.Context, protoMetrics *metricpb.Resou
 
 			// overwrite the error message with the response body
 			// if it is not empty
-			respStr := strings.TrimSpace(respData.String())
-			if respStr != "" {
-				// pass the error message along with retry-able error,
-				// so that it can be retried and also passes the message
-				rErr = newResponseError(resp.Header, respStr)
+			if respStr := strings.TrimSpace(respData.String()); respStr != "" {
+				// Include response for context.
+				e := errors.New(respStr)
+				rErr = newResponseError(resp.Header, e)
 			}
-
 		default:
 			rErr = fmt.Errorf("failed to send metrics to %s: %s", request.URL, resp.Status)
 		}
@@ -290,7 +288,7 @@ type retryableError struct {
 // newResponseError returns a retryableError and will extract any explicit
 // throttle delay contained in headers and if there is message in the response
 // body, it will be passed as error.
-func newResponseError(header http.Header, body string) error {
+func newResponseError(header http.Header, wrapped error) error {
 	var rErr retryableError
 	if v := header.Get("Retry-After"); v != "" {
 		if t, err := strconv.ParseInt(v, 10, 64); err == nil {
@@ -298,13 +296,13 @@ func newResponseError(header http.Header, body string) error {
 		}
 	}
 
-	rErr.err = fmt.Errorf("retry-able request failure: %s", body)
+	rErr.err = wrapped
 	return rErr
 }
 
 func (e retryableError) Error() string {
 	if e.err != nil {
-		return e.err.Error()
+		return fmt.Sprintf("retry-able request failure: %s", e.err.Error())
 	}
 
 	return "retry-able request failure"
diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
index e3efe797e71..1845423c7b2 100644
--- a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
+++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
@@ -212,7 +212,7 @@ func TestConfig(t *testing.T) {
 
 		var retryErr *retryableError
 		assert.True(t, errors.As(err, &retryErr))
-		assert.Equal(t, fmt.Sprintf("retry-able request failure: 429: %v", exporterErr), retryErr.Unwrap().Error())
+		assert.Equal(t, fmt.Sprintf("429: %v", exporterErr), retryErr.Unwrap().Error())
 
 		var newErr *retryableError
 		assert.True(t, retryErr.As(&newErr))
diff --git a/exporters/otlp/otlptrace/otlptracehttp/client.go b/exporters/otlp/otlptrace/otlptracehttp/client.go
index a0023f63ecb..481197ed347 100644
--- a/exporters/otlp/otlptrace/otlptracehttp/client.go
+++ b/exporters/otlp/otlptrace/otlptracehttp/client.go
@@ -152,7 +152,7 @@ func (d *client) UploadTraces(ctx context.Context, protoSpans []*tracepb.Resourc
 		resp, err := d.client.Do(request.Request)
 		var urlErr *url.Error
 		if errors.As(err, &urlErr) && urlErr.Temporary() {
-			return newResponseError(http.Header{}, err.Error())
+			return newResponseError(http.Header{}, err)
 		}
 		if err != nil {
 			return err
@@ -200,7 +200,7 @@ func (d *client) UploadTraces(ctx context.Context, protoSpans []*tracepb.Resourc
 			sc == http.StatusServiceUnavailable,
 			sc == http.StatusGatewayTimeout:
 			// Retry-able failures.
-			rErr := newResponseError(resp.Header, "")
+			rErr := newResponseError(resp.Header, nil)
 
 			// server may return a message with the response
 			// body, so we read it to include in the error
@@ -214,11 +214,10 @@ func (d *client) UploadTraces(ctx context.Context, protoSpans []*tracepb.Resourc
 
 			// overwrite the error message with the response body
 			// if it is not empty
-			respStr := strings.TrimSpace(respData.String())
-			if respStr != "" {
-				// pass the error message along with retry-able error,
-				// so that it can be retried and also passes the message
-				rErr = newResponseError(resp.Header, respStr)
+			if respStr := strings.TrimSpace(respData.String()); respStr != "" {
+				// Include response for context.
+				e := errors.New(respStr)
+				rErr = newResponseError(resp.Header, e)
 			}
 			return rErr
 		default:
@@ -315,7 +314,7 @@ type retryableError struct {
 // newResponseError returns a retryableError and will extract any explicit
 // throttle delay contained in headers and if there is message in the response
 // body, it will be passed as error.
-func newResponseError(header http.Header, body string) error {
+func newResponseError(header http.Header, wrapped error) error {
 	var rErr retryableError
 	if s, ok := header["Retry-After"]; ok {
 		if t, err := strconv.ParseInt(s[0], 10, 64); err == nil {
@@ -323,13 +322,13 @@ func newResponseError(header http.Header, body string) error {
 		}
 	}
 
-	rErr.err = fmt.Errorf("retry-able request failure: %s", body)
+	rErr.err = wrapped
 	return rErr
 }
 
 func (e retryableError) Error() string {
 	if e.err != nil {
-		return e.err.Error()
+		return fmt.Sprintf("retry-able request failure: %s", e.err.Error())
 	}
 
 	return "retry-able request failure"

From 0e2dd7c711afb9aee1d1a58bf92c5ab8c4298262 Mon Sep 17 00:00:00 2001
From: Preeti Dewani <preetidewani1990@gmail.com>
Date: Fri, 12 Jul 2024 14:35:59 +0530
Subject: [PATCH 10/11] parse-errormsgs-in-retry-able-statuscodes: 1. Use
 better assert functions. 2. Fix comments

---
 exporters/otlp/otlplog/otlploghttp/client.go        |  4 ++--
 exporters/otlp/otlplog/otlploghttp/client_test.go   | 12 +++++++-----
 exporters/otlp/otlpmetric/otlpmetrichttp/client.go  |  4 ++--
 .../otlp/otlpmetric/otlpmetrichttp/client_test.go   | 13 ++++++++-----
 exporters/otlp/otlptrace/otlptracehttp/client.go    |  4 ++--
 5 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/exporters/otlp/otlplog/otlploghttp/client.go b/exporters/otlp/otlplog/otlploghttp/client.go
index d31c3d68f8b..f185305bb5e 100644
--- a/exporters/otlp/otlplog/otlploghttp/client.go
+++ b/exporters/otlp/otlplog/otlploghttp/client.go
@@ -283,8 +283,8 @@ type retryableError struct {
 }
 
 // newResponseError returns a retryableError and will extract any explicit
-// throttle delay contained in headers and if there is message in the response
-// body, it will be passed as error.
+// throttle delay contained in headers. The returned error wraps wrapped 
+// if it is not nil.
 func newResponseError(header http.Header, wrapped error) error {
 	var rErr retryableError
 	if v := header.Get("Retry-After"); v != "" {
diff --git a/exporters/otlp/otlplog/otlploghttp/client_test.go b/exporters/otlp/otlplog/otlploghttp/client_test.go
index 78e66feb62e..4c6be0274a4 100644
--- a/exporters/otlp/otlplog/otlploghttp/client_test.go
+++ b/exporters/otlp/otlplog/otlploghttp/client_test.go
@@ -716,15 +716,17 @@ func TestConfig(t *testing.T) {
 		t.Cleanup(func() { close(rCh) })
 		t.Cleanup(func() { require.NoError(t, exp.Shutdown(ctx)) })
 		err := exp.Export(ctx, make([]log.Record, 1))
-		assert.Error(t, err, exporterErr)
+		assert.ErrorContains(t, err, exporterErr.Error())
 
+		// To test the `Unwrap` function of retryable error
 		var retryErr *retryableError
-		assert.True(t, errors.As(err, &retryErr))
-		assert.Equal(t, fmt.Sprintf("429: %v", exporterErr), retryErr.Unwrap().Error())
+		assert.ErrorAs(t, err, &retryErr)
+		assert.ErrorContains(t, retryErr, fmt.Sprintf("429: %v", exporterErr))
 
+		// To test `As` function of retryable error
 		var newErr *retryableError
-		assert.True(t, retryErr.As(&newErr))
-		assert.Equal(t, newErr.Unwrap().Error(), retryErr.Unwrap().Error())
+		assert.ErrorAs(t, retryErr, &newErr)
+		assert.ErrorIs(t, *retryErr, *newErr)
 	})
 
 	t.Run("WithURLPath", func(t *testing.T) {
diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
index f9fb3f864d0..7ef295e59e3 100644
--- a/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
+++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client.go
@@ -286,8 +286,8 @@ type retryableError struct {
 }
 
 // newResponseError returns a retryableError and will extract any explicit
-// throttle delay contained in headers and if there is message in the response
-// body, it will be passed as error.
+// throttle delay contained in headers. The returned error wraps wrapped
+// if it is not nil.
 func newResponseError(header http.Header, wrapped error) error {
 	var rErr retryableError
 	if v := header.Get("Retry-After"); v != "" {
diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
index 1845423c7b2..520660cb076 100644
--- a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
+++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
@@ -208,15 +208,18 @@ func TestConfig(t *testing.T) {
 		t.Cleanup(func() { close(rCh) })
 		t.Cleanup(func() { require.NoError(t, exp.Shutdown(ctx)) })
 		err := exp.Export(ctx, &metricdata.ResourceMetrics{})
-		assert.EqualError(t, err, fmt.Sprintf("failed to upload metrics: retry-able request failure: %d: %v", http.StatusTooManyRequests, exporterErr))
+		assert.ErrorContains(t, err, exporterErr.Error())
 
+		// To test the `Unwrap` function of retryable error
 		var retryErr *retryableError
-		assert.True(t, errors.As(err, &retryErr))
-		assert.Equal(t, fmt.Sprintf("429: %v", exporterErr), retryErr.Unwrap().Error())
+		// assert.True(t, errors.As(err, &retryErr))
+		assert.ErrorAs(t, err, &retryErr)
+		assert.ErrorContains(t, retryErr, fmt.Sprintf("429: %v", exporterErr))
 
+		// To test `As` function of retryable error
 		var newErr *retryableError
-		assert.True(t, retryErr.As(&newErr))
-		assert.Equal(t, newErr.Unwrap().Error(), retryErr.Unwrap().Error())
+		assert.ErrorAs(t, retryErr, &newErr)
+		assert.ErrorIs(t, *retryErr, *newErr)
 	})
 
 	t.Run("WithURLPath", func(t *testing.T) {
diff --git a/exporters/otlp/otlptrace/otlptracehttp/client.go b/exporters/otlp/otlptrace/otlptracehttp/client.go
index 481197ed347..bb2f3ffd1d8 100644
--- a/exporters/otlp/otlptrace/otlptracehttp/client.go
+++ b/exporters/otlp/otlptrace/otlptracehttp/client.go
@@ -312,8 +312,8 @@ type retryableError struct {
 }
 
 // newResponseError returns a retryableError and will extract any explicit
-// throttle delay contained in headers and if there is message in the response
-// body, it will be passed as error.
+// throttle delay contained in headers. The returned error wraps wrapped
+// if it is not nil.
 func newResponseError(header http.Header, wrapped error) error {
 	var rErr retryableError
 	if s, ok := header["Retry-After"]; ok {

From 75af2ca1c702f21ea590da2068f0121a906fcfd5 Mon Sep 17 00:00:00 2001
From: Preeti Dewani <preetidewani1990@gmail.com>
Date: Fri, 12 Jul 2024 14:45:11 +0530
Subject: [PATCH 11/11] parse-errormsgs-in-retry-able-statuscodes: cover unwrap
 and As function in one test

---
 exporters/otlp/otlplog/otlploghttp/client.go           |  2 +-
 exporters/otlp/otlplog/otlploghttp/client_test.go      |  9 ++-------
 .../otlp/otlpmetric/otlpmetrichttp/client_test.go      | 10 ++--------
 3 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/exporters/otlp/otlplog/otlploghttp/client.go b/exporters/otlp/otlplog/otlploghttp/client.go
index f185305bb5e..1539bec7b90 100644
--- a/exporters/otlp/otlplog/otlploghttp/client.go
+++ b/exporters/otlp/otlplog/otlploghttp/client.go
@@ -283,7 +283,7 @@ type retryableError struct {
 }
 
 // newResponseError returns a retryableError and will extract any explicit
-// throttle delay contained in headers. The returned error wraps wrapped 
+// throttle delay contained in headers. The returned error wraps wrapped
 // if it is not nil.
 func newResponseError(header http.Header, wrapped error) error {
 	var rErr retryableError
diff --git a/exporters/otlp/otlplog/otlploghttp/client_test.go b/exporters/otlp/otlplog/otlploghttp/client_test.go
index 4c6be0274a4..c1ded095ab1 100644
--- a/exporters/otlp/otlplog/otlploghttp/client_test.go
+++ b/exporters/otlp/otlplog/otlploghttp/client_test.go
@@ -718,15 +718,10 @@ func TestConfig(t *testing.T) {
 		err := exp.Export(ctx, make([]log.Record, 1))
 		assert.ErrorContains(t, err, exporterErr.Error())
 
-		// To test the `Unwrap` function of retryable error
+		// To test the `Unwrap` and `As` function of retryable error
 		var retryErr *retryableError
 		assert.ErrorAs(t, err, &retryErr)
-		assert.ErrorContains(t, retryErr, fmt.Sprintf("429: %v", exporterErr))
-
-		// To test `As` function of retryable error
-		var newErr *retryableError
-		assert.ErrorAs(t, retryErr, &newErr)
-		assert.ErrorIs(t, *retryErr, *newErr)
+		assert.ErrorIs(t, err, *retryErr)
 	})
 
 	t.Run("WithURLPath", func(t *testing.T) {
diff --git a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
index 520660cb076..79b35a96456 100644
--- a/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
+++ b/exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go
@@ -210,16 +210,10 @@ func TestConfig(t *testing.T) {
 		err := exp.Export(ctx, &metricdata.ResourceMetrics{})
 		assert.ErrorContains(t, err, exporterErr.Error())
 
-		// To test the `Unwrap` function of retryable error
+		// To test the `Unwrap` and `As` function of retryable error
 		var retryErr *retryableError
-		// assert.True(t, errors.As(err, &retryErr))
 		assert.ErrorAs(t, err, &retryErr)
-		assert.ErrorContains(t, retryErr, fmt.Sprintf("429: %v", exporterErr))
-
-		// To test `As` function of retryable error
-		var newErr *retryableError
-		assert.ErrorAs(t, retryErr, &newErr)
-		assert.ErrorIs(t, *retryErr, *newErr)
+		assert.ErrorIs(t, err, *retryErr)
 	})
 
 	t.Run("WithURLPath", func(t *testing.T) {