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.

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
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
2 changes: 1 addition & 1 deletion riotfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -2589,7 +2589,7 @@ def select_pys(min_version=MIN_PYTHON_VERSION, max_version=MAX_PYTHON_VERSION):
Venv(
name="gunicorn",
command="pytest {cmdargs} tests/contrib/gunicorn",
pkgs={"requests": latest},
pkgs={"requests": latest, "gevent": latest},
venvs=[
Venv(
pys="2.7",
Expand Down
23 changes: 23 additions & 0 deletions tests/contrib/gunicorn/gevent_wsgi_mw_app.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import ddtrace.bootstrap.sitecustomize # noqa: F401 # isort: skip
from ddtrace import tracer # noqa: I001
from ddtrace.contrib.wsgi import DDWSGIMiddleware
from tests.webclient import PingFilter


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


def simple_app(environ, start_response):
if environ["RAW_URI"] == "/shutdown":
tracer.shutdown()

data = b"Hello, World!\n"
start_response("200 OK", [("Content-Type", "text/plain"), ("Content-Length", str(len(data)))])
return iter([data])


app = DDWSGIMiddleware(simple_app)
57 changes: 57 additions & 0 deletions tests/contrib/gunicorn/test_gunicorn.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import sys
from typing import Dict
from typing import NamedTuple
from typing import Optional

import pytest
import tenacity
Expand Down Expand Up @@ -41,9 +42,12 @@ def _gunicorn_settings_factory(
bind="0.0.0.0:8080", # type: str
use_ddtracerun=True, # type: bool
post_worker_init="", # type: str
gevent_patching=None, # type: Optional[bool]
):
# type: (...) -> GunicornServerSettings
"""Factory for creating gunicorn settings with simple defaults if settings are not defined."""
if gevent_patching is not None:
env["DD_GEVENT_PATCH_ALL"] = str(gevent_patching)
return GunicornServerSettings(
env=env,
directory=directory,
Expand Down Expand Up @@ -128,3 +132,56 @@ def test_traced_basic(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",
[
# 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=True,
use_ddtracerun=True,
),
# 2. Import ddtrace sitecustomize as first import of application
_gunicorn_settings_factory(
app_path="tests.contrib.gunicorn.gevent_wsgi_mw_app:app",
worker_class="gevent",
use_ddtracerun=False,
),
# 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",
use_ddtracerun=False,
post_worker_init=_post_worker_init_ddtrace,
),
],
)
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"


@pytest.mark.parametrize(
"gunicorn_server_settings",
[
_gunicorn_settings_factory(
app_path="tests.contrib.gunicorn.wsgi_mw_app:app",
worker_class="gevent",
gevent_patching=None,
),
_gunicorn_settings_factory(
app_path="tests.contrib.gunicorn.wsgi_mw_app:app",
worker_class="gevent",
gevent_patching=False,
),
],
)
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
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

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

def test_gevent_reinit_patch():


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