[flask] rewrite Flask integration #667

brettlangdon · 2018-10-23T13:31:18Z

We have decided to rewrite our Flask integration to provide more visibility into the Flask internals as well as to add some auto-instrumentation around user generated code (views, error handlers, etc).

Changes:

Trace Flask internals rather than use signals/middleware to trace
Add tracing to request lifecycle (request, preprocess, dispatch, postprocess, etc)
Add tracing to signals (request_started, request_finished, etc)
Add tracing to hooks (before_request, after_request, after_this_request, etc)
Add tracing to response helpers (jsonify, send_file, etc)
Add tracing to all routes, views, endpoints added to the Flask app
Enable Flask tracing by default and move to patch on import

Before

After

Co-Authored-By: brettlangdon <me@brett.is>

…py into brettlangdon/flask-dev

brettlangdon · 2018-10-30T18:04:21Z

@Kyle-Verhoog I should have addressed all of your comments :)

brettlangdon · 2018-10-30T18:05:10Z

ugh, Github is really bad, I think there are a bunch of comments I missed...

brettlangdon · 2018-10-30T18:08:31Z

ddtrace/pin.py

@@ -56,6 +56,25 @@ def __repr__(self):
        return "Pin(service=%s, app=%s, app_type=%s, tags=%s, tracer=%s)" % (
            self.service, self.app, self.app_type, self.tags, self.tracer)

+    @staticmethod
+    def find(*objs):


@Kyle-Verhoog and @palazzem

I found myself writing a helper get_inherited_pin() with the idea that there was an order to which I would look-up a Pin, either in the wrapped, instance, or maybe flask.current_app, etc.

Not sure if this is a public API we care to expose, seemed "harmless" to add, and potentially useful as we broaden the discussion about pin/config overriding.

totally happy to either change my code to not need something like this, or to move it back into a flask specific helper.

I think the API is fine to have on the Pin. However, I think we can keep it private for now since we do document the Pin publicly: http://pypi.datadoghq.com/trace/docs/advanced_usage.html#pin.

But we are going to be deprecating the public Pin api soon anyway.

Co-Authored-By: brettlangdon <me@brett.is>

…py into brettlangdon/flask-dev

Kyle-Verhoog

Great! ❤️ the tests. They are definitely a model of what our future integrations should look lik.e They give me confidence in the integration.

Just some left over nits, but it's good to go in my opinion. 👍

ddtrace/pin.py

tests/contrib/flask/test_request.py

Kyle-Verhoog · 2018-10-31T16:29:32Z

tests/contrib/flask/test_signals.py

+            signal.send(*args, **kwargs)
+            return func
+        finally:
+            # DEV: There is a bug in `blinker.Signal.disconnect` :(


Is there a link to a bug report that we can reference here?

there isn't, I should probably open one.

the TL;DR; it has something to do with how they create weak references and when you disconnect they can't find the original function in their list... something like that.

clutchski · 2018-10-31T18:53:47Z

@brettlangdon i'd suggest trimming anything that won't be interesting (e.g. dispatch request might never be interesting whereas the method it callsapp.app_index is always interesting)

brettlangdon · 2018-11-01T14:34:52Z

@Kyle-Verhoog this needs one finally approval... approval gets reset whenever a commit is pushed.

… well

