fix(celery): handle upstream celery patch and propagation during an e… #11272

tabgok · 2024-11-01T19:21:43Z

Prior to this change two things weren't working:

An upstream Celery patch broke context propagation due to the way ddtrace propagates headers. Prior to this upstream patch, context had to be propagated under kwargs[headers][headers], but after the patch it can be propagated under kwargs[headers]. We now look in the right places for propagated information and retrieve from the right places.
Prior to this patch, If Task-A calls Task-B and Task-A errors out before Task-B creates its span, then Task-B would fail to parent itself on Task-A (instead going one level up). This caused errant service-entry celery.apply spans on workers, which should always have celery.run service entry spans.

Checklist

PR author has checked that all the criteria below are met
The PR description includes an overview of the change
The PR description articulates the motivation for the change
The change includes tests OR the PR description describes a testing strategy
The PR description notes risks associated with the change, if any
Newly-added code is easy to change
The change follows the library release note guidelines
The change includes or references documentation updates if necessary
Backport labels are set (if applicable)

Reviewer Checklist

Reviewer has checked that all the criteria below are met
Title is accurate
All changes are related to the pull request's stated goal
Avoids breaking API changes
Testing strategy adequately addresses listed risks
Newly-added code is easy to change
Release note makes sense to a user of the library
If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment
Backport labels are set in a manner that is consistent with the release branch maintenance policy

github-actions · 2024-11-01T19:22:13Z

CODEOWNERS have been resolved as:

releasenotes/notes/fix-celery-context-propagation-aa904d748362f2d1.yaml  @DataDog/apm-python
ddtrace/contrib/celery/__init__.py                                      @DataDog/apm-core-python @DataDog/apm-idm-python
ddtrace/contrib/internal/celery/constants.py                            @DataDog/apm-core-python @DataDog/apm-idm-python
ddtrace/contrib/internal/celery/signals.py                              @DataDog/apm-core-python @DataDog/apm-idm-python
ddtrace/contrib/internal/celery/utils.py                                @DataDog/apm-core-python @DataDog/apm-idm-python
tests/contrib/celery/base.py                                            @DataDog/apm-core-python @DataDog/apm-idm-python
tests/contrib/celery/test_integration.py                                @DataDog/apm-core-python @DataDog/apm-idm-python

ddtrace/contrib/internal/celery/utils.py

pr-commenter · 2024-11-01T20:01:05Z

Benchmarks

Benchmark execution time: 2024-11-14 15:25:02

Comparing candidate commit 09cb50d in PR branch teague.bick/APMS-13537/celery-async-fix with baseline commit ef58a6b in branch main.

Found 5 performance improvements and 0 performance regressions! Performance is the same for 383 metrics, 2 unstable metrics.

scenario:iast_aspects-split_aspect

🟩 execution_time [-241.002ns; -215.655ns] or [-12.624%; -11.296%]

scenario:iast_aspects-splitlines_aspect

🟩 execution_time [-188.291ns; -163.862ns] or [-10.140%; -8.824%]

scenario:iast_aspects-swapcase_aspect

🟩 execution_time [-380.309ns; -324.449ns] or [-11.677%; -9.962%]

scenario:iast_aspects-title_aspect

🟩 execution_time [-356.240ns; -309.975ns] or [-11.098%; -9.657%]

scenario:iast_aspects-upper_aspect

🟩 execution_time [-429.601ns; -386.434ns] or [-13.834%; -12.444%]

datadog-dd-trace-py-rkomorn · 2024-11-01T20:14:12Z

Datadog Report

Branch report: teague.bick/APMS-13537/celery-async-fix
Commit report: 09cb50d
Test service: dd-trace-py

✅ 0 Failed, 592 Passed, 604 Skipped, 19m 48s Total duration (16m 22.28s time saved)

tests/contrib/celery/test_integration.py

releasenotes/notes/fix-elasticsearch-always-populate-span-tags-aa904d748362f2d1.yaml

tests/contrib/celery/test_integration.py

tabgok · 2024-11-12T00:06:10Z

/merge

tabgok · 2024-11-12T00:06:13Z

/merge

dd-devflow · 2024-11-12T00:06:17Z

