Add integration tests for ASGI Django 3 by Kyle-Verhoog · Pull Request #2640 · DataDog/dd-trace-py

Kyle-Verhoog · 2021-07-13T00:43:03Z

Add integration tests which run a real Django instance using ASGI
and daphne.

A new function-scope daphne_client fixture is introduced which
initializes a real daphne Django server using the one in our
test suite. The client fixture provides an API to query the server much
like the pytest Django client.

Traces from the test cases are captured with the test agent and
snapshotted. Traces produced as a part of setup and teardown
are sent with distributed tracing headers which assign a trace id
of 1 which an installed TraceFilter will filter out.

Any temporal dependency is removed by adding a special shutdown
endpoint to the Django app which does a tracer.shutdown() to flush
the traces for the test case.

Kyle-Verhoog · 2021-07-16T13:39:18Z

Introduce an endpoint to force a flush of the traces to reduce this flakiness. Thanks @P403n1x87!

Interesting idea! We'd want to disable tracing for such an endpoint so we don't end up getting it's trace in the snapshot as well.

Another option would be to have the client fixture be per test such that flushing occurs during shutdown.

We'd want to disable tracing for such an endpoint so we don't end up getting it's trace in the snapshot as well.

Ah yes... good point!

Another option would be to have the client fixture be per test such that flushing occurs during shutdown.

yes - this would be more ideal IMO but there is a non-trivial cost of starting up the webserver unfortunately... although we don't run a lot of test scenarios

glad to say that all the time.sleep()s have been purged! Instead,

the django instance is started per process

traces are sent with a test token directly thanks to Add internal setting for additional writer headers #2689

polling spans are filtered by using http propagation headers with a special trace id

TraceFilter filters traces with the special trace id

traces are force-flushed with a call to a special endpoint

Add integration tests which run a real Django instance using ASGI and daphne.

For testing it's desirable to be able to pass additional headers with the trace requests made to the agent. With this ability we can propagate tokens which can be used to associate traces with test cases when running snapshot tests in subprocesses (like in DataDog#2640). I debated having the flag be read at run-time instead of initialization-time but opted against it as reading from the env is not cheap (on the order of 1µs): >>> %timeit os.environ.get("") 1.23 µs ± 49.6 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

For testing it's desirable to be able to pass additional headers with the trace requests made to the agent. With this ability we can propagate tokens which can be used to associate traces with test cases when running snapshot tests in subprocesses (like in #2640). I debated having the flag be read at run-time instead of initialization-time but opted against it as reading from the env is not cheap (on the order of 1µs): >>> %timeit os.environ.get("") 1.23 µs ± 49.6 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each) Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

brettlangdon · 2021-07-22T21:08:09Z

+from ddtrace.contrib.asgi import TraceMiddleware
+
+
+application = TraceMiddleware(get_asgi_application())


is this what we are recommending for customers to do now?

A bit of a long story.... because of #2559 there's no way to be able to identify traces as coming from a particular route (no route tagging, no http propagation, no query params, etc) in order to filter traces that we don't care about (polling, startup, teardown, etc).

I added in the ASGI middleware to mitigate this for now so that we don't get a bunch of flaky time.sleep()s everywhere.

I think it's ok because the ASGI spans just wrap the Django ones. With #2656 we'll remove this explicit usage.

Avoid the redirect

mabdinur · 2021-07-26T13:21:56Z

LGTM

Kyle-Verhoog added the changelog/no-changelog A changelog entry is not required for this PR. label Jul 13, 2021

Kyle-Verhoog force-pushed the django-integration branch 11 times, most recently from 5a1c530 to ee17085 Compare July 16, 2021 02:02

Kyle-Verhoog marked this pull request as ready for review July 16, 2021 13:37

Kyle-Verhoog requested a review from a team as a code owner July 16, 2021 13:37

Kyle-Verhoog commented Jul 16, 2021

View reviewed changes

