opentelemetry-python icon indicating copy to clipboard operation
opentelemetry-python copied to clipboard

Make `tracer.start_as_current_span()` decorator work with async functions

Open QuentinN42 opened this issue 1 year ago • 4 comments

Description

Fixing the start_as_current_span decorator that report near zero time for spans of decorated async functions.

Fixes #3270

Type of change

Please delete options that are not relevant.

  • [x] Bug fix
  • [ ] New feature
  • [ ] Breaking change : It's not a breaking change as the other libraries that implement their own Tracer class can still use the contextlib decorator (but it will still be bugged for them), as soon as they implement it, it must be working fine on their end two.
  • [ ] This change requires a documentation update

How Has This Been Tested?

  • [x] opentelemetry-api/tests/trace/test_tracer.py : Created the minimal reproduction and fixed it.

Does This PR Require a Contrib Repo Change?

  • [x] No.

Checklist:

  • [x] Followed the style guidelines of this project
  • [x] Changelogs have been updated
  • [x] Unit tests have been added
  • [x] Documentation has been updated - nothing to change

QuentinN42 avatar Jan 12 '24 23:01 QuentinN42

  • [x] I've added comments and tests.
  • [x] I've also tested mypy in both version 3.8 and 3.10.

PS: I still don't really understand how tox works, sorry in advance if the pipeline is still failing :(

QuentinN42 avatar Feb 15 '24 18:02 QuentinN42

@QuentinN42 the current CI failure "metrics/integration_test/test_console_exporter.py - TypeError: 'ABCMeta... is not subscritable" I think can be solved by quoting the type annotations

aabmass avatar Feb 16 '24 17:02 aabmass

I've added some code from contextlib in this commit acd649dff0ff3f28d63d91af6d6795f099a7caa4 to allow the linter to know that we are returning a Span in the with expression :

import opentelemetry.trace

TRACER = opentelemetry.trace.get_tracer(__name__)

with TRACER.start_as_current_span("foo") as span:
    print(span.get_span_context().trace_id)

QuentinN42 avatar Feb 16 '24 21:02 QuentinN42

I've also added comment on the tricky typing part with relevant python versions.

QuentinN42 avatar Feb 16 '24 21:02 QuentinN42

I've applied all asked changes and also merged main into my branch :)

QuentinN42 avatar Mar 16 '24 10:03 QuentinN42

Marking it as draft to prevent accidental merging. @QuentinN42 please check that this PR is ok, I have mistakenly force pushed changes that may have affected it, apologies for the mistake.

ocelotl avatar Mar 19 '24 00:03 ocelotl

Just checked right now with my local branch. All seems good :+1:

QuentinN42 avatar Mar 19 '24 21:03 QuentinN42

Just checked right now with my local branch. All seems good 👍

Thanks!

ocelotl avatar Mar 20 '24 16:03 ocelotl

@ocelotl When can we expect a new version of opentelemetry-api with this fix to be released?

ofekashery avatar Mar 25 '24 20:03 ofekashery