Add testbed for otel-ot-shim

[mauriciovasquezbernal]

This PR ports the OpenTracing testbed https://github.com/opentracing/opentracing-python/tree/master/testb to check that the ot-shim is working as expected using different frameworks.

Gevent doesn't support context vars yet gevent/gevent#1407, so those tests are not compatible with opentelemetry and were not ported.

[codecov-io]

Codecov Report

Merging #274 into master will decrease coverage by 4.58%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #274      +/-   ##
==========================================
- Coverage   89.50%   84.91%   -4.59%     
==========================================
  Files          43       38       -5     
  Lines        2229     1889     -340     
  Branches      248      217      -31     
==========================================
- Hits         1995     1604     -391     
- Misses        159      219      +60     
+ Partials       75       66       -9     

Impacted Files	Coverage Δ
...ry-sdk/src/opentelemetry/sdk/resources/__init__.py	70.83% <0.00%> (-22.03%)	⬇️
...elemetry-api/src/opentelemetry/metrics/__init__.py	87.93% <0.00%> (-5.03%)	⬇️
...opentelemetry/sdk/context/propagation/b3_format.py	84.61% <0.00%> (-2.66%)	⬇️
...sdk/src/opentelemetry/sdk/trace/export/__init__.py	86.79% <0.00%> (-2.57%)	⬇️
...xt-jaeger/src/opentelemetry/ext/jaeger/__init__.py	85.18% <0.00%> (-2.39%)	⬇️
...emetry-sdk/src/opentelemetry/sdk/trace/__init__.py	89.71% <0.00%> (-2.33%)	⬇️
...-ext-flask/src/opentelemetry/ext/flask/__init__.py	88.88% <0.00%> (-1.12%)	⬇️
...ntelemetry-api/src/opentelemetry/trace/__init__.py	84.56% <0.00%> (-0.25%)	⬇️
...src/opentelemetry/ext/opentracing_shim/__init__.py	95.90% <0.00%> (-0.14%)	⬇️
opentelemetry-api/src/opentelemetry/util/types.py	100.00% <0.00%> (ø)
... and 36 more

---

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 b799e54...8c6f945. Read the comment docs.

[carlosalberto]

Hey, it looks overall good. I'm wondering about the updates for the build scripts, but I guess @c24t can ponder that ;)

[carlosalberto]

Did a second review and I feel it looks fine, but I'm wondering about the build/script part (such as https://github.com/open-telemetry/opentelemetry-python/pull/274/files#r345754382) - but other than that I'm fine with all of this.

@c24t if you feel the script changes are fine, lets go with it. Else, lets postpone merging it and have someone else working on this.

[toumorokoshi]

Overall this looks good, and I feel like these patterns would be valuable to illustrate even for opentelemetry API examples.

There's a few parts that seem to not match the description, or I could be misunderstanding. I have comments around those parts.

[condorcet]

Please note supporting of contextvars in OT with relevant changes in testbed.

[condorcet]

I think we don't need to include tornado testbed here because of:

1. Tornado < 6 has their own mechanism (stack_context) for storing context variables. In OT we using special scope manager based on this feature to store current context. This is the reason why we have testbed for tornado.
2. OTel doesn't provide something like scope manager in OT. In terms of OT it's equal of using contextvars. We couldn't test this Tornado < 6 specific mechanism even if we want.
3. Tornado >=6 is fully asyncio-based framework where their own stack_context was removed.

All this means that Tornado testbed is redundant and could be covered by contextvars testbed.
You can see in OT we don't include tornado testbed if Tornado >=6 https://github.com/opentracing/opentracing-python/blob/master/testbed/__main__.py#L14

[carlosalberto]

Hey @condorcet Thanks for chiming it ;)

Yes, I think we should 'retire' the Tornado section, as it doesn't need its own thing and it's supposed to behave the same as asyncio, regarding to contextvars.

(I remember comments long, long time ago at being some differences between the Tornado and asyncio using contextvars, but couldn't find any note on this bit anymore ;) ).

[carlosalberto]

Hey hey @toumorokoshi

So probably we ought to add a longer description + rationale about this testbed component. The idea is not to have it as a simple set of examples nor a test case suite, but rather as a set of (exoteric) examples that would allow us to validate active Span handling (when writing prototypes). Assertions are there as a matter of 'completeness' ;)

(I can tell you this were incredible useful back at the time of implementing this API, as we needed to support a few frameworks: tornado, gevent, asyncio, etc - good thing is that now we have contextvars!)

[mauriciovasquezbernal]

@condorcet @carlosalberto I'm going to remove the Tornado test cases then.

[toumorokoshi]

Approving as I originally misunderstood the intent, and this is good code to validate opentracing that we should be adding into our suites. Thanks!

[toumorokoshi]

merged as part of #727. thanks!