Adding pymongo integration

[hectorhdzg]

Database fields
https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-semantic-conventions.md#databases-client-calls

#209

[Oberon00]

Is this based on OpenCensus, or OpenTracing, or is it a completely new implementation?

[hectorhdzg]

@Oberon00 I took OpenCensus as starting point, I could not find any pymongo integration for OpenTracing, some properties are different and we are getting more information in the span attributes now.
https://github.com/census-instrumentation/opencensus-python/tree/master/contrib/opencensus-ext-pymongo

[lzchen]

LGTM

[hectorhdzg]

@Oberon00 let me know if you have more feedback

[hectorhdzg]

@lzchen , @Oberon00 I updated the code to have an internal span dictionary like Christian suggested, keeping track of current span and avoid conflicts with other spans created in different MongoDB clients, thanks for reviewing and let me know if you have any feedback

[Oberon00]

Thank you @hectorhdzg, I think the code looks quite solid now.

Unfortunately, I have to request changes yet again because I'm pretty sure the unit tests aren't being run in the CI (you can verify locally by running just tox without the -e option and checking the output at the end for a line like py37-test-ext-pymongo: commands succeeded).

[Oberon00]

On using str: Shouldn't e.g. limit be an integer?

[hectorhdzg]

string casting is more important for other fields, dictionaries and lists are also part of COMMAND_ATTRIBUTES, I can add specific checks for limit and skip(both int) if the type is important here

[Oberon00]

I think at this point, it would be simpler to take "limit" out of COMMAND_ATTRIBUTES and query it separately.

[Oberon00]

LGTM! 👍 Thanks for going through all these rounds of comments!

[Oberon00]

@open-telemetry/python-approvers: If someone has knowledge of MongoDB (I don't), I'd love another review on this before merging.

[lzchen]

LGTM

[codecov-io]

Codecov Report

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

@@            Coverage Diff             @@
##           master     #232      +/-   ##
==========================================
- Coverage    85.5%   84.77%   -0.74%     
==========================================
  Files          33       33              
  Lines        1608     1589      -19     
  Branches      181      180       -1     
==========================================
- Hits         1375     1347      -28     
- Misses        186      195       +9     
  Partials       47       47

Impacted Files	Coverage Δ
...try-api/src/opentelemetry/context/async_context.py	17.39% <0%> (-34.79%)	⬇️
...elemetry-api/src/opentelemetry/context/__init__.py	88.88% <0%> (-11.12%)	⬇️
...ry-sdk/src/opentelemetry/sdk/resources/__init__.py	68.18% <0%> (-2.66%)	⬇️
...opentelemetry/sdk/context/propagation/b3_format.py	84% <0%> (-0.62%)	⬇️
...ry-ext-wsgi/src/opentelemetry/ext/wsgi/__init__.py	90.14% <0%> (-0.53%)	⬇️
opentelemetry-sdk/src/opentelemetry/sdk/util.py	85.54% <0%> (-0.35%)	⬇️
...src/opentelemetry/ext/opentracing_shim/__init__.py	93.33% <0%> (-0.31%)	⬇️
...ts/src/opentelemetry/ext/http_requests/__init__.py	88.88% <0%> (-0.31%)	⬇️
...xt-jaeger/src/opentelemetry/ext/jaeger/__init__.py	85% <0%> (-0.19%)	⬇️
...app/src/opentelemetry_example_app/flask_example.py	100% <0%> (ø)	⬆️

---

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 e95a115...f2c344e. Read the comment docs.

[c24t]

LGTM after fixing version numbers, but my review is taking a lot of the expected behavior on faith. It would be helpful to have an example or integration test here, or a better description in the package readme.

I agree with @Oberon00 that we'd benefit from a reviewer familiar with mongo, but don't want to hold this PR up.