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

Bugfix/check future cancelled

Open bourbonkk opened this issue 1 year ago • 2 comments
trafficstars

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Using python auto instrumentation version 0.44b0 the real trace source is being deprecated. Calling the exception() method when future is in the cancelled state is causing a CancelledError

Fixes #2460

Type of change

Please delete options that are not relevant.

  • [x] Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • [x] Execute Test Code
  • [x] gitflow validation

Does This PR Require a Core Repo Change?

  • [ ] Yes. - Link to PR:
  • [x] No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

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

bourbonkk avatar Apr 26 '24 01:04 bourbonkk

Could you please add a test?

Also #2392 looks a different solution of the same issue, like they choose to always pass an exception

xrmx avatar Apr 26 '24 08:04 xrmx

Could you please add a test?

Also #2392 looks a different solution of the same issue, like they choose to always pass an exception

@xrmx i've looked into the issue. It's good code, but I have a slightly different opinion. I don't report canceledError as an error in the span because there is a subject that did the cancel, and the coroutine follows the same strategy.

        # CancelledError is raised when a coroutine is cancelled
        # before it has a chance to run. We don't want to record
        # this as an error.

bourbonkk avatar Apr 26 '24 17:04 bourbonkk