python-flask
python-flask copied to clipboard
`start_span` on `FlaskTracer` should delegate to the underlying tracer implementation
I'm trying to manually add a span to a trace using the jaeger Python client, as described in the documentation. However in the example, the span is created by calling the underlying tracer implementation directly (jaeger_tracer.start_span() rather than the FlaskTracer instance.
I didn't notice this initially and attempted to call start_span() on my FlaskTracer instance directly, which resulted in an error being raised from the underlying opentracing.Tracer:
Traceback (most recent call last):
File "/home/jscn/venvs/ent-admin/lib/python3.4/site-packages/flask/app.py", line 2301, in __call__
return self.wsgi_app(environ, start_response)
File "/home/jscn/venvs/ent-admin/lib/python3.4/site-packages/flask/app.py", line 2287, in wsgi_app
response = self.handle_exception(e)
File "/home/jscn/venvs/ent-admin/lib/python3.4/site-packages/flask/app.py", line 1733, in handle_exception
reraise(exc_type, exc_value, tb)
File "/home/jscn/venvs/ent-admin/lib/python3.4/site-packages/flask/_compat.py", line 35, in reraise
raise value
File "/home/jscn/venvs/ent-admin/lib/python3.4/site-packages/flask/app.py", line 2284, in wsgi_app
response = self.full_dispatch_request()
File "/home/jscn/venvs/ent-admin/lib/python3.4/site-packages/flask/app.py", line 1807, in full_dispatch_request
rv = self.handle_user_exception(e)
File "/home/jscn/venvs/ent-admin/lib/python3.4/site-packages/flask/app.py", line 1710, in handle_user_exception
reraise(exc_type, exc_value, tb)
File "/home/jscn/venvs/ent-admin/lib/python3.4/site-packages/flask/_compat.py", line 35, in reraise
raise value
File "/home/jscn/venvs/ent-admin/lib/python3.4/site-packages/flask/app.py", line 1805, in full_dispatch_request
rv = self.dispatch_request()
File "/home/jscn/venvs/ent-admin/lib/python3.4/site-packages/flask/app.py", line 1791, in dispatch_request
return self.view_functions[rule.endpoint](**req.view_args)
File "/home/jscn/work/ent-admin/entadmin/api/people/people.py", line 23, in read
person = PersonRepository().me()
File "/home/jscn/work/ent-admin/entadmin/repositories/person.py", line 17, in me
person = self._get_one()
File "/home/jscn/work/ent-admin/entadmin/repositories/_base.py", line 80, in _get_one
response = self._get(url)
File "/home/jscn/work/ent-admin/entadmin/repositories/_base.py", line 68, in _get
with tracer.start_span('client get', child_of=parent_span) as span:
File "/home/jscn/venvs/ent-admin/lib/python3.4/site-packages/opentracing/tracer.py", line 88, in start_span
return self._noop_span
AttributeError: 'Tracer' object has no attribute '_noop_span'```
So it looks like, in order to make this work, I need to always pass around both the FlaskTracer instance, and the underlying jaeger_client.Tracer instance.
Since FlaskTracer already has a reference to that jaeger_client.Tracer instance, and that instance's implementation of start_span actually works, I think that FlaskTracer should delegate calls to start_span to whichever Tracer is passed in when FlaskTracer is instantiated.
As far as I can tell, there's no reason for FlaskTracer to inherit from opentracing.Tracer at all: FlaskTracer doesn't override any methods of the parent opentracing.Tracer class, and calling those methods on the opentracing.Tracer doesn't work because the parent class is never initialised. (Hence the above traceback). Furthermore, all the actual tracing in FlaskTracer is done via the tracer which is provided at instantiation time.
I do think it would make sense for FlaskTracer to implement all the methods of opentracing.Tracer, but delegate them to the tracer which is passed in when FlaskTracer is instantiated (in this case, jaeger_client.Tracer).
While I'm personally not a great fan of explicit interfaces in Python, if you want to encode the fact that FlaskTracer should provide the same interface as opentracing.Tracer, I'd be happy to open a ticket / PR on the the upstream project to provide an ABC which FlaskTracer could inherit from.
@jscn Actually the 'bad' name part was mentioned in the past - DjangoTracer and FlaskTracer implementations should be changed to DjangoTracing and FlaskTracing respectively, and maybe offer an actual public property exposing the underlying tracer (so you could do tracing.tracer.start_span() for any tracer and/or instrumentation library*).
Still, I think we should track that (and make it a priority once we land the OT 2.0 version ;) )
- That's also without mentioning a proper global
Tracer, which we plan to have -like Java or C# do- shortly after the mentioned OT 2.0 integration happens.
@jscn I know it's been a while since you raised this but do you by chance still remember what it was exactly that you did to work around this? I'm having the same issue. Thanks!
@jscn I know it's been a while since you raised this but do you by chance still remember what it was exactly that you did to work around this? I'm having the same issue. Thanks!
I am having this issue too