* [flask] start work on new patch implementation * [flask] trace more internal parts * [flask] trace more things * [flask] round out prototype of new tracing * [flask] replace old patch.py with new_patch.py * [flask] finish up prototype and deduplicate some code * [flask] move wrapper helpers to wrappers.py * [flask] add docstrings * [flask] remove unused import * [flask] Update documentation * [flask] < 0.12.0 does not have Flask.finalize_request * [flask] handle status code as a string * [flask] use config API * [flask] update version parsing * [flask] patch signal receivers_for and add unpatch() * [flask] add test for Flask signals * [flask] fix patching/unpatching lists * [flask] use template name as span resource name * [flask] set test template directory * [flask] add test cases for Flask helpers * [flask] add test cases for Flask render functions * [flask] add test helpers to check if something is wrapped * [flask] simplify pin cloning logic * [flask] add blueprint tests * [flask] rename patch.py to monkey.py * [flask] make sure do to do Pin(tracer=self.tracer) in tests * [flask] fix spelling * [flask] add patch/unpatch idempotency tests * [flask] add initial support for v0.9 * [flask] update tests for v0.9 * [flask] use <= (0, 9) instead of == (0, 9) * [flask] fix signal names * [flask] add assertion message * [flask] skip send_from_directory * [flask] add find_span_by_name test helper * [flask] add error handler test cases * [flask] Use start_response instead of Flask.finalize_request for response code * [flask] assert bytes equality * [flask] add test caes for flask.views.View * [flask] enable by default * [flask] remove large TODO comment * [flask] change 404 resource name to '<method> 404' * [flask] fix py2 vs py3 base exception name * [flask] add request lifecycle tests * [flask] support unicode * [flask] rewrite Flask autopatch tests * [flask] run py27-flask09 tests in circleci * [flask] add static file tests * [flask] rename monkey.py back to patch.py * [flask] update docstring for flask being enabled by default * [flask] fix comments and docstrings * [flask] use ddtrace.utils.importlib.func_name * [core] modify Pin.get_from to accept multiple objects * [flask] fix remaining get_arg_or_kwargs * [flask] only use '<method> 404' if the endpoint is unknown * [flask] use request.path for http.URL tag * [flask] remove/fix TODOs * [flask] Add Pin.find(*objs) helper * [flask] only use 'def _wrap()' where necessary * [flask] mark 5xx errors by default, allow config of additional error codes * Update tests/contrib/flask/test_request.py Co-Authored-By: brettlangdon <me@brett.is> * Update tests/contrib/flask/test_template.py Co-Authored-By: brettlangdon <me@brett.is> * Update tests/contrib/flask/test_template.py Co-Authored-By: brettlangdon <me@brett.is> * [flask] simplify fetching wrapped arg * [flask] fix spelling mistake * Update ddtrace/contrib/flask/patch.py Co-Authored-By: brettlangdon <me@brett.is> * [flask] remove unnecessary 'func_name(func)' call * Update tests/contrib/flask/test_blueprint.py Co-Authored-By: brettlangdon <me@brett.is> * [flask] fix remaining comments from kyle * [flask] fix flake8 issues * [flask] test distributed tracing * [flask] add find_span_parent helper * [flask] add hook test cases * [flask] fix spelling * [flask] rename Pin.find to Pin._find and add tests * [flask] one last Pin.find -> Pin._find * [flask] try to parse endpoint/url rule in Flask.preprocess_request as well * [flask] fix line too long issue

brettlangdon added 30 commits October 18, 2018 14:20

[flask] start work on new patch implementation

0e87554

[flask] trace more internal parts

4c2d0d6

[flask] trace more things

9de028e

[flask] round out prototype of new tracing

6a1a121

[flask] replace old patch.py with new_patch.py

15a2762

[flask] finish up prototype and deduplicate some code

93953ea

[flask] move wrapper helpers to wrappers.py

ad8d1d4

[flask] add docstrings

75b8720

[flask] remove unused import

688448d

[flask] Update documentation

3c2e040

[flask] < 0.12.0 does not have Flask.finalize_request

369a15f

[flask] handle status code as a string

2978926

[flask] use config API

5a8540e

[flask] update version parsing

db3a626

[flask] patch signal receivers_for and add unpatch()

f9ab7d2

[flask] add test for Flask signals

54d3463

[flask] fix patching/unpatching lists

6d72c76

[flask] use template name as span resource name

bf2a1d1

[flask] set test template directory

a899b67

[flask] add test cases for Flask helpers

6d80afd

[flask] add test cases for Flask render functions

805e510

[flask] add test helpers to check if something is wrapped

0cfb66a

[flask] simplify pin cloning logic

3b99172

[flask] add blueprint tests

5f8ac2d

[flask] rename patch.py to monkey.py

0564db2

[flask] make sure do to do Pin(tracer=self.tracer) in tests

a5d8006

[flask] fix spelling

e126904

[flask] add patch/unpatch idempotency tests

