Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(gunicorn): add gunicorn and gevent test scenarios #4810

Closed
wants to merge 4 commits into from
Closed

chore(gunicorn): add gunicorn and gevent test scenarios #4810

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
05c5405
Add test scenarios for supported gunicorn/gevent cases
Yun-Kim Dec 20, 2022
8d6ce11
pin gevent dependency to latest
Yun-Kim Dec 21, 2022
f17eb5e
Refactor tests, add debugger to wsgi app
Yun-Kim Jan 3, 2023
749dfac
Merge branch '1.x' into yunkim/gunicorn-test-scenarios
Yun-Kim Jan 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 18 additions & 36 deletions tests/contrib/gunicorn/test_gunicorn.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,36 +137,29 @@ def test_traced_basic(gunicorn_server_settings, gunicorn_server):
@pytest.mark.parametrize(
"gunicorn_server_settings",
[
# 1. Set `DD_GEVENT_PATCH_ALL` environment variable to true when running with ddtrace-run
_gunicorn_settings_factory(
app_path="tests.contrib.gunicorn.wsgi_mw_app:app",
worker_class="gevent",
gevent_patching=None,
gevent_patching=True,
use_ddtracerun=True,
),
# 2. Import ddtrace sitecustomize as first import of application
_gunicorn_settings_factory(
app_path="tests.contrib.gunicorn.wsgi_mw_app:app",
app_path="tests.contrib.gunicorn.gevent_wsgi_mw_app:app",
worker_class="gevent",
gevent_patching=False,
use_ddtracerun=False,
),
],
)
def test_gevent_patch_falsy_should_fail(gunicorn_server_settings, gunicorn_server):
# TODO: This test should fail as DD_GEVENT_PATCH_ALL is not set to 1.
r = gunicorn_server.get("/")
assert r.status_code == 200
assert r.content == b"Hello, World!\n"


@pytest.mark.parametrize(
"gunicorn_server_settings",
[
# 3. Use post worker init hook to import ddtrace sitecustomize
_gunicorn_settings_factory(
app_path="tests.contrib.gunicorn.wsgi_mw_app:app",
worker_class="gevent",
gevent_patching=True,
)
use_ddtracerun=False,
post_worker_init=_post_worker_init_ddtrace,
),
],
)
def test_gevent_patch_set_true_should_succeed(gunicorn_server_settings, gunicorn_server):
def test_correct_gevent_patching_should_succeed(gunicorn_server_settings, gunicorn_server):
r = gunicorn_server.get("/")
assert r.status_code == 200
assert r.content == b"Hello, World!\n"
Expand All @@ -176,30 +169,19 @@ def test_gevent_patch_set_true_should_succeed(gunicorn_server_settings, gunicorn
"gunicorn_server_settings",
[
_gunicorn_settings_factory(
app_path="tests.contrib.gunicorn.gevent_wsgi_mw_app:app",
app_path="tests.contrib.gunicorn.wsgi_mw_app:app",
worker_class="gevent",
use_ddtracerun=False,
)
],
)
def test_gevent_sitecustomize_first_import(gunicorn_server_settings, gunicorn_server):
r = gunicorn_server.get("/")
assert r.status_code == 200
assert r.content == b"Hello, World!\n"


@pytest.mark.parametrize(
"gunicorn_server_settings",
[
gevent_patching=None,
),
_gunicorn_settings_factory(
app_path="tests.contrib.gunicorn.wsgi_mw_app:app",
worker_class="gevent",
use_ddtracerun=False,
post_worker_init=_post_worker_init_ddtrace,
)
gevent_patching=False,
),
],
)
def test_gevent_post_worker_init_hook(gunicorn_server_settings, gunicorn_server):
def test_gevent_patch_falsy_should_fail(gunicorn_server_settings, gunicorn_server):
# FIXME: This test should fail as DD_GEVENT_PATCH_ALL is not set to 1.
r = gunicorn_server.get("/")
assert r.status_code == 200
assert r.content == b"Hello, World!\n"
3 changes: 3 additions & 0 deletions tests/contrib/gunicorn/wsgi_mw_app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
from ddtrace import tracer
from ddtrace.contrib.wsgi import DDWSGIMiddleware
from ddtrace.debugging import DynamicInstrumentation
from tests.webclient import PingFilter


DynamicInstrumentation.enable()
Copy link
Collaborator

@emmettbutler emmettbutler Jan 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Yun-Kim can you help me understand why you added this to the test app? I think it could play an important role in the tests I'm writing and I'm wanting to check my understanding. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@emmettbutler good question! I added Dynamic Instrumentation here while tinkering around with the test scenarios to see if we could see any unexpected behavior. If I recall correctly @P403n1x87 mentioned there were known issues with Dynamic Instrumentation with gevent, although I'm not sure if it was enabling Dynamic instrumentation alone that caused the issues.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the idea was to start a service that would register after-fork hooks that re-start background threads. DI is an example where this happens, since it requires RCM and the uploader thread to be started soon after fork. The general pattern is also in this test

dd-trace-py/tests/internal/test_forksafe.py

Line 299 in 58249cf

def test_gevent_reinit_patch():


tracer.configure(
settings={
"FILTERS": [PingFilter()],
Expand Down