Skip to content

Commit

Permalink
feat(celery): flatten context delivery_info tags (#4772)
Browse files Browse the repository at this point in the history
## 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 <brett.langdon@datadoghq.com>
Co-authored-by: Gabriele N. Tornetta <P403n1x87@users.noreply.github.com>
Co-authored-by: Kyle Verhoog <kyle@verhoog.ca>
  • Loading branch information
4 people authored Dec 19, 2022
1 parent 6cb75c6 commit 95d071e
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 14 deletions.
35 changes: 23 additions & 12 deletions ddtrace/contrib/celery/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from typing import Dict
from weakref import WeakValueDictionary

from ddtrace.contrib.trace_utils import set_flattened_tags
from ddtrace.span import Span

from .constants import CTX_KEY
Expand Down Expand Up @@ -33,27 +34,37 @@
)


def should_skip_context_value(key, value):
# type: (str, Any) -> bool
# Skip this key if it is not set
if value is None or value == "":
return True

# Skip `timelimit` if it is not set (its default/unset value is a
# tuple or a list of `None` values
if key == "timelimit" and all(_ is None for _ in value):
return True

# Skip `retries` if its value is `0`
if key == "retries" and value == 0:
return True

return False


def set_tags_from_context(span, context):
# type: (Span, Dict[str, Any]) -> None
"""Helper to extract meta values from a Celery Context"""

context_tags = []
for key, tag_name in TAG_KEYS:
value = context.get(key)

# Skip this key if it is not set
if value is None or value == "":
continue

# Skip `timelimit` if it is not set (its default/unset value is a
# tuple or a list of `None` values
if key == "timelimit" and all(_ is None for _ in value):
if should_skip_context_value(key, value):
continue

# Skip `retries` if its value is `0`
if key == "retries" and value == 0:
continue
context_tags.append((tag_name, value))

span.set_tag(tag_name, value)
set_flattened_tags(span, context_tags)


def attach_span(task, task_id, span, is_publish=False):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
features:
- |
celery: Enhances context tags containing dictionaries so that their contents are sent as individual tags (issue #4771).
6 changes: 4 additions & 2 deletions tests/contrib/celery/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def test_tags_from_context(self):
# it should extract only relevant keys
context = {
"correlation_id": "44b7f305",
"delivery_info": '{"eager": "True"}',
"delivery_info": {"eager": "True", "priority": "0", "int_zero": 0},
"eta": "soon",
"expires": "later",
"hostname": "localhost",
Expand All @@ -36,7 +36,9 @@ def test_tags_from_context(self):
metrics = span.get_metrics()
sentinel = object()
assert metas["celery.correlation_id"] == "44b7f305"
assert metas["celery.delivery_info"] == '{"eager": "True"}'
assert metas["celery.delivery_info.eager"] == "True"
assert metas["celery.delivery_info.priority"] == "0"
assert metrics["celery.delivery_info.int_zero"] == 0
assert metas["celery.eta"] == "soon"
assert metas["celery.expires"] == "later"
assert metas["celery.hostname"] == "localhost"
Expand Down

0 comments on commit 95d071e

Please sign in to comment.