Address PR feedback

Author: evan-bradley

consumer/consumererror/README.md

@@ -198,21 +198,20 @@ component can then later pull all errors out for analysis.
 > here is for design purposes. The code snippet may not work as-written.
 
 When a receiver gets a response that includes an error, it can get the data out
-by doing something similar to the following. Note that this uses the `ErrorData`
-type, which is for reading error data, as opposed to the `Error` type, which is
-for recording errors.
+by doing something similar to the following. Note that this uses the `Error`
+type, which is for reading error data, as opposed to the `ErrorContainer` type,
+which is for recording errors.
 
 ```go
-cerr := consumererror.Error{}
-var errData []consumerError.ErrorData 
+cerr := consumererror.ErrorContainer{}
 
 if errors.As(err, &cerr) {
-  errData := cerr.Data()
+  errs := cerr.Errors()
 
-  for _, data := range errData {
-    data.HTTPStatus()
-    data.Retryable()
-    data.Partial()
+  for _, e := range errs {
+    e.HTTPStatus()
+    e.Retryable()
+    e.Partial()
   }
 }
 ```
@@ -227,7 +226,7 @@ Obtaining data from an error can be done using an interface that looks something
 like this:
 
 ```go
-type ErrorData interface {
+type Error interface {
   // Returns the underlying error
   Error() error
 
@@ -302,5 +301,5 @@ The following describes how to transition to these error types:
 
 `consumererror.IsPermanent` will be deprecated in favor of checking whether
 retry information is available, and only retrying if it has been provided. This
-will be possible by calling `ErrorData.Retryable()` and checking for retry
+will be possible by calling `Error.Retryable()` and checking for retry
 information.

consumer/consumererror/error.go

@@ -11,26 +11,26 @@ import (
 	"go.opentelemetry.io/collector/consumer/consumererror/internal/statusconversion"
 )
 
-// Error acts as a container for errors coming from pipeline components.
+// ErrorContainer acts as a container for errors coming from pipeline components.
 // It may hold multiple errors from downstream components, and is designed
 // to act as a way to accumulate errors as it travels upstream in a pipeline.
-// `Error` should be obtained using `errors.As` and as a result, ideally
+// `ErrorContainer` should be obtained using `errors.As` and as a result, ideally
 // a single instance should exist in an error stack. If this is not possible,
-// the most `Error` object should be highest on the stack.
+// the most `ErrorContainer` object should be highest on the stack.
 //
 // Experimental: This API is at the early stage of development and may change without backward compatibility
-type Error struct {
-	errors []ErrorData
+type ErrorContainer struct {
+	errors []Error
 }
 
-// ErrorData is intended to be used to encapsulate various information
+// Error is intended to be used to encapsulate various information
 // that can add context to an error that occurred within a pipeline component.
-// ErrorData objects are constructed through calling `New` with the relevant
+// Error objects are constructed through calling `New` with the relevant
 // options to capture data around the error that occurred. It can then be pulled
 // out by an upstream component by calling `Error.Data`.
 //
 // Experimental: This API is at the early stage of development and may change without backward compatibility
-type ErrorData interface {
+type Error interface {
 	Error() string
 
 	Unwrap() error
@@ -55,32 +55,40 @@ type errorData struct {
 }
 
 // ErrorOption allows annotating an Error with metadata.
-type ErrorOption func(error *errorData)
+type ErrorOption interface {
+	applyOption(*errorData)
+}
+
+type errorOptionFunc func(*errorData)
+
+func (f errorOptionFunc) applyOption(e *errorData) {
+	f(e)
+}
 
 // New wraps an error that happened while consuming telemetry
 // and adds metadata onto it to be passed back up the pipeline.
 //
 // Experimental: This API is at the early stage of development and may change without backward compatibility
 func New(origErr error, options ...ErrorOption) error {
 	errData := &errorData{error: origErr}
-	err := &Error{errors: []ErrorData{errData}}
+	err := &ErrorContainer{errors: []Error{errData}}
 
 	for _, option := range options {
-		option(errData)
+		option.applyOption(errData)
 	}
 
 	return err
 }
 
-var _ error = &Error{}
+var _ error = (*ErrorContainer)(nil)
 
 // Error implements the `error` interface.
-func (e *Error) Error() string {
+func (e *ErrorContainer) Error() string {
 	return e.errors[len(e.errors)-1].Error()
 }
 
 // Unwrap returns the wrapped error for use by `errors.Is` and `errors.As`.
-func (e *Error) Unwrap() []error {
+func (e *ErrorContainer) Unwrap() []error {
 	errors := make([]error, 0, len(e.errors))
 
 	for _, err := range e.errors {
@@ -90,21 +98,21 @@ func (e *Error) Unwrap() []error {
 	return errors
 }
 
-// Data returns all the accumulated ErrorData errors
+// Errors returns all the accumulated Error objects
 // emitted by components downstream in the pipeline.
 // These can then be aggregated or worked with individually.
-func (e *Error) Data() []ErrorData {
+func (e *ErrorContainer) Errors() []Error {
 	return e.errors
 }
 
 // Combine joins errors that occur at a fanout into a single
 // `Error` object. The component that created the data submission
 // can then work with the `Error` object to understand the failure.
-func Combine(errs ...error) *Error {
-	e := &Error{errors: make([]ErrorData, 0, len(errs))}
+func Combine(errs ...error) *ErrorContainer {
+	e := &ErrorContainer{errors: make([]Error, 0, len(errs))}
 
 	for _, err := range errs {
-		var otherErr *Error
+		var otherErr *ErrorContainer
 		if errors.As(err, &otherErr) {
 			e.errors = append(e.errors, otherErr.errors...)
 		} else {
@@ -118,17 +126,17 @@ func Combine(errs ...error) *Error {
 // WithHTTPStatus records an HTTP status code that was received
 // from a server during data submission.
 func WithHTTPStatus(status int) ErrorOption {
-	return func(err *errorData) {
+	return errorOptionFunc(func(err *errorData) {
 		err.httpStatus = &status
-	}
+	})
 }
 
 // WithGRPCStatus records a gRPC status code that was received
 // from a server during data submission.
 func WithGRPCStatus(status *status.Status) ErrorOption {
-	return func(err *errorData) {
+	return errorOptionFunc(func(err *errorData) {
 		err.grpcStatus = status
-	}
+	})
 }
 
 var _ error = (*errorData)(nil)

consumer/consumererror/error_test.go

@@ -18,8 +18,8 @@ var errTest = errors.New("consumererror testing error")
 func Test_New(t *testing.T) {
 	httpStatus := 500
 	grpcStatus := status.New(codes.Aborted, "aborted")
-	wantErr := &Error{
-		errors: []ErrorData{
+	wantErr := &ErrorContainer{
+		errors: []Error{
 			&errorData{
 				error:      errTest,
 				httpStatus: &httpStatus,
@@ -43,8 +43,8 @@ func Test_Error(t *testing.T) {
 }
 
 func TestUnwrap(t *testing.T) {
-	err := &Error{
-		errors: []ErrorData{
+	err := &ErrorContainer{
+		errors: []Error{
 			&errorData{
 				error: errTest,
 			},
@@ -65,8 +65,8 @@ func TestUnwrap(t *testing.T) {
 
 func TestData(t *testing.T) {
 	httpStatus := 500
-	err := &Error{
-		errors: []ErrorData{
+	err := &ErrorContainer{
+		errors: []Error{
 			&errorData{
 				error:      errTest,
 				httpStatus: &httpStatus,
@@ -78,7 +78,7 @@ func TestData(t *testing.T) {
 		},
 	}
 
-	data := err.Data()
+	data := err.Errors()
 
 	require.Len(t, data, 2)
 
@@ -89,16 +89,47 @@ func TestData(t *testing.T) {
 	}
 }
 
+func TestErrorSliceIsCopy(t *testing.T) {
+	httpStatus := 500
+	err := &ErrorContainer{
+		errors: []Error{
+			&errorData{
+				error:      errTest,
+				httpStatus: &httpStatus,
+			},
+			&errorData{
+				error:      errTest,
+				httpStatus: &httpStatus,
+			},
+		},
+	}
+
+	errs := err.Errors()
+
+	require.Len(t, errs, 2)
+
+	for _, e := range errs {
+		status, ok := e.HTTPStatus()
+		require.True(t, ok)
+		require.Equal(t, httpStatus, status)
+	}
+
+	errs = append(errs, &errorData{error: errTest})
+
+	require.Len(t, errs, 3)
+	require.Len(t, err.errors, 2)
+}
+
 func TestCombine(t *testing.T) {
-	err0 := &Error{
-		errors: []ErrorData{
+	err0 := &ErrorContainer{
+		errors: []Error{
 			&errorData{
 				error: errTest,
 			},
 		},
 	}
-	err1 := &Error{
-		errors: []ErrorData{
+	err1 := &ErrorContainer{
+		errors: []Error{
 			&errorData{
 				error: errTest,
 			},
@@ -107,8 +138,8 @@ func TestCombine(t *testing.T) {
 			},
 		},
 	}
-	want := &Error{
-		errors: []ErrorData{
+	want := &ErrorContainer{
+		errors: []Error{
 			&errorData{
 				error: errTest,
 			},
@@ -129,23 +160,23 @@ func TestCombine(t *testing.T) {
 	require.Equal(t, want, err)
 }
 
-func TestErrorDataError(t *testing.T) {
+func TestError_Error(t *testing.T) {
 	err := &errorData{
 		error: errTest,
 	}
 
 	require.Equal(t, errTest.Error(), err.Error())
 }
 
-func TestErrorDataUnwrap(t *testing.T) {
+func TestError_Unwrap(t *testing.T) {
 	err := &errorData{
 		error: errTest,
 	}
 
 	require.Equal(t, errTest, err.Unwrap())
 }
 
-func TestErrorDataHTTPStatus(t *testing.T) {
+func TestError_HTTPStatus(t *testing.T) {
 	serverErr := http.StatusTooManyRequests
 	testCases := []struct {
 		name       string
@@ -195,7 +226,7 @@ func TestErrorDataHTTPStatus(t *testing.T) {
 	}
 }
 
-func TestErrorDataGRPCStatus(t *testing.T) {
+func TestError_GRPCStatus(t *testing.T) {
 	httpStatus := http.StatusTooManyRequests
 	otherHTTPStatus := http.StatusOK
 	serverErr := status.New(codes.ResourceExhausted, errTest.Error())