zalando-zmon / opentracing-utils

Convenient utilities for adding OpenTracing support in your python projects
MIT License
20 stars 8 forks source link

Add scope manager support #55

Closed mohabusama closed 3 years ago

mohabusama commented 3 years ago

Resolve #54

Changes

Add support for tracer scope manager support introduced in opentracing==2.0.

I am not fully decided on the order of detecting parent spans. The proposed implementation is the following:

~1. Using opentracing.tracer.active_span managed by the tracer context manager. The new span will be using the scope manager.~ ~2. Using span_extractor.~ ~3. Detecting span in kwargs.~ ~4. Using call stack frames inspection.~

  1. Using span_extractor.
  2. Detecting span in kwargs.
  3. Using opentracing.tracer.active_span managed by the tracer context manager. The new span will be using the scope manager.
  4. Using call stack frames inspection.

as span_extractor and passing spans in kwargs is more explicit.

Please let me know what you think.

Release strategy

We don't have to release a new version right away after merging. We are planning to first test the changes in our production systems, fix any related bugs observed and confirm that no breaking changes are introduced.

codecov[bot] commented 3 years ago

Codecov Report

Merging #55 (266b805) into master (e681bdd) will increase coverage by 0.13%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #55      +/-   ##
==========================================
+ Coverage   98.68%   98.81%   +0.13%     
==========================================
  Files           9        9              
  Lines         379      423      +44     
==========================================
+ Hits          374      418      +44     
  Misses          5        5              
Impacted Files Coverage Δ
opentracing_utils/decorators.py 100.00% <100.00%> (ø)
opentracing_utils/libs/_django.py 97.43% <100.00%> (+0.21%) :arrow_up:
opentracing_utils/libs/_flask.py 97.33% <100.00%> (+0.19%) :arrow_up:
opentracing_utils/libs/_requests.py 100.00% <100.00%> (ø)
opentracing_utils/libs/_sqlalchemy.py 100.00% <100.00%> (ø)
opentracing_utils/span.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e681bdd...266b805. Read the comment docs.

mohabusama commented 3 years ago

@vetinari @pitr @dneuhaeuser-zalando Please let me know what you think.

dneuhaeuser-zalando commented 3 years ago

It could be useful if we change to the following order: [...] as span_extractor and passing spans in kwargs is more explicit.

Agreed. Using span_extractor or passing a span clearly and explicitly communicates what the user wishes to happen in that specific location. I think that should override a scope or at the very least lead to a warning.

mohabusama commented 3 years ago

@mvalkon you might be interested in this one.

mohabusama commented 3 years ago

@vetinari @pitr this one is ready, we have tested it in production.

mohabusama commented 3 years ago

👍

pitr commented 3 years ago

👍