From 862a5a68a8a0186b12abc6c2ca42941ee15441b7 Mon Sep 17 00:00:00 2001
From: Sam Xie <xsambundy@gmail.com>
Date: Thu, 25 Mar 2021 23:22:48 +0800
Subject: [PATCH] Remove setting error status while recording error with Span
 from oteltest package (#1729)

* Remove setting error status while recording error

* Update CHANGELOG

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
---
 CHANGELOG.md          |  1 +
 oteltest/span.go      |  1 -
 oteltest/span_test.go | 30 +-----------------------------
 trace/trace.go        |  5 ++++-
 4 files changed, 6 insertions(+), 31 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1b38561314c..0080ffa2ebb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,6 +19,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - No longer set the links for a `Span` in `go.opentelemetry.io/otel/sdk/trace` that is configured to be a new root.
   This is unspecified behavior that the OpenTelemetry community plans to standardize in the future.
   To prevent backwards incompatible changes when it is specified, these links are removed. (#1726)
+- Setting error status while recording error with Span from oteltest package. (#1729)
 
 ## [0.19.0] - 2021-03-18
 
diff --git a/oteltest/span.go b/oteltest/span.go
index 2e109f2d88a..2b85516a31c 100644
--- a/oteltest/span.go
+++ b/oteltest/span.go
@@ -91,7 +91,6 @@ func (s *Span) RecordError(err error, opts ...trace.EventOption) {
 		errTypeString = errType.String()
 	}
 
-	s.SetStatus(codes.Error, "")
 	opts = append(opts, trace.WithAttributes(
 		errorTypeKey.String(errTypeString),
 		errorMessageKey.String(err.Error()),
diff --git a/oteltest/span_test.go b/oteltest/span_test.go
index 3f8304d065d..eb00f0e2922 100644
--- a/oteltest/span_test.go
+++ b/oteltest/span_test.go
@@ -167,39 +167,11 @@ func TestSpan(t *testing.T) {
 					},
 				}}
 				e.Expect(subject.Events()).ToEqual(expectedEvents)
-				e.Expect(subject.StatusCode()).ToEqual(codes.Error)
+				e.Expect(subject.StatusCode()).ToEqual(codes.Unset)
 				e.Expect(subject.StatusMessage()).ToEqual("")
 			}
 		})
 
-		t.Run("sets span status if provided", func(t *testing.T) {
-			t.Parallel()
-
-			e := matchers.NewExpecter(t)
-
-			tracer := tp.Tracer(t.Name())
-			_, span := tracer.Start(context.Background(), "test")
-
-			subject, ok := span.(*oteltest.Span)
-			e.Expect(ok).ToBeTrue()
-
-			errMsg := "test error message"
-			testErr := ottest.NewTestError(errMsg)
-			testTime := time.Now()
-			subject.RecordError(testErr, trace.WithTimestamp(testTime))
-
-			expectedEvents := []oteltest.Event{{
-				Timestamp: testTime,
-				Name:      "error",
-				Attributes: map[attribute.Key]attribute.Value{
-					attribute.Key("error.type"):    attribute.StringValue("go.opentelemetry.io/otel/internal/internaltest.TestError"),
-					attribute.Key("error.message"): attribute.StringValue(errMsg),
-				},
-			}}
-			e.Expect(subject.Events()).ToEqual(expectedEvents)
-			e.Expect(subject.StatusCode()).ToEqual(codes.Error)
-		})
-
 		t.Run("cannot be set after the span has ended", func(t *testing.T) {
 			t.Parallel()
 
diff --git a/trace/trace.go b/trace/trace.go
index fc45a86388f..5d5c284e80c 100644
--- a/trace/trace.go
+++ b/trace/trace.go
@@ -545,7 +545,10 @@ type Span interface {
 	// true if the Span is active and events can be recorded.
 	IsRecording() bool
 
-	// RecordError records an error as a Span event.
+	// RecordError will record err as a span event for this span. An additional call to
+	// SetStatus is required if the Status of the Span should be set to Error, this method
+	// does not change the Span status. If this span is not being recorded or err is nil
+	// than this method does nothing.
 	RecordError(err error, options ...EventOption)
 
 	// SpanContext returns the SpanContext of the Span. The returned