Devflow running: `/merge`

View all feedbacks in Devflow UI.

2024-11-12 00:06:17 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

Use /merge -c to cancel this operation!

⏳ command still in progress ...

dd-devflow · 2024-11-12T00:06:19Z

Devflow running: `/merge`

View all feedbacks in Devflow UI.

2024-11-12 00:06:18 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2024-11-12 04:06:21 UTC ⚠️ MergeQueue: This merge request was unqueued

This merge request was unqueued

tabgok · 2024-11-12T15:04:26Z

/merge

dd-devflow · 2024-11-12T15:05:44Z

Devflow running: `/merge`

View all feedbacks in Devflow UI.

2024-11-12 15:04:43 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2024-11-12 19:04:48 UTC ⚠️ MergeQueue: This merge request was unqueued

This merge request was unqueued

…rror Prior to this change two things weren't working: 1. An upstream Celery patch broke context propagation due to the way ddtrace propagates headers. Prior to this upstream patch, context had to be propagated under kwargs[headers][headers], but after the patch it can be propagated under kwargs[headers]. We now look in the right places for propagated information and retrieve from the right places. 2. Prior to this patch, If Task-A calls Task-B and Task-A errors out before Task-B creates its span, then Task-B would fail to parent itself on Task-A (instead going one level up). This caused errant service-entry `celery.apply` spans on workers, which should always have `celery.run` service entry spans.

tabgok · 2024-11-12T19:16:15Z

/merge

dd-devflow · 2024-11-12T19:16:27Z

Devflow running: `/merge`

View all feedbacks in Devflow UI.

2024-11-12 19:16:27 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2024-11-12 23:16:33 UTC ⚠️ MergeQueue: This merge request was unqueued

This merge request was unqueued

tabgok force-pushed the teague.bick/APMS-13537/celery-async-fix branch from c112d7b to 0c78226 Compare November 1, 2024 19:29

tabgok self-assigned this Nov 1, 2024

datadog-datadog-prod-us1 bot reviewed Nov 1, 2024

View reviewed changes

ddtrace/contrib/internal/celery/utils.py Outdated Show resolved Hide resolved

tabgok force-pushed the teague.bick/APMS-13537/celery-async-fix branch from c24aa4c to 28516ce Compare November 5, 2024 21:02

datadog-datadog-prod-us1 bot reviewed Nov 5, 2024

View reviewed changes

tests/contrib/celery/test_integration.py Show resolved Hide resolved

tabgok force-pushed the teague.bick/APMS-13537/celery-async-fix branch 4 times, most recently from 6855260 to 3a041e2 Compare November 6, 2024 19:18

tabgok marked this pull request as ready for review November 6, 2024 20:19

tabgok requested review from a team as code owners November 6, 2024 20:19

tabgok requested review from wconti27, mabdinur and erikayasuda November 6, 2024 20:19

timmc-edx mentioned this pull request Nov 6, 2024

Fix orphaned spans on Celery workers edx/edx-arch-experiments#822

Open

wantsui reviewed Nov 6, 2024

View reviewed changes

releasenotes/notes/fix-elasticsearch-always-populate-span-tags-aa904d748362f2d1.yaml Outdated Show resolved Hide resolved

emmettbutler approved these changes Nov 7, 2024

View reviewed changes

wantsui approved these changes Nov 8, 2024

View reviewed changes

quinna-h reviewed Nov 11, 2024

View reviewed changes

tests/contrib/celery/test_integration.py Show resolved Hide resolved

tests/contrib/celery/test_integration.py Show resolved Hide resolved

dd-devflow bot added the mergequeue-status: waiting label Nov 12, 2024

tabgok enabled auto-merge (squash) November 12, 2024 00:06

tabgok disabled auto-merge November 12, 2024 00:06

dd-devflow bot added mergequeue-status: removed and removed mergequeue-status: waiting labels Nov 12, 2024

dd-devflow bot added mergequeue-status: waiting and removed mergequeue-status: removed labels Nov 12, 2024

tabgok added 5 commits November 12, 2024 10:13

reno update

74d67d2

Update docstrings and release note name

aee8da0

capitalization

