Span Status: only set description for ERROR status code.

According to the spec:

> Description MUST only be used with the Error StatusCode value.
> An empty Description is equivalent with a not present one.

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#set-status

This commit updates the `Status` implementation to comply with the spec.

Author: owais

CHANGELOG.md

@@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v0.18b0...HEAD)
+- Status now only sets `description` when `status_code` is set to `StatusCode.ERROR`
+  ([#1673](https://github.com/open-telemetry/opentelemetry-python/pull/1673))
 
 ### Added
 - Add `max_attr_value_length` support to Jaeger exporter

exporter/opentelemetry-exporter-jaeger/tests/test_jaeger_exporter_protobuf.py

@@ -192,7 +192,7 @@ def test_translate_to_jaeger(self):
         otel_spans[1].end(end_time=end_times[1])
 
         otel_spans[2].start(start_time=start_times[2])
-        otel_spans[2].set_status(Status(StatusCode.OK, "Example description"))
+        otel_spans[2].set_status(Status(StatusCode.OK))
         otel_spans[2].end(end_time=end_times[2])
 
         translate = Translate(otel_spans)
@@ -352,11 +352,6 @@ def test_translate_to_jaeger(self):
                         v_type=model_pb2.ValueType.STRING,
                         v_str="OK",
                     ),
-                    model_pb2.KeyValue(
-                        key="otel.status_description",
-                        v_type=model_pb2.ValueType.STRING,
-                        v_str="Example description",
-                    ),
                     model_pb2.KeyValue(
                         key="span.kind",
                         v_type=model_pb2.ValueType.STRING,

exporter/opentelemetry-exporter-jaeger/tests/test_jaeger_exporter_thrift.py

@@ -274,7 +274,7 @@ def test_translate_to_jaeger(self):
         otel_spans[1].end(end_time=end_times[1])
 
         otel_spans[2].start(start_time=start_times[2])
-        otel_spans[2].set_status(Status(StatusCode.OK, "Example description"))
+        otel_spans[2].set_status(Status(StatusCode.OK))
         otel_spans[2].end(end_time=end_times[2])
 
         translate = Translate(otel_spans)
@@ -396,11 +396,6 @@ def test_translate_to_jaeger(self):
                         vType=jaeger.TagType.STRING,
                         vStr="OK",
                     ),
-                    jaeger.Tag(
-                        key="otel.status_description",
-                        vType=jaeger.TagType.STRING,
-                        vStr="Example description",
-                    ),
                     jaeger.Tag(
                         key="span.kind",
                         vType=jaeger.TagType.STRING,

exporter/opentelemetry-exporter-opencensus/tests/test_otcollector_trace_exporter.py

@@ -147,9 +147,7 @@ def test_translate_to_collector(self):
         otel_spans[0].set_attribute("key_float", 111.22)
         otel_spans[0].set_attribute("key_int", 333)
         otel_spans[0].set_status(
-            trace_api.Status(
-                trace_api.status.StatusCode.OK, "test description",
-            )
+            trace_api.Status(trace_api.status.StatusCode.OK)
         )
         otel_spans[0].end(end_time=end_times[0])
         otel_spans[1].start(start_time=start_times[1])
@@ -198,7 +196,6 @@ def test_translate_to_collector(self):
         self.assertEqual(
             output_spans[0].status.code, trace_api.status.StatusCode.OK.value,
         )
-        self.assertEqual(output_spans[0].status.message, "test description")
         self.assertEqual(len(output_spans[0].tracestate.entries), 1)
         self.assertEqual(output_spans[0].tracestate.entries[0].key, "testkey")
         self.assertEqual(

opentelemetry-api/src/opentelemetry/trace/status.py

@@ -48,10 +48,18 @@ def __init__(
     ):
         self._status_code = status_code
         self._description = None
+
         if description is not None and not isinstance(description, str):
             logger.warning("Invalid status description type, expected str")
-        else:
-            self._description = description
+            return
+
+        if status_code != StatusCode.ERROR:
+            logger.warning(
+                "description should only be set when status_code is set to StatusCode.ERROR"
+            )
+            return
+
+        self._description = description
 
     @property
     def status_code(self) -> StatusCode:

opentelemetry-api/tests/trace/test_status.py

@@ -29,7 +29,40 @@ def test_constructor(self):
         self.assertEqual(status.description, "unavailable")
 
     def test_invalid_description(self):
-        with self.assertLogs(level=WARNING):
+        with self.assertLogs(level=WARNING) as warning:
             status = Status(description={"test": "val"})  # type: ignore
             self.assertIs(status.status_code, StatusCode.UNSET)
             self.assertEqual(status.description, None)
+            self.assertIn(
+                "Invalid status description type, expected str",
+                warning.output[0],
+            )
+
+    def test_description_and_non_error_status(self):
+        with self.assertLogs(level=WARNING) as warning:
+            status = Status(
+                status_code=StatusCode.OK, description="status description"
+            )
+            self.assertIs(status.status_code, StatusCode.OK)
+            self.assertEqual(status.description, None)
+            self.assertIn(
+                "description should only be set when status_code is set to StatusCode.ERROR",
+                warning.output[0],
+            )
+
+        with self.assertLogs(level=WARNING) as warning:
+            status = Status(
+                status_code=StatusCode.UNSET, description="status description"
+            )
+            self.assertIs(status.status_code, StatusCode.UNSET)
+            self.assertEqual(status.description, None)
+            self.assertIn(
+                "description should only be set when status_code is set to StatusCode.ERROR",
+                warning.output[0],
+            )
+
+        status = Status(
+            status_code=StatusCode.ERROR, description="status description"
+        )
+        self.assertIs(status.status_code, StatusCode.ERROR)
+        self.assertEqual(status.description, "status description")

opentelemetry-sdk/tests/trace/test_trace.py

@@ -951,13 +951,10 @@ def test_override_error_status(self):
         def error_status_test(context):
             with self.assertRaises(AssertionError):
                 with context as root:
-                    root.set_status(
-                        trace_api.status.Status(StatusCode.OK, "OK")
-                    )
+                    root.set_status(trace_api.status.Status(StatusCode.OK))
                     raise AssertionError("unknown")
 
             self.assertIs(root.status.status_code, StatusCode.OK)
-            self.assertEqual(root.status.description, "OK")
 
         error_status_test(
             trace.TracerProvider().get_tracer(__name__).start_span("root")