Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

document pitfalls, e.g. confusion between tracer.Tracer() and tracer.tracer() #110

Closed
alban opened this issue Aug 27, 2019 · 5 comments
Closed
Labels
api Affects the API package. doc Documentation-related tracing usability
Milestone

Comments

@alban
Copy link
Contributor

alban commented Aug 27, 2019

Is your feature request related to a problem?

Yes, I initially tried the example from the README.md:

trace.set_preferred_tracer_implementation(lambda T: Tracer())
tracer = trace.tracer()
with tracer.start_span('foo'):
    print(Context)
    with tracer.start_span('bar'):
        print(Context)
        with tracer.start_span('baz'):
            print(Context)
        print(Context)
    print(Context)

But I made a typo with the casing: I typed trace.Tracer() (the constructor for the abstract class Tracer) instead of trace.tracer() (returning the preferred implementation, configurable with set_preferred_tracer_implementation()). The output ended up as the following:

AsyncRuntimeContext({'current_span': None})
AsyncRuntimeContext({'current_span': None})
AsyncRuntimeContext({'current_span': None})
AsyncRuntimeContext({'current_span': None})
AsyncRuntimeContext({'current_span': None})

After fixing the typo, the correct output is displayed:

AsyncRuntimeContext({'current_span': Span(name="foo")})
AsyncRuntimeContext({'current_span': Span(name="bar")})
AsyncRuntimeContext({'current_span': Span(name="baz")})
AsyncRuntimeContext({'current_span': Span(name="bar")})
AsyncRuntimeContext({'current_span': Span(name="foo")})

It confused me at first that there was no error message from the typo.

Describe the solution you'd like

Maybe some documentation docs/pitfalls.md with a checklist of symptoms and common mistakes.

@reyang reyang added api Affects the API package. doc Documentation-related tracing labels Aug 27, 2019
@toumorokoshi
Copy link
Member

Would it be better to rename of modify the API to eliminate the pitfall? Docs are good but oftentimes they are not read or missed.

@carlosalberto
Copy link
Contributor

Actually for this reason (and a few others) we have global_tracer() instead of tracer() in OpenTracing. Maybe get_tracer() would be ok for us?

@toumorokoshi
Copy link
Member

how about get_global_tracer()? I like the explicit "global" in the name.

@Oberon00
Copy link
Member

Oberon00 commented Sep 9, 2019

Depending on #123, globals().tracer() sounds good.

@c24t c24t added this to the Alpha v0.3 milestone Oct 11, 2019
@c24t c24t modified the milestones: Alpha v0.3, Alpha v0.4 Dec 5, 2019
@c24t c24t added the usability label Dec 5, 2019
@c24t
Copy link
Member

c24t commented Dec 19, 2019

This is covered now in #123.

@c24t c24t closed this as completed Dec 19, 2019
srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this issue Nov 1, 2020
* Add Plugin interface

* Alter moduleExports comment

* export Plugin

* Add BasePlugin

* rename applyPatch to patch and applyUnpatch to unpatch

* Add an optional [config] object

* style: add underscore prefix for protected properties
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Affects the API package. doc Documentation-related tracing usability
Projects
None yet
Development

No branches or pull requests

6 participants