Do not handle empty partial OTLP successes (#3438)

* 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

* Fix tests

* Add changes to changelog

Author: NullBrotasli

CHANGELOG.md

@@ -32,6 +32,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - Cumulative metrics from the OpenCensus bridge (`go.opentelemetry.io/otel/bridge/opencensus`) are defined as monotonic sums, instead of non-monotonic. (#3389)
 - Asynchronous counters (`Counter` and `UpDownCounter`) from the metric SDK now produce delta sums when configured with delta temporality. (#3398)
 - Exported `Status` codes in the `go.opentelemetry.io/otel/exporters/zipkin` exporter are now exported as all upper case values. (#3340)
+- Do not report empty partial-success responses in the `go.opentelemetry.io/otel/exporters/otlp` exporters. (#3438, #3432)
 
 ## [1.11.1/0.33.0] 2022-10-19
 

exporters/otlp/internal/partialsuccess.go

@@ -16,27 +16,14 @@ 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
 // error handler belongs to this category.
 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",
 	}
 }

exporters/otlp/internal/partialsuccess_test.go

@@ -36,9 +36,8 @@ func requireErrorString(t *testing.T, expect string, err error) {
 }
 
 func TestPartialSuccessFormat(t *testing.T) {
-	requireErrorString(t, "empty message (0 metric data points rejected)", PartialSuccessToError(MetricsPartialSuccess, 0, ""))
-	requireErrorString(t, "help help (0 metric data points rejected)", PartialSuccessToError(MetricsPartialSuccess, 0, "help help"))
-	requireErrorString(t, "what happened (10 metric data points rejected)", PartialSuccessToError(MetricsPartialSuccess, 10, "what happened"))
-	requireErrorString(t, "what happened (15 spans rejected)", PartialSuccessToError(TracingPartialSuccess, 15, "what happened"))
-	requireErrorString(t, "empty message (7 log records rejected)", PartialSuccessToError("log records", 7, ""))
+	requireErrorString(t, "empty message (0 metric data points rejected)", MetricPartialSuccessError(0, ""))
+	requireErrorString(t, "help help (0 metric data points rejected)", MetricPartialSuccessError(0, "help help"))
+	requireErrorString(t, "what happened (10 metric data points rejected)", MetricPartialSuccessError(10, "what happened"))
+	requireErrorString(t, "what happened (15 spans rejected)", TracePartialSuccessError(15, "what happened"))
 }

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 {

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