f6a9786

fix lint errors

b456652

tabgok force-pushed the teague.bick/APMS-13537/celery-async-fix branch from 3baee48 to b456652 Compare November 12, 2024 15:13

dd-devflow bot added mergequeue-status: removed and removed mergequeue-status: waiting labels Nov 12, 2024

dd-devflow bot added mergequeue-status: waiting mergequeue-status: removed and removed mergequeue-status: removed mergequeue-status: waiting labels Nov 12, 2024

Merge branch 'main' into teague.bick/APMS-13537/celery-async-fix

2b66ff5

tabgok enabled auto-merge (squash) November 13, 2024 15:23

tabgok added 5 commits November 13, 2024 11:39

Merge branch 'main' into teague.bick/APMS-13537/celery-async-fix

1d8a722

Merge branch 'main' into teague.bick/APMS-13537/celery-async-fix

90916b5

Merge branch 'main' into teague.bick/APMS-13537/celery-async-fix

27ea089

Merge branch 'main' into teague.bick/APMS-13537/celery-async-fix

16f1100

Merge branch 'main' into teague.bick/APMS-13537/celery-async-fix

09cb50d

tabgok merged commit c7246a4 into main Nov 14, 2024
546 of 548 checks passed

tabgok deleted the teague.bick/APMS-13537/celery-async-fix branch November 14, 2024 16:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(celery): handle upstream celery patch and propagation during an e… #11272

fix(celery): handle upstream celery patch and propagation during an e… #11272

tabgok commented Nov 1, 2024 •

edited by wantsui

Loading

github-actions bot commented Nov 1, 2024 •

edited

Loading

pr-commenter bot commented Nov 1, 2024 •

edited

Loading

datadog-dd-trace-py-rkomorn bot commented Nov 1, 2024 •

edited

Loading

tabgok commented Nov 12, 2024

tabgok commented Nov 12, 2024

dd-devflow bot commented Nov 12, 2024

dd-devflow bot commented Nov 12, 2024 •

edited

Loading

tabgok commented Nov 12, 2024

dd-devflow bot commented Nov 12, 2024 •

edited

Loading

tabgok commented Nov 12, 2024

dd-devflow bot commented Nov 12, 2024 •

edited

Loading

fix(celery): handle upstream celery patch and propagation during an e… #11272

fix(celery): handle upstream celery patch and propagation during an e… #11272

Conversation

tabgok commented Nov 1, 2024 • edited by wantsui Loading

Checklist

Reviewer Checklist

github-actions bot commented Nov 1, 2024 • edited Loading

pr-commenter bot commented Nov 1, 2024 • edited Loading

Benchmarks

scenario:iast_aspects-split_aspect

scenario:iast_aspects-splitlines_aspect

scenario:iast_aspects-swapcase_aspect

scenario:iast_aspects-title_aspect

scenario:iast_aspects-upper_aspect

datadog-dd-trace-py-rkomorn bot commented Nov 1, 2024 • edited Loading

Datadog Report

tabgok commented Nov 12, 2024

tabgok commented Nov 12, 2024

dd-devflow bot commented Nov 12, 2024

Devflow running: /merge

dd-devflow bot commented Nov 12, 2024 • edited Loading

Devflow running: /merge

tabgok commented Nov 12, 2024

dd-devflow bot commented Nov 12, 2024 • edited Loading

Devflow running: /merge

tabgok commented Nov 12, 2024

dd-devflow bot commented Nov 12, 2024 • edited Loading

Devflow running: /merge

tabgok commented Nov 1, 2024 •

edited by wantsui

Loading

github-actions bot commented Nov 1, 2024 •

edited

Loading

pr-commenter bot commented Nov 1, 2024 •

edited

Loading

datadog-dd-trace-py-rkomorn bot commented Nov 1, 2024 •

edited

Loading

Devflow running: `/merge`

dd-devflow bot commented Nov 12, 2024 •

edited

Loading

Devflow running: `/merge`

dd-devflow bot commented Nov 12, 2024 •

edited

Loading

Devflow running: `/merge`

dd-devflow bot commented Nov 12, 2024 •

edited

Loading

Devflow running: `/merge`