402ca93

[flask] add initial support for v0.9

cc7cef4

[flask] update tests for v0.9

822c9cc

Kyle-Verhoog and others added 3 commits October 30, 2018 13:38

Update ddtrace/contrib/flask/patch.py

92f32a5

Co-Authored-By: brettlangdon <me@brett.is>

[flask] remove unnecessary 'func_name(func)' call

f878da4

Merge branch 'brettlangdon/flask-dev' of github.com:datadog/dd-trace-…

a4e08db

…py into brettlangdon/flask-dev

brettlangdon commented Oct 30, 2018

View reviewed changes

Kyle-Verhoog and others added 3 commits October 30, 2018 14:09

Update tests/contrib/flask/test_blueprint.py

09b2b92

Co-Authored-By: brettlangdon <me@brett.is>

[flask] fix remaining comments from kyle

8b1dcc4

Merge branch 'brettlangdon/flask-dev' of github.com:datadog/dd-trace-…

d1507d7

…py into brettlangdon/flask-dev

brettlangdon removed the do-not-merge/WIP label Oct 30, 2018

brettlangdon added 4 commits October 30, 2018 14:46

[flask] fix flake8 issues

1797a60

[flask] test distributed tracing

199406d

[flask] add find_span_parent helper

ed4a5bb

[flask] add hook test cases

4afe310

Kyle-Verhoog previously approved these changes Oct 31, 2018

View reviewed changes

brettlangdon added 2 commits November 1, 2018 09:46

[flask] fix spelling

65f0c6b

[flask] rename Pin.find to Pin._find and add tests

25ae08c

brettlangdon dismissed Kyle-Verhoog’s stale review via 25ae08c November 1, 2018 13:52

[flask] one last Pin.find -> Pin._find

a676d3c

brettlangdon added 2 commits November 2, 2018 14:21

[flask] try to parse endpoint/url rule in Flask.preprocess_request as…

e4d6080

… well

[flask] fix line too long issue

fcc7f6a

Kyle-Verhoog previously approved these changes Nov 5, 2018

View reviewed changes

brettlangdon changed the base branch from master to 0.16-dev November 6, 2018 13:21

Merge branch '0.16-dev' into brettlangdon/flask-dev

9fe7c7b

brettlangdon dismissed Kyle-Verhoog’s stale review via 9fe7c7b November 6, 2018 13:24

brettlangdon merged commit 4c3a745 into 0.16-dev Nov 6, 2018

brettlangdon deleted the brettlangdon/flask-dev branch November 20, 2018 18:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[flask] rewrite Flask integration #667

[flask] rewrite Flask integration #667

brettlangdon commented Oct 23, 2018 •

edited

brettlangdon commented Oct 30, 2018

brettlangdon commented Oct 30, 2018

brettlangdon Oct 30, 2018

brettlangdon Oct 30, 2018

Kyle-Verhoog Oct 31, 2018

Kyle-Verhoog left a comment •

edited

Kyle-Verhoog Oct 31, 2018

brettlangdon Oct 31, 2018

clutchski commented Oct 31, 2018 •

edited

brettlangdon commented Nov 1, 2018

[flask] rewrite Flask integration #667

[flask] rewrite Flask integration #667

Conversation

brettlangdon commented Oct 23, 2018 • edited

Before

After

brettlangdon commented Oct 30, 2018

brettlangdon commented Oct 30, 2018

brettlangdon Oct 30, 2018

Choose a reason for hiding this comment

brettlangdon Oct 30, 2018

Choose a reason for hiding this comment

Kyle-Verhoog Oct 31, 2018

Choose a reason for hiding this comment

Kyle-Verhoog left a comment • edited

Choose a reason for hiding this comment

Kyle-Verhoog Oct 31, 2018

Choose a reason for hiding this comment

brettlangdon Oct 31, 2018

Choose a reason for hiding this comment

clutchski commented Oct 31, 2018 • edited

brettlangdon commented Nov 1, 2018

brettlangdon commented Oct 23, 2018 •

edited

Kyle-Verhoog left a comment •

edited

clutchski commented Oct 31, 2018 •

edited