Add OpenTracing shim example #282

c24t · 2019-11-11T06:24:14Z

I was working on a demo for OpenTracing compatibility and thought it'd be useful to include as an example here.

I wrote this in the style of #277, and copied some boilerplate text from that PR. We should consider making examples work with any backend, but I've only included Jaeger for now.

@johananl, @codeboten, @carlosalberto, let me know what you think!

codecov-io · 2019-11-11T06:31:08Z

Codecov Report

Merging #282 into master will decrease coverage by 0.02%.
The diff coverage is 87.5%.

@@            Coverage Diff             @@
##           master     #282      +/-   ##
==========================================
- Coverage   85.32%   85.29%   -0.03%     
==========================================
  Files          38       38              
  Lines        1928     1931       +3     
  Branches      227      227              
==========================================
+ Hits         1645     1647       +2     
- Misses        218      219       +1     
  Partials       65       65

Impacted Files	Coverage Δ
...xt-jaeger/src/opentelemetry/ext/jaeger/__init__.py	`86.54% <ø> (ø)`	⬆️
...elemetry-api/src/opentelemetry/metrics/__init__.py	`88.13% <100%> (ø)`	⬆️
...app/src/opentelemetry_example_app/flask_example.py	`100% <100%> (ø)`	⬆️
...emetry-sdk/src/opentelemetry/sdk/trace/__init__.py	`90.74% <80%> (+0.03%)`	⬆️
...sdk/src/opentelemetry/sdk/trace/export/__init__.py	`86.11% <0%> (-0.69%)`	⬇️

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 0239c08...73bada3. Read the comment docs.

mauriciovasquezbernal

I didn't try it myself by generally it looks fine.

I added comments about some details I spotted.

examples/opentracing/README.md

examples/opentracing/requirements.txt

examples/opentracing/README.md

examples/opentracing/main.py

codeboten · 2019-11-12T19:29:28Z

Ran through the example. Other than the missing opentelemetry-ext-jaeger requirement, it looks good

examples/opentracing/requirements.txt

ocelotl

Please review, I tried the example itself and it works fine 👍

ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py

ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py

codeboten · 2020-01-08T17:04:33Z

Thanks for fixing this @ocelotl! Approved with a non-blocking comment.

examples/opentracing/README.md

examples/opentracing/main.py

mauriciovasquezbernal · 2020-01-17T02:22:53Z

I see that now the PR has also some changes to the code of the shim, are those changes necessary?, if yes my suggestion would be to open a PR containing only those changes, IMO a PR that implements an example but also modifies the source code creates confusion.

This reverts commit 5a224ad.

c24t · 2020-01-23T01:03:46Z

I see that now the PR has also some changes to the code of the shim, are those changes necessary?

It looks like 5a224ad was necessary to avoid some false-positive no-member pylint errors now that we're running it via eachdist.py.

mauriciovasquezbernal · 2020-01-23T14:42:09Z

Is this commit needed 79912de?, I see that there is a revert and and then a revert of the revert.

If that commit is still needed is think it should be part of a separated PR. This PR is adding an example and should do only that, like in your first commit: c3e965d.

c24t · 2020-01-29T23:30:40Z

@mauriciovasquezbernal this should be ready to go now that #376 is in.

Oberon00

I don't really like that we need to work around pylint here, but otherwise LGTM.

examples/opentracing/README.md

mauriciovasquezbernal

I still see changes to the opentracing-shim code. Is it missing the revert (of the revert of the revert 🤣 ) of 9d3ef06 after #376?

Besides that LGTM!

examples/opentracing/rediscache.py

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

Co-Authored-By: Mauricio Vásquez <mauricio@kinvolk.io>

This reverts commit 9d3ef06.

c24t · 2020-02-04T19:50:24Z

@mauriciovasquezbernal 0239c08 is the last revert (of a revert of a revert...).

mauriciovasquezbernal

LGTM!

examples/opentelemetry-example-app/tests/test_flask_example.py

Co-authored-by: Diego Hurtado <ocelotl@users.noreply.github.com> Co-authored-by: Christian Neumüller <christian+github@neumueller.me> Co-authored-by: Mauricio Vásquez <mauricio@kinvolk.io>

c24t added the doc Documentation-related label Nov 11, 2019

c24t requested review from a-feld, carlosalberto, lzchen, Oberon00, reyang and toumorokoshi as code owners November 11, 2019 06:24

mauriciovasquezbernal reviewed Nov 12, 2019

View reviewed changes

mayurkale22 mentioned this pull request Nov 19, 2019

Add OpenTracing shim example open-telemetry/opentelemetry-js#550

Closed

codeboten suggested changes Nov 21, 2019

View reviewed changes

examples/opentracing/requirements.txt Show resolved Hide resolved

examples/opentracing/requirements.txt Show resolved Hide resolved

a-feld removed their request for review December 10, 2019 00:55

c24t requested a review from a team December 20, 2019 00:52

c24t and others added 3 commits December 20, 2019 10:38

Add opentracing shim example

c3e965d

