fix: _set_status should not override existing span status (#3713)

Author: RiyaChaturvedi37

opentelemetry-instrumentation/src/opentelemetry/instrumentation/_semconv.py

@@ -650,7 +650,21 @@ def _set_status(
                     span.set_attribute(ERROR_TYPE, status_code_str)
                 metrics_attributes[ERROR_TYPE] = status_code_str
         if span.is_recording():
-            span.set_status(Status(status))
+            current_status = getattr(span, "status", None)
+            if current_status is not None:
+                _STATUS_PRIORITY = {
+                    StatusCode.UNSET: 0,
+                    StatusCode.OK: 1,
+                    StatusCode.ERROR: 2,
+                }
+                current_priority = _STATUS_PRIORITY.get(current_status.status_code, 0)
+                incoming_priority = _STATUS_PRIORITY.get(status, 0)
+                if incoming_priority < current_priority:
+                    return
+                description = current_status.description if current_status.description and status == current_status.status_code else None
+                span.set_status(Status(status, description))
+            else:
+                span.set_status(Status(status))
 
 
 def _get_schema_url(mode: _StabilityMode) -> str:

opentelemetry-instrumentation/tests/test_semconv_status.py

@@ -0,0 +1,65 @@
+# Copyright The OpenTelemetry Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+
+import unittest
+from unittest.mock import MagicMock
+from opentelemetry.trace.status import Status, StatusCode
+from opentelemetry.instrumentation._semconv import (
+    _set_status,
+    _StabilityMode,
+)
+
+
+class TestSetStatus(unittest.TestCase):
+
+    def _make_span(self, status_code, description=None):
+        span = MagicMock()
+        span.is_recording.return_value = True
+        span.status = Status(status_code, description)
+        return span
+
+    def test_does_not_downgrade_error_to_ok(self):
+        """ERROR status should not be overridden by a lower priority OK"""
+        span = self._make_span(StatusCode.ERROR, "original error")
+        _set_status(span, {}, 200, "200", server_span=True,
+                    sem_conv_opt_in_mode=_StabilityMode.DEFAULT)
+        # set_status should NOT have been called (no downgrade)
+        for call in span.set_status.call_args_list:
+            args = call[0]
+            if args:
+                self.assertNotEqual(args[0].status_code, StatusCode.OK)
+
+    def test_does_not_wipe_description_with_none(self):
+        """Same ERROR status should preserve existing description"""
+        span = self._make_span(StatusCode.ERROR, "keep this message")
+        _set_status(span, {}, 500, "500", server_span=True,
+                    sem_conv_opt_in_mode=_StabilityMode.DEFAULT)
+        last_call = span.set_status.call_args
+        if last_call:
+            status_arg = last_call[0][0]
+            self.assertEqual(status_arg.description, "keep this message")
+
+    def test_upgrades_unset_to_error(self):
+        """UNSET status should be upgraded to ERROR"""
+        span = self._make_span(StatusCode.UNSET)
+        _set_status(span, {}, 500, "500", server_span=True,
+                    sem_conv_opt_in_mode=_StabilityMode.DEFAULT)
+        span.set_status.assert_called()
+        last_call = span.set_status.call_args[0][0]
+        self.assertEqual(last_call.status_code, StatusCode.ERROR)
+
+    def test_unset_to_ok(self):
+        """UNSET status should be upgraded to OK for 2xx"""
+        span = self._make_span(StatusCode.UNSET)
+        _set_status(span, {}, 200, "200", server_span=False,
+                    sem_conv_opt_in_mode=_StabilityMode.DEFAULT)
+        span.set_status.assert_called()
+
+
+if __name__ == "__main__":
+    unittest.main()