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

Lazily initialize tracer for Jaeger/1.10 compatibility #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

benjigoldberg
Copy link

@benjigoldberg benjigoldberg commented Jan 8, 2018

When running The Jaeger Python Client with this library, the initialization step hangs indefinitely with Django 1.10.

@mikebryant if you take your example OpenTracing/Jaeger Django project and upgrade Django to >=1.10 and try running, the server will fail to start.

I believe this comes down to the way in which Django changed the initialization of Middleware. In versions prior to Django 1.10, the middleware wouldnt be initialized until the first request. In Django 1.10+, the middleware is initialized on start. Initializing the Jaeger tracer before the first request causes the Jaeger client to deadlock -- check out Jaeger Issue 31 and Jaeger Issue 60.

This change simply forces the initialization to happen once on the first request, thus avoiding the above behavior.

@SEJeff
Copy link

SEJeff commented Apr 12, 2018

@mikebryant ^^

@rmfitzpatrick
Copy link

Is there anything preventing this from being accepted, @mikebryant? It doesn't seem to introduce any long term overhead for apps or tracers and is necessary for the jaeger client to work in django.

@mikebryant
Copy link
Collaborator

ugh, I'm really sorry. I've been overloaded with other stuff and missed this.

Thanks for pinging me about this!

@naphthalene
Copy link

Ping :)

@carlosalberto carlosalberto self-assigned this Oct 31, 2018
@rmfitzpatrick
Copy link

@carlosalberto, @benjigoldberg if it's any help I was able to get this working against master w/ these changes: rmfitzpatrick@301045b.

@Jamim
Copy link

Jamim commented Nov 22, 2018

Hi folks,

It would be great to see rmfitzpatrick/python-django@301045b cherry-picked into master.
Thank you for the effort, @benjigoldberg and @rmfitzpatrick!

@vinhlh
Copy link

vinhlh commented Dec 21, 2018

Hi guys, any update 💃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants