Skip to content

python3 fixes for tracer.py iteritems, Readme #16

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 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 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
9 changes: 5 additions & 4 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ In order to implement tracing in your system, add the following lines of code to

# if not included, defaults to False
# has to come before OPENTRACING_TRACER setting because python...
OPENTRACING_TRACE_ALL = False,
OPENTRACING_TRACE_ALL = False

# defaults to []
# only valid if OPENTRACING_TRACE_ALL == True
OPENTRACING_TRACED_ATTRIBUTES = ['arg1', 'arg2'],
OPENTRACING_TRACED_ATTRIBUTES = ['arg1', 'arg2']

# Callable that returns an `opentracing.Tracer` implementation.
OPENTRACING_TRACER_CALLABLE = 'opentracing.Tracer'
Expand All @@ -50,7 +50,7 @@ If you want to directly override the `DjangoTracer` used, you can use the follow
.. code-block:: python

# some_opentracing_tracer can be any valid OpenTracing tracer implementation
OPENTRACING_TRACER = django_opentracing.DjangoTracer(some_opentracing_tracer),
OPENTRACING_TRACER = django_opentracing.DjangoTracer(some_opentracing_tracer)

**Note:** Valid request attributes to trace are listed [here](https://docs.djangoproject.com/en/1.9/ref/request-response/#django.http.HttpRequest). When you trace an attribute, this means that created spans will have tags with the attribute name and the request's value.

Expand Down Expand Up @@ -101,14 +101,15 @@ Tracing an RPC
If you want to make an RPC and continue an existing trace, you can inject the current span into the RPC. For example, if making an http request, the following code will continue your trace across the wire:

.. code-block:: python
from future.utils import listitems # for python3 compatibility

@tracer.trace()
def some_view_func(request):
new_request = some_http_request
current_span = tracer.get_span(request)
text_carrier = {}
opentracing_tracer.inject(span, opentracing.Format.TEXT_MAP, text_carrier)
for k, v in text_carrier.iteritems():
for k, v in listitems(text_carrier):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't we preserve the existing behaviour of not converting to a list?

e.g.

from future.utils import iteritems
...
for k, v in iteritems(text_carrier):

request.add_header(k,v)
... # make request

Expand Down
3 changes: 2 additions & 1 deletion django_opentracing/tracer.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from django.conf import settings
from django.utils.module_loading import import_string
from future.utils import listitems
import opentracing
import threading

Expand Down Expand Up @@ -64,7 +65,7 @@ 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 listitems(request.META):
Copy link
Collaborator

Choose a reason for hiding this comment

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

again, stick with iteritems here?

Copy link

@crccheck crccheck Apr 20, 2018

Choose a reason for hiding this comment

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

well we can't stick with iteritems because iteritems doesn't exist in Python 3. The simplest approach I've used is just using .items() because:

  • Python3 still uses iterators
  • it's not a big deal to not have an iterator because it's not that big
  • Only Python2 has to deal with that and no one really cares about Python2 anymore

k = k.lower().replace('_','-')
if k.startswith('http-'):
k = k[5:]
Expand Down
3 changes: 2 additions & 1 deletion example/client/views.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from django.conf import settings
from django.http import HttpResponse
from django.shortcuts import render
from future.utils import listitems

import opentracing
import urllib2
Expand Down Expand Up @@ -51,6 +52,6 @@ def client_child_span(request):
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 listitmes(text_carrier):
Copy link
Collaborator

Choose a reason for hiding this comment

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

and here.

Also - this has a typo in any case

request.add_header(k,v)

3 changes: 2 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
platforms='any',
install_requires=[
'django',
'opentracing>=1.1,<1.2'
'opentracing>=1.1,<1.2',
'future',
],
classifiers=[
'Environment :: Web Environment',
Expand Down