fix(requests): set correct service name when using split_by_domain (backport #4067) (#4091)

This is an automatic backport of pull request #4067 done by [Mergify](https://mergify.com).


---


<details>
<summary>Mergify commands and options</summary>

<br />

More conditions and actions can be found in the [documentation](https://docs.mergify.com/).

You can also trigger Mergify actions by commenting on this pull request:

- `@Mergifyio refresh` will re-evaluate the rules
- `@Mergifyio rebase` will rebase this PR on its base branch
- `@Mergifyio update` will merge the base branch into this PR
- `@Mergifyio backport <destination>` will backport this PR on `<destination>` branch

Additionally, on Mergify [dashboard](https://dashboard.mergify.com/) you can:

- look at your merge queues
- generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com
</details>

Author: mergify[bot]

ddtrace/contrib/requests/connection.py

@@ -7,6 +7,7 @@
 from ...constants import ANALYTICS_SAMPLE_RATE_KEY
 from ...constants import SPAN_MEASURED_KEY
 from ...ext import SpanTypes
+from ...internal.compat import parse
 from ...internal.logger import get_logger
 from ...internal.utils import get_argument_value
 from ...propagation.http import HTTPPropagator
@@ -17,23 +18,17 @@
 
 def _extract_hostname(uri):
     # type: (str) -> str
-    end = len(uri)
-    j = uri.rfind("#", 0, end)
-    if j != -1:
-        end = j
-    j = uri.rfind("&", 0, end)
-    if j != -1:
-        end = j
-
-    start = uri.find("://", 0, end) + 3
-    i = uri.find("@", start, end) + 1
-    if i != 0:
-        start = i
-    j = uri.find("/", start, end)
-    if j != -1:
-        end = j
-
-    return uri[start:end]
+    parsed_uri = parse.urlparse(uri)
+    port = None
+    try:
+        port = parsed_uri.port
+    except ValueError:
+        # ValueError is raised in PY>3.5 when parsed_uri.port < 0 or parsed_uri.port > 65535
+        return "%s:?" % (parsed_uri.hostname,)
+
+    if port is not None:
+        return "%s:%s" % (parsed_uri.hostname, str(port))
+    return parsed_uri.hostname
 
 
 def _extract_query_string(uri):

releasenotes/notes/fix-split-by-domain-requests-978e1f51a5b3f77d.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - |
+    requests: fix split-by-domain service name when multiple ``@`` signs are present in the url

tests/contrib/requests/test_requests.py

@@ -302,6 +302,24 @@ def test_user_service_name_precedence(self):
         assert s.name == "requests.request"
         assert s.service == "clients"
 
+    def test_split_by_domain_with_ampersat(self):
+        # Regression test for: https://github.com/DataDog/dd-trace-py/issues/4062
+        # ensure a service name is generated by the domain name
+        cfg = config.get_from(self.session)
+        cfg["split_by_domain"] = True
+        # domain name should take precedence over monkey_service
+        cfg["service_name"] = "monkey_service"
+        url = URL_200 + "?email=monkey_monkey@zoo_mail.ca"
+
+        out = self.session.get(url)
+        assert out.status_code == 200
+
+        spans = self.pop_spans()
+        assert len(spans) == 1
+        s = spans[0]
+
+        assert s.service == "httpbin.org"
+
     def test_split_by_domain(self):
         # ensure a service name is generated by the domain name
         # of the ongoing call
@@ -594,6 +612,13 @@ def test_extract_hostname(uri, hostname):
     assert _extract_hostname(uri) == hostname
 
 
+def test_extract_hostname_invalid_port():
+    if sys.version_info < (3, 6):
+        assert _extract_hostname("http://localhost:-1/") == "localhost"
+    else:
+        assert _extract_hostname("http://localhost:-1/") == "localhost:?"
+
+
 @pytest.mark.parametrize(
     "uri,qs",
     [