Move images

73f9c65

Test

6ffa86c

ocelotl force-pushed the opentracing-shim-example branch from 009aaa2 to 6ffa86c Compare December 20, 2019 16:40

ocelotl added 4 commits December 20, 2019 11:03

Add minor fixes

2396db1

More pylint fixes

79912de

Update example to latest OpenTelemetry changes

bb87f34

More pylint fixes

255d2a5

ocelotl added ext needs reviewers PRs with this label are ready for review and needs people to review to move forward. labels Dec 20, 2019

ocelotl reviewed Dec 20, 2019

View reviewed changes

codeboten approved these changes Jan 8, 2020

View reviewed changes

ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py Outdated Show resolved Hide resolved

ext/opentelemetry-ext-opentracing-shim/tests/test_shim.py Outdated Show resolved Hide resolved

mauriciovasquezbernal reviewed Jan 17, 2020

View reviewed changes

examples/opentracing/README.md Show resolved Hide resolved

mauriciovasquezbernal reviewed Jan 17, 2020

View reviewed changes

examples/opentracing/main.py Outdated Show resolved Hide resolved

c24t added 7 commits January 22, 2020 15:29

Merge branch 'master' into opentracing-shim-example

d581134

Update example for TracerSource

a552ab0

Use SimpleExportSpanProcessor in example

4197f02

Revert import changes to shim

5a224ad

Remove processor shutdown call from example

1c49797

Update example README to match requirements file

21390e9

Revert "Revert import changes to shim"

9d3ef06

This reverts commit 5a224ad.

c24t mentioned this pull request Jan 23, 2020

Change imports to avoid pylint namespace errors #376

Merged

Merge branch 'master' into opentracing-shim-example

de8823b

mauriciovasquezbernal added the shim OpenTracing or OpenCensus compatibility label Jan 28, 2020

Merge branch 'master' into opentracing-shim-example

706f3d6

Oberon00 approved these changes Feb 4, 2020

View reviewed changes

examples/opentracing/README.md Outdated Show resolved Hide resolved

mauriciovasquezbernal reviewed Feb 4, 2020

View reviewed changes

examples/opentracing/rediscache.py Outdated Show resolved Hide resolved

c24t and others added 4 commits February 4, 2020 11:44

Update examples/opentracing/README.md

0328684

Co-Authored-By: Christian Neumüller <christian+github@neumueller.me>

Update examples/opentracing/rediscache.py

b348075

Co-Authored-By: Mauricio Vásquez <mauricio@kinvolk.io>

Revert "Revert "Revert import changes to shim""

0239c08

This reverts commit 9d3ef06.

Merge branch 'master' into opentracing-shim-example

e9eaa5e

mauriciovasquezbernal approved these changes Feb 4, 2020

View reviewed changes

mauriciovasquezbernal reviewed Feb 4, 2020

View reviewed changes

examples/opentelemetry-example-app/tests/test_flask_example.py Outdated Show resolved Hide resolved

Obey isort

73bada3

c24t merged commit ae484cb into open-telemetry:master Feb 4, 2020

c24t deleted the opentracing-shim-example branch February 4, 2020 20:37

srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this pull request Nov 1, 2020

feat(exporter-jaeger): flush data before shutdown (open-telemetry#282)

0c61fc2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add OpenTracing shim example #282

Add OpenTracing shim example #282

c24t commented Nov 11, 2019 •

edited

Loading

codecov-io commented Nov 11, 2019 •

edited

Loading

mauriciovasquezbernal left a comment

codeboten commented Nov 12, 2019

ocelotl left a comment

codeboten commented Jan 8, 2020

mauriciovasquezbernal commented Jan 17, 2020

c24t commented Jan 23, 2020

mauriciovasquezbernal commented Jan 23, 2020

c24t commented Jan 29, 2020

Oberon00 left a comment

mauriciovasquezbernal left a comment •

edited

Loading

c24t commented Feb 4, 2020

mauriciovasquezbernal left a comment

Add OpenTracing shim example #282

Add OpenTracing shim example #282

Conversation

c24t commented Nov 11, 2019 • edited Loading

codecov-io commented Nov 11, 2019 • edited Loading

Codecov Report

mauriciovasquezbernal left a comment

Choose a reason for hiding this comment

codeboten commented Nov 12, 2019

ocelotl left a comment

Choose a reason for hiding this comment

codeboten commented Jan 8, 2020

mauriciovasquezbernal commented Jan 17, 2020

c24t commented Jan 23, 2020

mauriciovasquezbernal commented Jan 23, 2020

c24t commented Jan 29, 2020

Oberon00 left a comment

Choose a reason for hiding this comment

mauriciovasquezbernal left a comment • edited Loading

Choose a reason for hiding this comment

c24t commented Feb 4, 2020

mauriciovasquezbernal left a comment

Choose a reason for hiding this comment

c24t commented Nov 11, 2019 •

edited

Loading

codecov-io commented Nov 11, 2019 •

edited

Loading

mauriciovasquezbernal left a comment •

edited

Loading