fix issue in httpx instrumentation when request is a kwarg (#1337)

* fix issue in httpx instrumentation when request is a kwarg

The issue is that some_dict.get("x", alt) doesn't short-circuit,
so alt is always evaluated.

closes #1336

* update changelog

* fix CHANGELOG

CHANGELOG.asciidoc

@@ -28,7 +28,7 @@ endif::[]
 //
 //[float]
 //===== Bug fixes
-//
+
 
 === Unreleased
 
@@ -42,7 +42,9 @@ endif::[]
 ===== Bug fixes
 
 * Improve span coverage for `asyncpg` {pull}1328[#1328]
-* aiohttp: Correctly pass custom client to tracing middleware {pull}1345[#1345]
+* aiohttp: Correctly pass custom client to tracing middleware {pull}1345[#1345] 
+* Fixed an issue with httpx instrumentation {pull}1337[#1337]
+
 
 [[release-notes-6.x]]
 === Python Agent version 6.x

elasticapm/instrumentation/packages/asyncio/httpx.py

@@ -39,7 +39,7 @@ class HttpxAsyncClientInstrumentation(AsyncAbstractInstrumentedModule):
     instrument_list = [("httpx", "AsyncClient.send")]
 
     async def call(self, module, method, wrapped, instance, args, kwargs):
-        request = kwargs.get("request", args[0])
+        request = kwargs.get("request") or args[0]
 
         request_method = request.method.upper()
         url = str(request.url)

elasticapm/instrumentation/packages/httpx.py

@@ -39,7 +39,7 @@ class HttpxClientInstrumentation(AbstractInstrumentedModule):
     instrument_list = [("httpx", "Client.send")]
 
     def call(self, module, method, wrapped, instance, args, kwargs):
-        request = kwargs.get("request", args[0])
+        request = kwargs.get("request") or args[0]
 
         request_method = request.method.upper()
         url = str(request.url)

tests/instrumentation/asyncio_tests/httpx_tests.py

@@ -168,3 +168,22 @@ async def test_httpx_error(instrument, elasticapm_client, waiting_httpserver, st
     assert spans[0]["context"]["http"]["url"] == url
     assert spans[0]["context"]["http"]["status_code"] == status_code
     assert spans[0]["outcome"] == "failure"
+
+
+async def test_httpx_streaming(instrument, elasticapm_client, waiting_httpserver):
+    # client.stream passes the request as a keyword argument to the instrumented method.
+    # This helps test that we can handle it both as an arg and a kwarg
+    # see https://github.com/elastic/apm-agent-python/issues/1336
+    elasticapm_client.begin_transaction("httpx-context-manager-client-streaming")
+    client = httpx.AsyncClient()
+    try:
+        async with client.stream("GET", url=waiting_httpserver.url) as response:
+            assert response
+    finally:
+        await client.aclose()
+        elasticapm_client.end_transaction("httpx-context-manager-client-streaming")
+
+    transactions = elasticapm_client.events[TRANSACTION]
+    span = elasticapm_client.spans_for_transaction(transactions[0])[0]
+    assert span["type"] == "external"
+    assert span["subtype"] == "http"

tests/instrumentation/httpx_tests.py

@@ -167,3 +167,22 @@ def test_httpx_error(instrument, elasticapm_client, waiting_httpserver, status_c
     assert spans[0]["context"]["http"]["url"] == url
     assert spans[0]["context"]["http"]["status_code"] == status_code
     assert spans[0]["outcome"] == "failure"
+
+
+def test_httpx_streaming(instrument, elasticapm_client, waiting_httpserver):
+    # client.stream passes the request as a keyword argument to the instrumented method.
+    # This helps test that we can handle it both as an arg and a kwarg
+    # see https://github.com/elastic/apm-agent-python/issues/1336
+    elasticapm_client.begin_transaction("httpx-context-manager-client-streaming")
+    client = httpx.Client()
+    try:
+        with client.stream("GET", url=waiting_httpserver.url) as response:
+            assert response
+    finally:
+        client.close()
+        elasticapm_client.end_transaction("httpx-context-manager-client-streaming")
+
+    transactions = elasticapm_client.events[TRANSACTION]
+    span = elasticapm_client.spans_for_transaction(transactions[0])[0]
+    assert span["type"] == "external"
+    assert span["subtype"] == "http"