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

Celery contrib: read custom parent span id from the task headers #1500

Merged
merged 8 commits into from
Mar 29, 2022

Conversation

namoshizun
Copy link
Contributor

@namoshizun namoshizun commented Mar 26, 2022

What does this pull request do?

In a complex Celery workflow, tasks might be invoked in a chain like this:

<A> ---> <B> ---> <C> ... --> <Z>

In such setting, because the current Celery instrument only propagates the first caller's traceparent, all downstream tasks share the same traceparent from A. This might be undesirable if every task should be regarded as a subtask of its caller.

For instance, "submit_task_for_computation" initiates "do_heavy_lifting" and waits for completion so that it can immediately rollback a database transaction if the job failed. But instead of correlate do_heavy_lifting with submit_task_for_computation, current instrument produces the following trace:

messy-spans

Whereas we actually hope for this:

nicely-aligned

This PR is to allow a caller task to set the receiver task's parent span by passing a parent_span_id in the Celery headers.

I have prepared this snippet as a demo.

@cla-checker-service
Copy link

cla-checker-service bot commented Mar 26, 2022

💚 CLA has been signed

@github-actions github-actions bot added agent-python community Issues opened by the community triage Issues awaiting triage labels Mar 26, 2022
@apmmachine
Copy link
Contributor

apmmachine commented Mar 26, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-03-29T18:04:21.125+0000

  • Duration: 20 min 45 sec

Test stats 🧪

Test Results
Failed 0
Passed 4773
Skipped 3209
Total 7982

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /test linters : Run the Python linters only.

  • /test full : Run the full matrix of tests.

  • /test benchmark : Run the APM Agent Python benchmarks tests.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

Copy link
Contributor

@basepi basepi left a comment

Choose a reason for hiding this comment

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

Looks great! We're fixing a CI failure caused by upstream changes in another library, and then we should be able to get the tests run here and merge this.

Can you add a note to the CHANGELOG?

elasticapm/contrib/celery/__init__.py Show resolved Hide resolved
@basepi
Copy link
Contributor

basepi commented Mar 28, 2022

/test

@basepi basepi requested a review from beniwohli March 28, 2022 15:41
@basepi
Copy link
Contributor

basepi commented Mar 29, 2022

run elasticsearch-ci/docs

@basepi basepi merged commit 7f9de0d into elastic:main Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-python community Issues opened by the community triage Issues awaiting triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants