Skip to content

Django updates #6

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

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion django_opentracing/__init__.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
from .middleware import OpenTracingMiddleware
from .tracer import DjangoTracer
from .tracer import DjangoTracer, get_current_span, get_tracer
47 changes: 29 additions & 18 deletions django_opentracing/middleware.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
from django.conf import settings
from __future__ import absolute_import
import django
from django.conf import settings
from jaeger_client import Config
import opentracing
from .tracer import DjangoTracer

try:
# Django >= 1.10
from django.utils.deprecation import MiddlewareMixin
Expand All @@ -10,29 +15,35 @@

class OpenTracingMiddleware(MiddlewareMixin):
'''
__init__() is only called once, no arguments, when the Web server responds to the first request
In Django <= 1.9 __init__() is only called once, no arguments, when the Web server responds to the first request
'''
def __init__(self, get_response=None):
'''
TODO: ANSWER Qs
- Is it better to place all tracing info in the settings file, or to require a tracing.py file with configurations?
- Also, better to have try/catch with empty tracer or just fail fast if there's no tracer specified
'''
if hasattr(settings, 'OPENTRACING_TRACER'):
self._tracer = settings.OPENTRACING_TRACER
else:
self._tracer = opentracing.Tracer()
self._tracer = None
self.get_response = get_response

def process_view(self, request, view_func, view_args, view_kwargs):
# determine whether this middleware should be applied
# NOTE: if tracing is on but not tracing all requests, then the tracing occurs
# through decorator functions rather than middleware
if not self._tracer._trace_all:
return None
# in django 1.10, __init__ is called on server startup
if int(django.get_version().split('.')[1]) <= 9:
if self._tracer == None:
self._tracer = self.init_tracer()

'''
For Django >= 1.10, executed on every request
'''
def __call__(self, request):
if self._tracer == None:
self._tracer = self.init_tracer()

response = self.get_response(request)

return response

def init_tracer(self):
return DjangoTracer(Config(config=settings.OPENTRACING_TRACER_CONFIG, service_name=settings.SERVICE_NAME).initialize_tracer())

def process_view(self, request, view_func, view_args, view_kwargs):
if hasattr(settings, 'OPENTRACING_TRACED_ATTRIBUTES'):
traced_attributes = getattr(settings, 'OPENTRACING_TRACED_ATTRIBUTES')
else:
else:
traced_attributes = []
self._tracer._apply_tracing(request, view_func, traced_attributes)

Expand Down
33 changes: 25 additions & 8 deletions django_opentracing/tracer.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,30 @@
from builtins import object, str
from django.conf import settings
import opentracing

django_tracer = None

def get_tracer():
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's need for this function, because of it always returning opentracing.tracer - moreover, I don't think we need to expose it in the public api. Or is there any use scenario where you need it?

return opentracing.tracer

def get_current_span(request):
# this lets django rest framework work seamlessly since they wrap the request
if hasattr(request, '_request'):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something we could add, yes.

request = request._request
if django_tracer != None:
return django_tracer.get_span(request)
else:
return None


Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far we had been saving the span for each request in a dictionary, as part of the DjangoTracer. Is there any need to change this behaviour?

class DjangoTracer(object):
'''
@param tracer the OpenTracing tracer to be used
to trace requests using this DjangoTracer
'''
def __init__(self, tracer):
global django_tracer
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I don't like the idea of having a global here. We have it for other connectors, but for frameworks we have this toplevel-tracer approach, which is most likely easier/better. As before, if there's any need for this change, we can take it into consideration.

django_tracer = self
self._tracer = tracer
self._current_spans = {}
if not hasattr(settings, 'OPENTRACING_TRACE_ALL'):
Expand All @@ -16,9 +34,9 @@ def __init__(self, tracer):
else:
self._trace_all = True

def get_span(self, request):
def get_span(self, request):
'''
@param request
@param request
Returns the span tracing this request
'''
return self._current_spans.get(request, None)
Expand Down Expand Up @@ -53,11 +71,11 @@ def _apply_tracing(self, request, view_func, attributes):
'''
# strip headers for trace info
headers = {}
for k,v in request.META.iteritems():
for k,v in list(request.META.items()):
k = k.lower().replace('_','-')
if k.startswith('http-'):
k = k[5:]
headers[k] = v
headers[k] = v
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually spaces/styling changes shouldn't exist (unless we are trying to follow flake8 or similar, and then they should exist in a standalone patch).


# start new span from trace info
span = None
Expand All @@ -79,10 +97,9 @@ def _apply_tracing(self, request, view_func, attributes):
payload = str(getattr(request, attr))
if payload:
span.set_tag(attr, payload)

return span
return span

def _finish_tracing(self, request):
span = self._current_spans.pop(request, None)
span = self._current_spans.pop(request, None)
if span is not None:
span.finish()
span.finish()
25 changes: 14 additions & 11 deletions example/client/views.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
from future import standard_library
standard_library.install_aliases()
from builtins import str
from django.conf import settings
from django.http import HttpResponse
from django.shortcuts import render

import opentracing
import urllib2
import urllib.request, urllib.error, urllib.parse

tracer = settings.OPENTRACING_TRACER

Expand All @@ -15,42 +18,42 @@ def client_index(request):
@tracer.trace()
def client_simple(request):
url = "http://localhost:8000/server/simple"
new_request = urllib2.Request(url)
new_request = urllib.request.Request(url)
current_span = tracer.get_span(request)
inject_as_headers(tracer, current_span, new_request)
try:
response = urllib2.urlopen(new_request)
response = urllib.request.urlopen(new_request)
return HttpResponse("Made a simple request")
except urllib2.URLError as e:
except urllib.error.URLError as e:
return HttpResponse("Error: " + str(e))

@tracer.trace()
def client_log(request):
url = "http://localhost:8000/server/log"
new_request = urllib2.Request(url)
new_request = urllib.request.Request(url)
current_span = tracer.get_span(request)
inject_as_headers(tracer, current_span, new_request)
try:
response = urllib2.urlopen(new_request)
response = urllib.request.urlopen(new_request)
return HttpResponse("Sent a request to log")
except urllib2.URLError as e:
except urllib.error.URLError as e:
return HttpResponse("Error: " + str(e))

@tracer.trace()
def client_child_span(request):
url = "http://localhost:8000/server/childspan"
new_request = urllib2.Request(url)
new_request = urllib.request.Request(url)
current_span = tracer.get_span(request)
inject_as_headers(tracer, current_span, new_request)
try:
response = urllib2.urlopen(new_request)
response = urllib.request.urlopen(new_request)
return HttpResponse("Sent a request that should produce an additional child span")
except urllib2.URLError as e:
except urllib.error.URLError as e:
return HttpResponse("Error: " + str(e))

def inject_as_headers(tracer, span, request):
text_carrier = {}
tracer._tracer.inject(span.context, opentracing.Format.TEXT_MAP, text_carrier)
for k, v in text_carrier.iteritems():
for k, v in text_carrier.items():
request.add_header(k,v)

3 changes: 2 additions & 1 deletion tests/test_site/__init__.py
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
import test_middleware
from __future__ import absolute_import
from . import test_middleware