diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 5dbaa0e21..4e103653e 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -43,6 +43,7 @@ jobs: - mongodb - mssql - mysql + - nginx - postgres16 - postgres9 - rabbitmq @@ -250,6 +251,69 @@ jobs: path: ./**/.coverage.* retention-days: 1 + nginx: + env: + TOTAL_GROUPS: 1 + + strategy: + fail-fast: false + matrix: + group-number: [1] + + runs-on: ubuntu-latest + container: + image: ghcr.io/newrelic/newrelic-python-agent-ci:latest + options: >- + --add-host=host.docker.internal:host-gateway + timeout-minutes: 30 + services: + nginx: + image: tpansino1643652/nginx-hello-world:latest + ports: + - 8080:8080 + - 8081:8081 + - 8082:8082 + # Set health checks to wait until nginx has started + options: >- + --health-cmd "service nginx status || exit 1" + --health-interval 10s + --health-timeout 5s + --health-retries 5 + + steps: + - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # 4.1.1 + + - name: Fetch git tags + run: | + git config --global --add safe.directory "$GITHUB_WORKSPACE" + git fetch --tags origin + + - name: Configure pip cache + run: | + mkdir -p /github/home/.cache/pip + chown -R $(whoami) /github/home/.cache/pip + + - name: Get Environments + id: get-envs + run: | + echo "envs=$(tox -l | grep '^${{ github.job }}\-' | ./.github/workflows/get-envs.py)" >> $GITHUB_OUTPUT + env: + GROUP_NUMBER: ${{ matrix.group-number }} + + - name: Test + run: | + tox -vv -e ${{ steps.get-envs.outputs.envs }} -p auto + env: + TOX_PARALLEL_NO_SPINNER: 1 + PY_COLORS: 0 + + - name: Upload Coverage Artifacts + uses: actions/upload-artifact@5d5d22a31266ced268874388b861e4b58bb5c2f3 # 4.3.1 + with: + name: coverage-${{ github.job }}-${{ strategy.job-index }} + path: ./**/.coverage.* + retention-days: 1 + postgres16: env: TOTAL_GROUPS: 2 diff --git a/newrelic/hooks/external_httpx.py b/newrelic/hooks/external_httpx.py index 5be4943c3..371330429 100644 --- a/newrelic/hooks/external_httpx.py +++ b/newrelic/hooks/external_httpx.py @@ -101,41 +101,28 @@ async def async_send_wrapper(wrapped, instance, args, kwargs): return await wrapped(*args, **kwargs) -@property -def nr_first_event_hooks(self): - if not hasattr(self, "_nr_event_hooks"): - # This branch should only be hit if agent initialize is called after - # the initialization of the http client - self._event_hooks = vars(self)["_event_hooks"] - del vars(self)["_event_hooks"] - return self._nr_event_hooks +def create_nr_first_event_hooks(is_async=False): + @property + def nr_first_event_hooks(self): + if not hasattr(self, "_nr_event_hooks"): + # This branch should only be hit if agent initialize is called after + # the initialization of the http client + self._event_hooks = vars(self)["_event_hooks"] + del vars(self)["_event_hooks"] + return self._nr_event_hooks -@nr_first_event_hooks.setter -def nr_first_event_hooks(self, value): - value = NewRelicFirstDict(value, is_async=False) - self._nr_event_hooks = value - - -@property -def nr_first_event_hooks_async(self): - if not hasattr(self, "_nr_event_hooks"): - # This branch should only be hit if agent initialize is called after - # the initialization of the http client - self._event_hooks = vars(self)["_event_hooks"] - del vars(self)["_event_hooks"] - return self._nr_event_hooks - - -@nr_first_event_hooks_async.setter -def nr_first_event_hooks_async(self, value): - value = NewRelicFirstDict(value, is_async=True) - self._nr_event_hooks = value + @nr_first_event_hooks.setter + def nr_first_event_hooks(self, value): + value = NewRelicFirstDict(value, is_async=is_async) + self._nr_event_hooks = value + + return nr_first_event_hooks def instrument_httpx_client(module): - module.Client._event_hooks = nr_first_event_hooks - module.AsyncClient._event_hooks = nr_first_event_hooks_async + module.Client._event_hooks = create_nr_first_event_hooks(is_async=False) + module.AsyncClient._event_hooks = create_nr_first_event_hooks(is_async=True) wrap_function_wrapper(module, "Client.send", sync_send_wrapper) wrap_function_wrapper(module, "AsyncClient.send", async_send_wrapper) diff --git a/tests/agent_unittests/cert.pem b/tests/agent_unittests/cert.pem index 0bbbf3a17..b4a8741cc 100644 --- a/tests/agent_unittests/cert.pem +++ b/tests/agent_unittests/cert.pem @@ -1,7 +1,7 @@ This is not a secret key! This private key is used only for testing and is not functionally used in the agent. To generate a new key and certificate, use the following. -openssl req -nodes -newkey rsa:2048 -x509 -keyout key.pem -out cert.pem -subj '/CN=localhost' -days 3650 +openssl req -nodes -newkey rsa:2048 -x509 -keyout cert.pem -out cert.pem -subj '/CN=localhost' -days 3650 -----BEGIN PRIVATE KEY----- MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQDYF0U/0zXikonW Ez532avkDL1QbQ8Yz5ULMwZz8j+cLEhdw/4pQJ7Dox6KEZbsan1nZZqpcZWT0d39 diff --git a/tests/external_httpx/conftest.py b/tests/external_httpx/conftest.py index 760e18cde..d4c83db1c 100644 --- a/tests/external_httpx/conftest.py +++ b/tests/external_httpx/conftest.py @@ -20,6 +20,7 @@ collector_agent_registration_fixture, collector_available_fixture, ) +from testing_support.db_settings import nginx_settings _default_settings = { "package_reporting.enabled": False, # Turn off package reporting for testing as it causes slow downs. @@ -40,3 +41,24 @@ def httpx(): import httpx return httpx + + +@pytest.fixture(scope="session") +def real_server(): + settings = nginx_settings()[0] + + class RealHTTP2Server: + host = settings["host"] + port = settings["port"] + + yield RealHTTP2Server + + +@pytest.fixture(scope="function") +def sync_client(httpx): + return httpx.Client() + + +@pytest.fixture(scope="function") +def async_client(httpx): + return httpx.AsyncClient() diff --git a/tests/external_httpx/test_client.py b/tests/external_httpx/test_client.py index 756f7d977..4b1b75d2a 100644 --- a/tests/external_httpx/test_client.py +++ b/tests/external_httpx/test_client.py @@ -72,24 +72,24 @@ def cat_response_handler(self): @pytest.fixture(scope="session") -def server(): +def mock_server(): external = MockExternalHTTPHResponseHeadersServer(handler=cat_response_handler) with external: yield external @pytest.fixture() -def populate_metrics(server, request): +def populate_metrics(mock_server, request): SCOPED_METRICS[:] = [] method = request.getfixturevalue("method").upper() - SCOPED_METRICS.append((f"External/localhost:{server.port}/httpx/{method}", 2)) + SCOPED_METRICS.append((f"External/localhost:{mock_server.port}/httpx/{method}", 2)) -def exercise_sync_client(server, client, method): +def exercise_sync_client(server, client, method, protocol="http"): with client as client: resolved_method = getattr(client, method) - resolved_method(f"http://localhost:{server.port}") - response = resolved_method(f"http://localhost:{server.port}") + resolved_method(f"{protocol}://{server.host}:{server.port}") + response = resolved_method(f"{protocol}://{server.host}:{server.port}") return response @@ -113,19 +113,19 @@ def exercise_sync_client(server, client, method): background_task=True, ) @background_task(name="test_sync_client") -def test_sync_client(httpx, server, method): +def test_sync_client(httpx, sync_client, mock_server, method): global CAT_RESPONSE_CODE CAT_RESPONSE_CODE = 200 - assert exercise_sync_client(server, httpx.Client(), method).status_code == 200 + assert exercise_sync_client(mock_server, sync_client, method).status_code == 200 -async def exercise_async_client(server, client, method): +async def exercise_async_client(server, client, method, protocol="http"): async with client as client: resolved_method = getattr(client, method) responses = await asyncio.gather( - resolved_method(f"http://localhost:{server.port}"), - resolved_method(f"http://localhost:{server.port}"), + resolved_method(f"{protocol}://{server.host}:{server.port}"), + resolved_method(f"{protocol}://{server.host}:{server.port}"), ) return responses @@ -150,11 +150,11 @@ async def exercise_async_client(server, client, method): background_task=True, ) @background_task(name="test_async_client") -def test_async_client(httpx, server, loop, method): +def test_async_client(httpx, async_client, mock_server, loop, method): global CAT_RESPONSE_CODE CAT_RESPONSE_CODE = 200 - responses = loop.run_until_complete(exercise_async_client(server, httpx.AsyncClient(), method)) + responses = loop.run_until_complete(exercise_async_client(mock_server, async_client, method)) assert all(response.status_code == 200 for response in responses) @@ -166,7 +166,7 @@ def test_async_client(httpx, server, loop, method): (False, False), ), ) -def test_sync_cross_process_request(httpx, server, distributed_tracing, span_events): +def test_sync_cross_process_request(httpx, sync_client, mock_server, distributed_tracing, span_events): global CAT_RESPONSE_CODE CAT_RESPONSE_CODE = 200 @@ -183,8 +183,8 @@ def test_sync_cross_process_request(httpx, server, distributed_tracing, span_eve def _test(): transaction = current_transaction() - with httpx.Client() as client: - response = client.get(f"http://localhost:{server.port}") + with sync_client: + response = sync_client.get(f"http://localhost:{mock_server.port}") transaction._test_request_headers = response.request.headers @@ -204,7 +204,7 @@ def _test(): @validate_transaction_errors(errors=[]) @background_task(name="test_async_cross_process_request") @validate_cross_process_headers -def test_async_cross_process_request(httpx, server, loop, distributed_tracing, span_events): +def test_async_cross_process_request(httpx, async_client, mock_server, loop, distributed_tracing, span_events): global CAT_RESPONSE_CODE CAT_RESPONSE_CODE = 200 @@ -215,8 +215,8 @@ def test_async_cross_process_request(httpx, server, loop, distributed_tracing, s } ) async def _test(): - async with httpx.AsyncClient() as client: - response = await client.get(f"http://localhost:{server.port}") + async with async_client: + response = await async_client.get(f"http://localhost:{mock_server.port}") return response @@ -236,14 +236,14 @@ async def _test(): ) @validate_transaction_errors(errors=[]) @background_task(name="test_sync_cross_process_override_headers") -def test_sync_cross_process_override_headers(httpx, server, loop): +def test_sync_cross_process_override_headers(httpx, sync_client, mock_server, loop): global CAT_RESPONSE_CODE CAT_RESPONSE_CODE = 200 transaction = current_transaction() - with httpx.Client() as client: - response = client.get(f"http://localhost:{server.port}", headers={"newrelic": "1234"}) + with sync_client: + response = sync_client.get(f"http://localhost:{mock_server.port}", headers={"newrelic": "1234"}) transaction._test_request_headers = response.request.headers @@ -259,13 +259,13 @@ def test_sync_cross_process_override_headers(httpx, server, loop): ) @validate_transaction_errors(errors=[]) @background_task(name="test_async_cross_process_override_headers") -def test_async_cross_process_override_headers(httpx, server, loop): +def test_async_cross_process_override_headers(httpx, async_client, mock_server, loop): global CAT_RESPONSE_CODE CAT_RESPONSE_CODE = 200 async def _test(): - async with httpx.AsyncClient() as client: - response = await client.get(f"http://localhost:{server.port}", headers={"newrelic": "1234"}) + async with async_client: + response = await async_client.get(f"http://localhost:{mock_server.port}", headers={"newrelic": "1234"}) return response @@ -277,7 +277,7 @@ async def _test(): @pytest.mark.parametrize("cat_enabled", [True, False]) @pytest.mark.parametrize("response_code", [200, 500]) -def test_sync_client_cat_response_processing(cat_enabled, response_code, server, httpx): +def test_sync_client_cat_response_processing(cat_enabled, response_code, sync_client, mock_server, httpx): global CAT_RESPONSE_CODE CAT_RESPONSE_CODE = response_code @@ -292,7 +292,7 @@ def test_sync_client_cat_response_processing(cat_enabled, response_code, server, expected_metrics = [ ( - f"ExternalTransaction/localhost:{server.port}/1#1/WebTransaction/Function/app:beep", + f"ExternalTransaction/localhost:{mock_server.port}/1#1/WebTransaction/Function/app:beep", 1 if cat_enabled else None, ), ] @@ -307,15 +307,15 @@ def test_sync_client_cat_response_processing(cat_enabled, response_code, server, @override_application_settings(_custom_settings) @background_task(name="test_sync_client_cat_response_processing") def _test(): - with httpx.Client() as client: - response = client.get(f"http://localhost:{server.port}") + with sync_client: + response = sync_client.get(f"http://localhost:{mock_server.port}") _test() @pytest.mark.parametrize("cat_enabled", [True, False]) @pytest.mark.parametrize("response_code", [200, 500]) -def test_async_client_cat_response_processing(cat_enabled, response_code, httpx, server, loop): +def test_async_client_cat_response_processing(cat_enabled, response_code, httpx, async_client, mock_server, loop): global CAT_RESPONSE_CODE CAT_RESPONSE_CODE = response_code @@ -330,7 +330,7 @@ def test_async_client_cat_response_processing(cat_enabled, response_code, httpx, expected_metrics = [ ( - f"ExternalTransaction/localhost:{server.port}/1#1/WebTransaction/Function/app:beep", + f"ExternalTransaction/localhost:{mock_server.port}/1#1/WebTransaction/Function/app:beep", 1 if cat_enabled else None, ), ] @@ -346,8 +346,8 @@ def test_async_client_cat_response_processing(cat_enabled, response_code, httpx, @background_task(name="test_async_client_cat_response_processing") def _test(): async def coro(): - async with httpx.AsyncClient() as client: - response = await client.get(f"http://localhost:{server.port}") + async with async_client: + response = await async_client.get(f"http://localhost:{mock_server.port}") return response @@ -357,7 +357,7 @@ async def coro(): @dt_enabled -def test_sync_client_event_hook_exception(httpx, server): +def test_sync_client_event_hook_exception(httpx, mock_server): global CAT_RESPONSE_CODE CAT_RESPONSE_CODE = 500 @@ -370,16 +370,16 @@ def empty_hook(response): @validate_span_events( count=1, - exact_intrinsics={"name": f"External/localhost:{server.port}/httpx/GET"}, + exact_intrinsics={"name": f"External/localhost:{mock_server.port}/httpx/GET"}, exact_agents={"http.statusCode": CAT_RESPONSE_CODE}, ) @background_task(name="test_sync_client_event_hook_exception") def make_request(client, exc_expected=True): if exc_expected: with pytest.raises(RuntimeError): - client.get(f"http://localhost:{server.port}") + client.get(f"http://localhost:{mock_server.port}") else: - client.get(f"http://localhost:{server.port}") + client.get(f"http://localhost:{mock_server.port}") with httpx.Client(event_hooks={"response": [exception_event_hook]}) as client: # Test client init @@ -403,7 +403,7 @@ def make_request(client, exc_expected=True): @override_application_settings({"distributed_tracing.enabled": True, "span_events.enabled": True}) -def test_async_client_event_hook_exception(httpx, server, loop): +def test_async_client_event_hook_exception(httpx, mock_server, loop): global CAT_RESPONSE_CODE CAT_RESPONSE_CODE = 500 @@ -416,7 +416,7 @@ def empty_hook(response): @validate_span_events( count=1, - exact_intrinsics={"name": f"External/localhost:{server.port}/httpx/GET"}, + exact_intrinsics={"name": f"External/localhost:{mock_server.port}/httpx/GET"}, exact_agents={"http.statusCode": CAT_RESPONSE_CODE}, ) @background_task(name="test_sync_client_event_hook_exception") @@ -424,9 +424,9 @@ def make_request(client, exc_expected=True): async def coro(): if exc_expected: with pytest.raises(RuntimeError): - await client.get(f"http://localhost:{server.port}") + await client.get(f"http://localhost:{mock_server.port}") else: - await client.get(f"http://localhost:{server.port}") + await client.get(f"http://localhost:{mock_server.port}") loop.run_until_complete(coro()) @@ -451,20 +451,19 @@ def _test(): client.event_hooks = {"request": [empty_hook]} make_request(client, exc_expected=False) - @override_generic_settings( global_settings(), { "enabled": False, }, ) -def test_sync_nr_disabled(httpx, server): +def test_sync_nr_disabled(httpx, mock_server): global CAT_RESPONSE_CODE CAT_RESPONSE_CODE = 200 with httpx.Client() as client: trace = current_trace() - response = client.get(f"http://localhost:{server.port}") + response = client.get(f"http://localhost:{mock_server.port}") assert response.status_code == 200 assert trace is None @@ -476,13 +475,13 @@ def test_sync_nr_disabled(httpx, server): "enabled": False, }, ) -def test_async_nr_disabled(httpx, server, loop): +def test_async_nr_disabled(httpx, mock_server, loop): global CAT_RESPONSE_CODE CAT_RESPONSE_CODE = 200 async def _test(): async with httpx.AsyncClient() as client: - response = await client.get(f"http://localhost:{server.port}") + response = await client.get(f"http://localhost:{mock_server.port}") return response @@ -499,7 +498,7 @@ async def _test(): "AsyncClient", ), ) -def test_invalid_import_order_client(monkeypatch, httpx, server, loop, client): +def test_invalid_import_order_client(monkeypatch, httpx, mock_server, loop, client): global CAT_RESPONSE_CODE CAT_RESPONSE_CODE = 200 @@ -520,8 +519,44 @@ def test_invalid_import_order_client(monkeypatch, httpx, server, loop, client): monkeypatch.undo() if is_async: - responses = loop.run_until_complete(exercise_async_client(server, client, "get")) + responses = loop.run_until_complete(exercise_async_client(mock_server, client, "get")) assert all(response.status_code == 200 for response in responses) else: - response = exercise_sync_client(server, client, "get") + response = exercise_sync_client(mock_server, client, "get") assert response.status_code == 200 + + +@validate_transaction_metrics( + "test_sync_client_http2", + scoped_metrics=SCOPED_METRICS, + rollup_metrics=ROLLUP_METRICS, + background_task=True, +) +@background_task(name="test_sync_client_http2") +def test_sync_client_http2(httpx, real_server): + global CAT_RESPONSE_CODE + CAT_RESPONSE_CODE = 200 + + client = httpx.Client(http1=False, http2=True, verify=False) + response = exercise_sync_client(real_server, client, "get", protocol="https") + + assert response.status_code == 200 + assert response.http_version in {"HTTP/2", "HTTP/2.0"} + + +@validate_transaction_metrics( + "test_async_client_http2", + scoped_metrics=SCOPED_METRICS, + rollup_metrics=ROLLUP_METRICS, + background_task=True, +) +@background_task(name="test_async_client_http2") +def test_async_client_http2(httpx, real_server, loop): + global CAT_RESPONSE_CODE + CAT_RESPONSE_CODE = 200 + + client = httpx.AsyncClient(http1=False, http2=True, verify=False) + + responses = loop.run_until_complete(exercise_async_client(real_server, client, "get", protocol="https")) + assert all(response.status_code == 200 for response in responses) + assert all(response.http_version in {"HTTP/2", "HTTP/2.0"} for response in responses) diff --git a/tests/testing_support/db_settings.py b/tests/testing_support/db_settings.py index 4de2cb661..e2d847235 100644 --- a/tests/testing_support/db_settings.py +++ b/tests/testing_support/db_settings.py @@ -356,3 +356,26 @@ def gearman_settings(): for instance_num in range(instances) ] return settings + + +def nginx_settings(): + """Return a list of dict of settings for connecting to nginx. + + Will return the correct settings, depending on which of the environments it + is running in. It attempts to set variables in the following order, where + later environments override earlier ones. + + 1. Local + 2. Github Actions + """ + + host = "host.docker.internal" if "GITHUB_ACTIONS" in os.environ else "localhost" + instances = 1 + settings = [ + { + "host": host, + "port": 8080 + instance_num, + } + for instance_num in range(instances) + ] + return settings diff --git a/tests/testing_support/mock_external_http_server.py b/tests/testing_support/mock_external_http_server.py index d8858816a..c76abd14e 100644 --- a/tests/testing_support/mock_external_http_server.py +++ b/tests/testing_support/mock_external_http_server.py @@ -41,6 +41,7 @@ class MockExternalHTTPServer(threading.Thread): # calls. For an example see: # ../framework_tornado_r3/test_async_application.py RESPONSE = b'external response' + host = "localhost" def __init__(self, handler=simple_get, port=None, *args, **kwargs): super(MockExternalHTTPServer, self).__init__(*args, **kwargs) diff --git a/tox.ini b/tox.ini index 8493d6b41..861ea93f2 100644 --- a/tox.ini +++ b/tox.ini @@ -65,6 +65,7 @@ envlist = mssql-datastore_pymssql-{py37,py38,py39,py310,py311,py312,py313}, mysql-datastore_mysql-mysqllatest-{py37,py38,py39,py310,py311,py312,py313}, mysql-datastore_pymysql-{py37,py38,py39,py310,py311,py312,py313,pypy310}, + nginx-external_httpx-{py37,py38,py39,py310,py311,py312,py313}, postgres16-datastore_asyncpg-{py37,py38,py39,py310,py311,py312}, ;; Package not ready for Python 3.13 (missing wheel) ; postgres16-datastore_asyncpg-py313, @@ -120,7 +121,6 @@ envlist = python-external_http-{py37,py38,py39,py310,py311,py312,py313}, python-external_httplib-{py37,py38,py39,py310,py311,py312,py313,pypy310}, python-external_httplib2-{py37,py38,py39,py310,py311,py312,py313,pypy310}, - python-external_httpx-{py37,py38,py39,py310,py311,py312,py313}, python-external_requests-{py37,py38,py39,py310,py311,py312,py313,pypy310}, python-external_urllib3-{py37,py38,py39,py310,py311,py312,py313,pypy310}-urllib3latest, python-external_urllib3-{py37,py312,py313,pypy310}-urllib30126, @@ -305,7 +305,7 @@ deps = external_botocore: moto external_feedparser-feedparser06: feedparser<7 external_httplib2: httplib2<1.0 - external_httpx: httpx<0.17 + external_httpx: httpx[http2] external_requests: urllib3 external_requests: requests external_urllib3-urllib30126: urllib3<1.27