Skip to content

Commit c79c8b4

Browse files
brettlangdonRyankeyP403n1x87Kyle-Verhoog
authored
feat(celery): flatten context delivery_info tags (backport #4772) (#4802)
## Description Tag data from celery is included as a raw string representation for `delivery_info`, which makes it impossible to use as a facet or measure in APM. The fix is necessary because routing info is valuable to have when understanding why delays happen between task creation and task execution. Issue #4771. This is only breaking for the tag output format from celery spans, and should only break any cases currently modifying or depending on dicts to be represented as raw strings. ## Checklist - [x] Followed the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines) when writing a release note. - [x] Add additional sections for `feat` and `fix` pull requests. - [x] [Library documentation](https://github.com/DataDog/dd-trace-py/tree/1.x/docs) and/or [Datadog's documentation site](https://github.com/DataDog/documentation/) is updated. Link to doc PR in description. <!-- Copy and paste the relevant snippet based on the type of pull request --> <!-- START fix --> ## Relevant issue(s) Fixes #4771 ## Testing strategy Modified existing unit tests to expect the new tag structure. Manually tested on project that included celery to ensure expected dictionary (`delivery_info`) would be flattened as expected. <!-- END fix --> ## Reviewer Checklist - [x] Title is accurate. - [x] Description motivates each change. - [x] No unnecessary changes were introduced in this PR. - [x] Avoid breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Tests provided or description of manual testing performed is included in the code or PR. - [x] Release note has been added and follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines), or else `changelog/no-changelog` label added. - [x] All relevant GitHub issues are correctly linked. - [x] Backports are identified and tagged with Mergifyio. Co-authored-by: Brett Langdon <[email protected]> Co-authored-by: Gabriele N. Tornetta <[email protected]> Co-authored-by: Kyle Verhoog <[email protected]> ## Description <!-- Briefly describe the change and why it was required. --> <!-- If this is a breaking change, explain why it is necessary. Breaking changes must append `!` after the type/scope. See https://ddtrace.readthedocs.io/en/stable/contributing.html for more details. --> ## Checklist - [ ] Followed the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines) when writing a release note. - [ ] Add additional sections for `feat` and `fix` pull requests. - [ ] [Library documentation](https://github.com/DataDog/dd-trace-py/tree/1.x/docs) and/or [Datadog's documentation site](https://github.com/DataDog/documentation/) is updated. Link to doc PR in description. <!-- Copy and paste the relevant snippet based on the type of pull request --> <!-- START feat --> ## Motivation <!-- Expand on why the change is required, include relevant context for reviewers --> ## Design <!-- Include benefits from the change as well as possible drawbacks and trade-offs --> ## Testing strategy <!-- Describe the automated tests and/or the steps for manual testing. <!-- END feat --> <!-- START fix --> ## Relevant issue(s) <!-- Link the pull request to any issues related to the fix. Use keywords for links to automate closing the issues once the pull request is merged. --> ## Testing strategy <!-- Describe any added regression tests and/or the manual testing performed. --> <!-- END fix --> ## Reviewer Checklist - [ ] Title is accurate. - [ ] Description motivates each change. - [ ] No unnecessary changes were introduced in this PR. - [ ] Avoid breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [ ] Tests provided or description of manual testing performed is included in the code or PR. - [ ] Release note has been added and follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines), or else `changelog/no-changelog` label added. - [ ] All relevant GitHub issues are correctly linked. - [ ] Backports are identified and tagged with Mergifyio. Co-authored-by: Ryankey <[email protected]> Co-authored-by: Gabriele N. Tornetta <[email protected]> Co-authored-by: Kyle Verhoog <[email protected]>
1 parent 3d49de5 commit c79c8b4

File tree

3 files changed

+30
-14
lines changed

3 files changed

+30
-14
lines changed

ddtrace/contrib/celery/utils.py

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from typing import Dict
33
from weakref import WeakValueDictionary
44

5+
from ddtrace.contrib.trace_utils import set_flattened_tags
56
from ddtrace.span import Span
67

78
from .constants import CTX_KEY
@@ -33,27 +34,37 @@
3334
)
3435

3536

37+
def should_skip_context_value(key, value):
38+
# type: (str, Any) -> bool
39+
# Skip this key if it is not set
40+
if value is None or value == "":
41+
return True
42+
43+
# Skip `timelimit` if it is not set (its default/unset value is a
44+
# tuple or a list of `None` values
45+
if key == "timelimit" and all(_ is None for _ in value):
46+
return True
47+
48+
# Skip `retries` if its value is `0`
49+
if key == "retries" and value == 0:
50+
return True
51+
52+
return False
53+
54+
3655
def set_tags_from_context(span, context):
3756
# type: (Span, Dict[str, Any]) -> None
3857
"""Helper to extract meta values from a Celery Context"""
3958

59+
context_tags = []
4060
for key, tag_name in TAG_KEYS:
4161
value = context.get(key)
42-
43-
# Skip this key if it is not set
44-
if value is None or value == "":
45-
continue
46-
47-
# Skip `timelimit` if it is not set (its default/unset value is a
48-
# tuple or a list of `None` values
49-
if key == "timelimit" and all(_ is None for _ in value):
62+
if should_skip_context_value(key, value):
5063
continue
5164

52-
# Skip `retries` if its value is `0`
53-
if key == "retries" and value == 0:
54-
continue
65+
context_tags.append((tag_name, value))
5566

56-
span.set_tag(tag_name, value)
67+
set_flattened_tags(span, context_tags)
5768

5869

5970
def attach_span(task, task_id, span, is_publish=False):
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
features:
2+
- |
3+
celery: Enhances context tags containing dictionaries so that their contents are sent as individual tags (issue #4771).

tests/contrib/celery/test_utils.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ def test_tags_from_context(self):
1919
# it should extract only relevant keys
2020
context = {
2121
"correlation_id": "44b7f305",
22-
"delivery_info": '{"eager": "True"}',
22+
"delivery_info": {"eager": "True", "priority": "0", "int_zero": 0},
2323
"eta": "soon",
2424
"expires": "later",
2525
"hostname": "localhost",
@@ -36,7 +36,9 @@ def test_tags_from_context(self):
3636
metrics = span.get_metrics()
3737
sentinel = object()
3838
assert metas["celery.correlation_id"] == "44b7f305"
39-
assert metas["celery.delivery_info"] == '{"eager": "True"}'
39+
assert metas["celery.delivery_info.eager"] == "True"
40+
assert metas["celery.delivery_info.priority"] == "0"
41+
assert metrics["celery.delivery_info.int_zero"] == 0
4042
assert metas["celery.eta"] == "soon"
4143
assert metas["celery.expires"] == "later"
4244
assert metas["celery.hostname"] == "localhost"

0 commit comments

Comments
 (0)