Kyle-Verhoog force-pushed the django-integration branch 6 times, most recently from dab0c2e to b9dc65c Compare July 20, 2021 22:52

Add integration tests for ASGI Django 3

e4a1c02

Add integration tests which run a real Django instance using ASGI and daphne.

Kyle-Verhoog force-pushed the django-integration branch 4 times, most recently from 2d3ee99 to 0bc3679 Compare July 21, 2021 18:35

Kyle-Verhoog mentioned this pull request Jul 21, 2021

Add internal setting for additional writer headers #2689

Merged

brettlangdon reviewed Jul 22, 2021

View reviewed changes

Comment thread ddtrace/internal/writer.py Outdated

Comment thread tests/snapshots/tests.contrib.django.test_django_snapshots.test_asgi_200_31.snap Outdated

Comment thread tests/snapshots/tests.contrib.django.test_django_snapshots.test_asgi_200_3x.snap Outdated

Kyle-Verhoog added 2 commits July 22, 2021 10:54

Add pytest snapshot fixture

3092962

Merge remote-tracking branch 'origin/master' into django-integration

ceed6ca

Kyle-Verhoog force-pushed the django-integration branch from 1baaba9 to ceed6ca Compare July 22, 2021 14:57

mergify Bot removed the conflict label Jul 22, 2021

Shutdown tracer on test finish

6f399d1

Kyle-Verhoog force-pushed the django-integration branch from a7a71a1 to 6f399d1 Compare July 22, 2021 17:16

Use asgi middleware, add ignore route

cceca3a

Kyle-Verhoog force-pushed the django-integration branch from 23fd044 to cceca3a Compare July 22, 2021 20:30

brettlangdon reviewed Jul 22, 2021

View reviewed changes

Fix snapshot

3cab2b9

Kyle-Verhoog force-pushed the django-integration branch from a9838ac to 3cab2b9 Compare July 22, 2021 21:49

Kyle-Verhoog commented Jul 22, 2021

View reviewed changes

Comment thread tests/snapshots/tests.contrib.django.test_django_snapshots.test_asgi_500_3x.snap

Trailing slash

9267520

Avoid the redirect

Kyle-Verhoog requested review from brettlangdon and majorgreys July 22, 2021 22:34

P403n1x87 previously approved these changes Jul 23, 2021

View reviewed changes

mergify Bot added 2 commits July 23, 2021 08:36

Merge branch 'master' into django-integration

19050c1

Merge branch 'master' into django-integration

dab2542

brettlangdon reviewed Jul 23, 2021

View reviewed changes

Remove old configuration, add docs

771ed40

Kyle-Verhoog dismissed P403n1x87’s stale review via 771ed40 July 23, 2021 18:17

brettlangdon approved these changes Jul 23, 2021

View reviewed changes

Kyle-Verhoog requested a review from P403n1x87 July 26, 2021 12:32

mabdinur approved these changes Jul 26, 2021

View reviewed changes

mergify Bot merged commit fddcd82 into DataDog:master Jul 26, 2021

Kyle-Verhoog deleted the django-integration branch July 26, 2021 14:05

Kyle-Verhoog restored the django-integration branch May 10, 2022 04:32

		from ddtrace.contrib.asgi import TraceMiddleware


		application = TraceMiddleware(get_asgi_application())

Conversation

Kyle-Verhoog commented Jul 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kyle-Verhoog Jul 16, 2021

Choose a reason for hiding this comment

Uh oh!

majorgreys Jul 16, 2021

Choose a reason for hiding this comment

Uh oh!

majorgreys Jul 16, 2021

Choose a reason for hiding this comment

Uh oh!

Kyle-Verhoog Jul 16, 2021

Choose a reason for hiding this comment

Uh oh!

Kyle-Verhoog Jul 22, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brettlangdon Jul 22, 2021

Choose a reason for hiding this comment

Uh oh!

Kyle-Verhoog Jul 22, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mabdinur commented Jul 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Kyle-Verhoog commented Jul 13, 2021 •

edited

Loading