Catch TypeError if port is not set (#567)

If the port not found in the parse result the instrumentation will fail with
`int() argument must be a string, a bytes-like object or a number, not 'NoneType'`

closes #567

Author: Azd325

CHANGELOG.md

@@ -7,6 +7,7 @@
  * fixed an issue with empty responses from APM Server's config endpoint (#562, #563)
  * fixed Windows tests by avoiding time.sleep in breakdown metrics tests (#537, #550)
  * fixed container ID matching to match CloudFoundry Garden container IDs (#523, #564)
+ * fixed an issue in the urllib instrumentation if no port is set (#567)
 
 ### Other
  * Added Python 3.8 RC to the test matrix (#565)

elasticapm/instrumentation/packages/urllib.py

@@ -49,12 +49,12 @@ def request_host(request):
     scheme, host, port = parse_result.scheme, parse_result.hostname, parse_result.port
     try:
         port = int(port)
-    except ValueError:
+    except (ValueError, TypeError):
         pass
     if host == "":
         host = request.get_header("Host", "")
 
-    if port != default_ports.get(scheme):
+    if port and port != default_ports.get(scheme):
         host = "%s:%s" % (host, port)
     return host
 

tests/instrumentation/urllib_tests.py

@@ -28,6 +28,7 @@
 #  OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 #  OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
+import mock
 import pytest
 
 from elasticapm.conf import constants
@@ -39,10 +40,16 @@
 try:
     from urllib.request import urlopen
     from urllib.error import URLError
+
+    request_method = "http.client.HTTPConnection.request"
+    getresponse_method = "http.client.HTTPConnection.getresponse"
 except ImportError:
     from urllib2 import urlopen
     from urllib2 import URLError
 
+    request_method = "httplib.HTTPConnection.request"
+    getresponse_method = "httplib.HTTPConnection.getresponse"
+
 
 def test_urllib(instrument, elasticapm_client, waiting_httpserver):
     waiting_httpserver.serve_content("")
@@ -77,6 +84,30 @@ def test_urllib(instrument, elasticapm_client, waiting_httpserver):
     assert spans[1]["parent_id"] == transactions[0]["id"]
 
 
+@mock.patch(request_method)
+@mock.patch(getresponse_method)
+def test_urllib_standard_port(mock_getresponse, mock_request, instrument, elasticapm_client):
+    # "code" is needed for Python 3, "status" for Python 2
+    mock_getresponse.return_value = mock.Mock(code=200, status=200)
+
+    url = "http://example.com/"
+    parsed_url = urlparse.urlparse(url)
+    elasticapm_client.begin_transaction("transaction")
+    expected_sig = "GET {0}".format(parsed_url.netloc)
+    r = urlopen(url)
+
+    elasticapm_client.end_transaction("MyView")
+
+    transactions = elasticapm_client.events[TRANSACTION]
+    spans = elasticapm_client.spans_for_transaction(transactions[0])
+
+    assert spans[0]["name"] == expected_sig
+    assert spans[0]["type"] == "external"
+    assert spans[0]["subtype"] == "http"
+    assert spans[0]["context"]["http"]["url"] == url
+    assert spans[0]["parent_id"] == transactions[0]["id"]
+
+
 def test_trace_parent_propagation_sampled(instrument, elasticapm_client, waiting_httpserver):
     waiting_httpserver.serve_content("")
     url = waiting_httpserver.url + "/hello_world"