From f994e1463645d6e5fbd617fc534cbbf70f4383ef Mon Sep 17 00:00:00 2001
From: Shalev Roda <65566801+shalevr@users.noreply.github.com>
Date: Sat, 5 Nov 2022 16:29:47 +0200
Subject: [PATCH] Fix Urllib instrumentation - Add status code to span if not
 None (#1430)

---
 CHANGELOG.md                                  |  5 ++++
 .../instrumentation/urllib/__init__.py        |  2 +-
 .../tests/test_urllib_integration.py          | 30 +++++++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f9dd4c9b99..44c1be3903 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+### Fixed
+
+- Fix bug in Urllib instrumentation - add status code to span attributes only if the status code is not None. 
+  ([#1430](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1430))
+
 ## Version 1.14.0/0.35b0 (2022-11-03)
 
 ### Deprecated
diff --git a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py
index 04244989f3..b57304f762 100644
--- a/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py
+++ b/instrumentation/opentelemetry-instrumentation-urllib/src/opentelemetry/instrumentation/urllib/__init__.py
@@ -206,7 +206,7 @@ def _instrumented_open_call(
                 code_ = result.getcode()
                 labels[SpanAttributes.HTTP_STATUS_CODE] = str(code_)
 
-                if span.is_recording():
+                if span.is_recording() and code_ is not None:
                     span.set_attribute(SpanAttributes.HTTP_STATUS_CODE, code_)
                     span.set_status(Status(http_status_to_status_code(code_)))
 
diff --git a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py
index d819d481cc..73c96ede14 100644
--- a/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py
+++ b/instrumentation/opentelemetry-instrumentation-urllib/tests/test_urllib_integration.py
@@ -16,6 +16,7 @@
 import socket
 import urllib
 from unittest import mock
+from unittest.mock import patch
 from urllib import request
 from urllib.error import HTTPError
 from urllib.request import OpenerDirector
@@ -150,6 +151,35 @@ def test_not_foundbasic(self):
             trace.StatusCode.ERROR,
         )
 
+    @staticmethod
+    def mock_get_code(*args, **kwargs):
+        return None
+
+    @patch("http.client.HTTPResponse.getcode", new=mock_get_code)
+    def test_response_code_none(self):
+
+        result = self.perform_request(self.URL)
+
+        self.assertEqual(result.read(), b"Hello!")
+        span = self.assert_span()
+
+        self.assertIs(span.kind, trace.SpanKind.CLIENT)
+        self.assertEqual(span.name, "HTTP GET")
+
+        self.assertEqual(
+            span.attributes,
+            {
+                SpanAttributes.HTTP_METHOD: "GET",
+                SpanAttributes.HTTP_URL: self.URL,
+            },
+        )
+
+        self.assertIs(span.status.status_code, trace.StatusCode.UNSET)
+
+        self.assertEqualSpanInstrumentationInfo(
+            span, opentelemetry.instrumentation.urllib
+        )
+
     def test_uninstrument(self):
         URLLibInstrumentor().uninstrument()
         result = self.perform_request(self.URL)