Mauricio/add testbed for otshim#727
Merged
toumorokoshi merged 14 commits intoopen-telemetry:masterfrom Jun 3, 2020
Merged
Conversation
This commit ports the OpenTracing testbed[1] to check that the ot-shim is working as expected using different frameworks. Gevent doesn't support context vars yet[2], so those tests are not compatible with opentelemetry and were not ported. [1] https://github.com/opentracing/opentracing-python/tree/master/testbed [2] gevent/gevent#1407
In asyncio it is not needed to activate the span as the context is handled using contextvars in this case. This commit makes that a good solution and adds some comments to clarify why it is a good solution and why the thread is a bad one.
naming the tests module as a namespace package ensures that relative imports will resolve properly for other test packages, as it enables searching for a composite of multiple test modules. only the opentelemetry-api directory needs this code, as it is the first tests module found by pylint during eachdist.py lint Please enter the commit message for your changes. Lines starting with '#' will be ignored, and an empty message aborts the commit.
toumorokoshi
commented
May 30, 2020
| # | ||
| # only the opentelemetry-api directory needs this code, as it is | ||
| # the first tests module found by pylint during eachdist.py lint | ||
| pkg_resources.declare_namespace(__name__) |
Member
Author
There was a problem hiding this comment.
this can be removed once this bug is fixed / clarified: pylint-dev/pylint#3609
Member
Author
|
@carlosalberto can you approve this PR? this is a re-creation of PR #274, I just fixed it up to work with CI again. @c24t @codeboten if one of you feel comfortable doing a proxy approval for my previous approval, that would be great. |
codeboten
approved these changes
Jun 2, 2020
Contributor
codeboten
left a comment
codeboten
left a comment
There was a problem hiding this comment.
LGTM, thanks for picking this up!
c24t
approved these changes
Jun 2, 2020
Member
c24t
left a comment
c24t
left a comment
There was a problem hiding this comment.
LGTM to go ahead, but I think there's still good reason to put this in a separate repo like the trace context integration tests.
toumorokoshi
added a commit
to toumorokoshi/opentelemetry-python
that referenced
this pull request
Jun 5, 2020
This commit ports the OpenTracing testbed[1] to check that the ot-shim is working as expected using different frameworks. Gevent doesn't support context vars yet[2], so those tests are not compatible with opentelemetry and were not ported. [1] https://github.com/opentracing/opentracing-python/tree/master/testbed [2] gevent/gevent#1407 Co-authored-by: Mauricio Vásquez <mauricio@kinvolk.io> Co-authored-by: alrex <aboten@lightstep.com>
sethmaxwl
pushed a commit
to sethmaxwl/opentelemetry-python
that referenced
this pull request
Jun 5, 2020
This commit ports the OpenTracing testbed[1] to check that the ot-shim is working as expected using different frameworks. Gevent doesn't support context vars yet[2], so those tests are not compatible with opentelemetry and were not ported. [1] https://github.com/opentracing/opentracing-python/tree/master/testbed [2] gevent/gevent#1407 Co-authored-by: Mauricio Vásquez <mauricio@kinvolk.io> Co-authored-by: alrex <aboten@lightstep.com>
srikanthccv
pushed a commit
to srikanthccv/opentelemetry-python
that referenced
this pull request
Nov 1, 2020
* feat: create an api package * chore: update circle for new api package * chore: bring back getTracer * chore: add wrongly removed dev dependency * chore: review comments * chore: review comments * chore: lint * chore: export all noop implementations * chore: update API README * chore: ignore known working links that are not yet published * chore: add jsdoc for getInstance calls * chore: add jsdoc for private constructors * chore: review comments * chore: fix readme npm url * chore: fix old readmes without registry * chore: update api calling convention
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
re-creation of #274 as I'm still figuring out how to push to a fork. Includes fixes to get tests to pass.