2020-12-27 14:18

Make dependency on tornado/threadloop optional

I would like to use jaeger-client as an opentracing tracer with zipkin. I've already completed a proof-of-concept integration, and I fully intend to release the Zipkin codec and Zipkin reporter as soon as they are ready.

My goal is to use the client on a Django/Gunicorn/Greenlet environment, so I would like to know if there is any possibility to make the tornado/threadloop dependencies optional.

Ideally, I think that since the client's architecture is so modular, the Reporter could be renamed to TornadoReporter and properly presented on the Python package (along with the local_agent) only if the tornado/threadloop modules are on the Python PATH.


  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享
  • 邀请回答


  • weixin_39917090 weixin_39917090 4月前

    I don't have objections to this approach. However, what happens to requirements.txt in this case? Would we need to include all different frameworks in there? Or do we put them only in test requirements and tell the users that they need to depend on the correct module?

    点赞 评论 复制链接分享
  • weixin_39610774 weixin_39610774 4月前

    The second solution was what I had in mind. We could require all the dependencies only for the tests and "recommend" to the users the right dependencies, issuing appropriate warning messages or raising an exception if one tries to use the tornado reporter without having tornado in the python path.

    Another more bureaucratic approach is to create another python package just for the framework specific code.

    点赞 评论 复制链接分享
  • weixin_39917090 weixin_39917090 4月前

    Update: there have been other issues and discussions related to this, I think the ideal approach is to remove Tornado dependency completely and implement async span flushing in a dedicated thread. The thread should also be started lazily (e.g. only when the first span is reported), which should address two issues: * when running under WSGI-style frameworks, the thread will naturally be started post-fork * if the thread is always started asynchronously (unlike the threadloop which currently blocks), then it won't be causing the deadlocks

    点赞 评论 复制链接分享
  • weixin_39869043 weixin_39869043 4月前

    I'm playing with making this work nicely with https://github.com/opentracing-contrib/python-flask. Would love to see a separate repo for framework specific niceties, like jaeger-flask, jaeger-django etc.

    Side note; thanks for all the hard work!

    点赞 评论 复制链接分享
  • weixin_39606361 weixin_39606361 4月前

    , do you think a threading.Timer would be an acceptable replacement for the PeriodicCallbacks in RemoteControlledSampler* and RemoteThrottler? I'm not sure how straightforward it would be to ensure their invocations to be post-fork.

    afaict these are the only other places that require nontrivial removal of tornado, assuming thrift-generated code is 1:1 when not specifying the tornado option.

    点赞 评论 复制链接分享
  • weixin_39611765 weixin_39611765 4月前

    Is the removal of tornado from thrift included in the scope of this ticket? I didn't know that thrift depended directly on tornado until I read 's comment 1 2

    which makes this kind of a non-starter.

    点赞 评论 复制链接分享
  • weixin_39917090 weixin_39917090 4月前

    I don't know off hand what threading.timer does. Does it require a separate thread?

    点赞 评论 复制链接分享