Skip to content
New issue

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

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

Already on GitHub? # to your account

Reorder on_finish call order to correctly instrument all tornado work done during a request #499

Merged
merged 8 commits into from
Jun 7, 2021

Conversation

pitabwire
Copy link
Contributor

@pitabwire pitabwire commented May 16, 2021

Description

This PR ensures trace processing is closed only after tornado's on_finish method is called

Fixes #495

Type of change

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

Verified

This commit was signed with the committer’s verified signature.
@pitabwire pitabwire requested review from a team, codeboten and ocelotl and removed request for a team May 16, 2021 08:45
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented May 16, 2021

CLA Signed

The committers are authorized under a signed CLA.

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @pitabwire, thanks for the PR. Is it possible to add a test to capture the issue being fixed here?

Also please add an entry to CHANGELOG.md.

Copy link
Contributor

@ocelotl ocelotl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat 😎

Yes, please add a test, as requested by @codeboten ✌️

pitabwire added 3 commits June 2, 2021 16:32

Verified

This commit was signed with the committer’s verified signature.
@pitabwire pitabwire requested review from codeboten and ocelotl June 2, 2021 14:07
Copy link
Contributor

@ocelotl ocelotl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice ✌️

@lzchen
Copy link
Contributor

lzchen commented Jun 4, 2021

@pitabwire
Please fix the build errors and then we can merge this in!

@pitabwire pitabwire requested a review from lzchen June 7, 2021 12:18
@codeboten codeboten merged commit fe4e2d4 into open-telemetry:main Jun 7, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tornado instrumentation closes spans before on_finish is completed.
5 participants