Skip to content

Conversation

@goatsthatcode
Copy link
Owner

@goatsthatcode goatsthatcode commented Apr 6, 2022

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.

Fixes open-telemetry#1002

Type of change

Please delete options that are not relevant.

  • 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

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

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

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

Copy link

@blumamir blumamir left a comment

Choose a reason for hiding this comment

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

I am too interested in a fix for this issue.
Thanks for working on a PR. Do you have any estimate on when this could be finalized, and if you plan to upstream it to the contrib repo?

and cannot run `task.apply_async()`
"""
if task is None:
return
Copy link

@blumamir blumamir Apr 12, 2022

Choose a reason for hiding this comment

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

I think the effect of this return statement is that the span (along with activation) will not be recorded into the CTX_KEY, which means that a later invocation of the _trace_after_publish signal will not be able to retrieve_span thus will not end the span.
Am I missing something?

Copy link
Owner Author

Choose a reason for hiding this comment

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

so I tested this against the following repo https://github.com/goatsthatcode/opentelemetry-instrumentation-celery-example which was reproducing the disconnected scan use case. As far as I can tell it seems to be nesting and closing spans correctly but I could be missing something as well. I just opened the upstream PR (moved this to my company GitHub since that was the request from my company) here open-telemetry#1052

@goatsthatcode
Copy link
Owner Author

hey @blumamir sorry I got really sick this last week, I'm picking this up again now and can get this all wrapped up today/tomorrow. Please let me know if there is anything else missing other than re-running the codegen stuff and ensuring the contributing.md is updated as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using CeleryInstrumentor and linking Celery task trace back to requested trace

3 participants