From 8df11939c1d6093d84a52fdc380010b525514d05 Mon Sep 17 00:00:00 2001
From: Tyler Yahn <codingalias@gmail.com>
Date: Wed, 2 Nov 2022 09:08:35 -0700
Subject: [PATCH] Do not handle empty partial OTLP successes

Fix #3432.

The OTLP server will respond with empty partial success responses (i.e.
empty messages and 0 count). Treat these as equivalent to it not being
set/present like the documentation specifies in the proto:
https://github.com/open-telemetry/opentelemetry-proto/blob/724e427879e3d2bae2edc0218fff06e37b9eb46e/opentelemetry/proto/collector/trace/v1/trace_service.proto#L58
---
 exporters/otlp/internal/partialsuccess.go     | 34 ++++++++-----------
 .../otlp/otlptrace/otlptracegrpc/client.go    | 11 +++---
 .../otlp/otlptrace/otlptracehttp/client.go    | 11 +++---
 3 files changed, 27 insertions(+), 29 deletions(-)

diff --git a/exporters/otlp/internal/partialsuccess.go b/exporters/otlp/internal/partialsuccess.go
index 7994706ab51..9ab89b37574 100644
--- a/exporters/otlp/internal/partialsuccess.go
+++ b/exporters/otlp/internal/partialsuccess.go
@@ -16,19 +16,6 @@ package internal // import "go.opentelemetry.io/otel/exporters/otlp/internal"
 
 import "fmt"
 
-// PartialSuccessDropKind indicates the kind of partial success error
-// received by an OTLP exporter, which corresponds with the signal
-// being exported.
-type PartialSuccessDropKind string
-
-const (
-	// TracingPartialSuccess indicates that some spans were rejected.
-	TracingPartialSuccess PartialSuccessDropKind = "spans"
-
-	// MetricsPartialSuccess indicates that some metric data points were rejected.
-	MetricsPartialSuccess PartialSuccessDropKind = "metric data points"
-)
-
 // PartialSuccess represents the underlying error for all handling
 // OTLP partial success messages.  Use `errors.Is(err,
 // PartialSuccess{})` to test whether an error passed to the OTel
@@ -36,7 +23,7 @@ const (
 type PartialSuccess struct {
 	ErrorMessage  string
 	RejectedItems int64
-	RejectedKind  PartialSuccessDropKind
+	RejectedKind  string
 }
 
 var _ error = PartialSuccess{}
@@ -56,13 +43,22 @@ func (ps PartialSuccess) Is(err error) bool {
 	return ok
 }
 
-// PartialSuccessToError produces an error suitable for passing to
-// `otel.Handle()` out of the fields in a partial success response,
-// independent of which signal produced the outcome.
-func PartialSuccessToError(kind PartialSuccessDropKind, itemsRejected int64, errorMessage string) error {
+// TracePartialSuccessError returns an error describing a partial success
+// response for the trace signal.
+func TracePartialSuccessError(itemsRejected int64, errorMessage string) error {
+	return PartialSuccess{
+		ErrorMessage:  errorMessage,
+		RejectedItems: itemsRejected,
+		RejectedKind:  "spans",
+	}
+}
+
+// MetricPartialSuccessError returns an error describing a partial success
+// response for the metric signal.
+func MetricPartialSuccessError(itemsRejected int64, errorMessage string) error {
 	return PartialSuccess{
 		ErrorMessage:  errorMessage,
 		RejectedItems: itemsRejected,
-		RejectedKind:  kind,
+		RejectedKind:  "metric data points",
 	}
 }
diff --git a/exporters/otlp/otlptrace/otlptracegrpc/client.go b/exporters/otlp/otlptrace/otlptracegrpc/client.go
index 9d6e1898b14..fe23f8e3766 100644
--- a/exporters/otlp/otlptrace/otlptracegrpc/client.go
+++ b/exporters/otlp/otlptrace/otlptracegrpc/client.go
@@ -202,11 +202,12 @@ func (c *client) UploadTraces(ctx context.Context, protoSpans []*tracepb.Resourc
 			ResourceSpans: protoSpans,
 		})
 		if resp != nil && resp.PartialSuccess != nil {
-			otel.Handle(internal.PartialSuccessToError(
-				internal.TracingPartialSuccess,
-				resp.PartialSuccess.RejectedSpans,
-				resp.PartialSuccess.ErrorMessage,
-			))
+			msg := resp.PartialSuccess.GetErrorMessage()
+			n := resp.PartialSuccess.GetRejectedSpans()
+			if n != 0 || msg != "" {
+				err := internal.TracePartialSuccessError(n, msg)
+				otel.Handle(err)
+			}
 		}
 		// nil is converted to OK.
 		if status.Code(err) == codes.OK {
diff --git a/exporters/otlp/otlptrace/otlptracehttp/client.go b/exporters/otlp/otlptrace/otlptracehttp/client.go
index 8f742dfc1bd..79b8af73286 100644
--- a/exporters/otlp/otlptrace/otlptracehttp/client.go
+++ b/exporters/otlp/otlptrace/otlptracehttp/client.go
@@ -180,11 +180,12 @@ func (d *client) UploadTraces(ctx context.Context, protoSpans []*tracepb.Resourc
 				}
 
 				if respProto.PartialSuccess != nil {
-					otel.Handle(internal.PartialSuccessToError(
-						internal.TracingPartialSuccess,
-						respProto.PartialSuccess.RejectedSpans,
-						respProto.PartialSuccess.ErrorMessage,
-					))
+					msg := respProto.PartialSuccess.GetErrorMessage()
+					n := respProto.PartialSuccess.GetRejectedSpans()
+					if n != 0 || msg != "" {
+						err := internal.TracePartialSuccessError(n, msg)
+						otel.Handle(err)
+					}
 				}
 			}
 			return nil