More complete example of Tornado instrumentation

[ekelleyv]

I have been trying to piece together how to properly instrument a tornado application based on the documentation/comments throughout this repository, such as:

https://github.com/uber-common/opentracing-python-instrumentation#usage

https://github.com/uber-common/opentracing-python-instrumentation/blob/master/opentracing_instrumentation/request_context.py#L231-L240

However, I cannot quite connect all the dots. I think what would be extremely useful is a more complete example of how to use the middleware with a Tornado RequestHandler.

Also, if you just want to make a quick note in this issue showing how it is to be used, I would be more than happy to commit PR the actual documentation once we have our instrumentation working. Thanks!

[yurishkuro]

let me try to pull some of our internal files. They will reference some internal libs, but hopefully it's clear what's going on. Adapting this into an open-source example would be awesome. Our internal repo has a example that also runs as unit test and checks interop between different python libs like Flask, Tornado, and TChannel. Ironically, the screenshot on https://zipkin.io/ is from that example.

• example/tornado_server.py https://pastebin.com/rdrZqFpK
• example/tornado_server_routes.py https://pastebin.com/r777qn7w
• jaeger_clay_tornado/tornado_middleware.py https://pastebin.com/uJ4w7um5
• jaeger_clay_tornado/initializer.py https://pastebin.com/2GgwunzJ
• jaeger_clay/initializer.py https://pastebin.com/GyEMu2Kh

It's possible some of the jaeger_xxx files above might be refactored and moved to open source.

[ekelleyv]

Thanks for the quick response @yurishkuro! Any chance you have an example where JaegerTracerMiddleware is actually instantiated/used (or did I just miss that)?

[yurishkuro]

ah, right, it's used with this extension we have for tornado.web.App: https://pastebin.com/547sjA6f

[ekelleyv]

Ok, so I have put together a super simple tornado app to try and work through this in isolation of existing codebases (and with stock tornado rather than clay-tornado):
https://github.com/ekelleyv/jaeger-tornado-example

It definitely does not handle threads correctly etc, but is mostly meant to be illustrative of the pattern we are looking at (wraps every handler that extends from BaseHandler, requires minimal code changes to existing app, etc). It creates a "correct" span record when just hitting locally:

I am wondering how close to "out of the box" tornado this can be kept? Tornado provides prepare and on_finish for every request in RequestHandler. Is there a way to do tracing properly with just those methods?

[yurishkuro]

Is there a way to do tracing properly with just those methods?

The complication is that you need to propagate the current span using Tornado's native version of TLS (StackContext). I think it's possible to do via callbacks instead of a context manager.

Another nice to have thing is to be able to access the route defined for each endpoint, so that instead of GET as the operation names you could capture something like /delay/{num}.

[ekelleyv]

Good point about the operations names. Any chance you could share what tornado_extras_http.middleware.MiddlewareHandler looks like? I think I otherwise understand all the pieces of the middleware approach (patching dispatcher etc).

[yurishkuro]

https://pastebin.com/gnqBRq0x

[ekelleyv]

Awesome, this is super helpful.

Just a heads up that tornado.web._RequestDispatcher went away in recent tornado versions (tornadoweb/tornado#1806).

[ekelleyv]

I seem to have the test app working properly now. I basically just copy/pasted and rearranged the code you provided (removed some clay references, unwound circular dep, changed some naming etc):
https://github.com/ekelleyv/jaeger-tornado-example

Basically if the folders jaeger_tornado_tracer and the tornado_middleware are released as standalone projects, then anyone using tornado could install them, instantiate the tracer middleware, use the build_app helper, and have tracing set up super quickly.

So a couple questions:

1. Does the sample project look sane/correct?
2. What do you think of packaging those folders? This is effectively y'alls code, so how/if you want to do that is up to you. What org do you think they should live under? Only jaeger_tornado_tracer is inherently jaeger/opentracing specific.
3. Is there somewhere better to discuss this? Think this is now a bit larger than the scope of the issue opened this morning

Thanks again for your help here!

[yurishkuro]

The sample looks reasonable.

Regarding packaging it, I am reluctant to create real modules and take the maintenance burden, since we have equivalent internal modules at Uber, and open sourcing those is also quite a bit of work that's hard to justify. I would be fine with just keeping them internal to the examples/tornado/ in this repo, if people want to use them they can copy & paste. We can use the same MIT license header as the rest of the files in this repo.

I do wonder if there's a simpler way than several wrapper/middleware classes.

Is there somewhere better to discuss this?

you mean like chat? Don't think we have an active community interested specifically in this repo, but otherwise you can find us on https://gitter.im/jaegertracing/Lobby and https://gitter.im/opentracing/public.

[awiddersheim]

This seems to be working for me. Code inspired by many sources.

from tornado.web import HTTPError
from tornado.web import RequestHandler
from opentracing_instrumentation import http_server
from opentracing_instrumentation import request_context


class BaseRequestHandler(RequestHandler):
    def _execute(self, *args, **kwargs):
        request = http_server.TornadoRequestWrapper(self.request)
        self.span = http_server.before_request(request)

        with request_context.span_in_stack_context(self.span):
            super(BaseRequestHandler, self)._execute(*args, **kwargs)

    def log_exception(self, typ, value, tb):
        if not isinstance(value, HTTPError) or 500 <= value.status_code <= 599:
            self.span.set_tag('error', True)
            self.span.log_event(event='error', payload=value)

        super(BaseRequestHandler, self).log_exception(typ, value, tb)

    def on_finish(self):
        self.span.set_tag('http.status_code', self.get_status())
        self.span.finish()