Skip to content

Commit

Permalink
chore: deprecate the span= arguments on ExecutionContext methods (#7957)
Browse files Browse the repository at this point in the history
This pull request deprecates the `span` arguments on `ExecutionContext`
methods to indicate that they are only for backward compatibility and
not meant to be used by newly written code. This has been the plan since
the beginning of the core API in
#6202 (comment).

Deprecations are usually used for non-internal functionality, but I
think a deprecation is appropriate in this internal case as we encourage
broader usage of the core API.

## Checklist

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.
- [x] Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
- [x] If this PR touches code that signs or publishes builds or
packages, or handles credentials of any kind, I've requested a review
from `@DataDog/security-design-and-guidance`.
- [x] This PR doesn't touch any of that.
  • Loading branch information
emmettbutler committed Jan 3, 2024
1 parent 387aec4 commit 14c0a97
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
3 changes: 2 additions & 1 deletion .github/workflows/test_frameworks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,8 @@ jobs:
run: pip install ../ddtrace
- name: Test
if: needs.needs-run.outputs.outcome == 'success'
run: PYTHONPATH=../ddtrace/tests/debugging/exploration/ ddtrace-run pytest -p no:warnings tests
# https://github.com/tiangolo/fastapi/pull/10876
run: PYTHONPATH=../ddtrace/tests/debugging/exploration/ ddtrace-run pytest -p no:warnings tests -k 'not test_warn_duplicate_operation_id'
- name: Debugger exploration results
if: needs.needs-run.outputs.outcome == 'success'
run: cat debugger-expl.txt
Expand Down
26 changes: 26 additions & 0 deletions ddtrace/internal/core/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ def _on_jsonify_context_started_flask(ctx):
"""
from contextlib import contextmanager
import logging
import sys
from typing import TYPE_CHECKING # noqa:F401
from typing import Any # noqa:F401
from typing import Callable # noqa:F401
Expand All @@ -110,6 +111,9 @@ def _on_jsonify_context_started_flask(ctx):
from typing import Optional # noqa:F401
from typing import Tuple # noqa:F401

from ddtrace.vendor.debtcollector import deprecate

from ..utils.deprecations import DDTraceDeprecationWarning
from . import event_hub # noqa:F401
from .event_hub import EventResultDict # noqa:F401
from .event_hub import dispatch
Expand All @@ -134,12 +138,30 @@ def _on_jsonify_context_started_flask(ctx):

_CURRENT_CONTEXT = None
ROOT_CONTEXT_ID = "__root"
SPAN_DEPRECATION_MESSAGE = (
"The 'span' keyword argument on ExecutionContext methods is deprecated and will be removed in a future version."
)
SPAN_DEPRECATION_SUGGESTION = (
"Please store contextual data on the ExecutionContext object using other kwargs and/or set_item()"
)


def _deprecate_span_kwarg(span):
if span is not None:
# https://github.com/tiangolo/fastapi/pull/10876
if "fastapi" not in sys.modules and "fastapi.applications" not in sys.modules:
deprecate(
SPAN_DEPRECATION_MESSAGE,
message=SPAN_DEPRECATION_SUGGESTION,
category=DDTraceDeprecationWarning,
)


class ExecutionContext:
__slots__ = ["identifier", "_data", "_parents", "_span", "_token"]

def __init__(self, identifier, parent=None, span=None, **kwargs):
_deprecate_span_kwarg(span)
self.identifier = identifier
self._data = {}
self._parents = []
Expand Down Expand Up @@ -260,6 +282,7 @@ def context_with_data(identifier, parent=None, **kwargs):

def get_item(data_key, span=None):
# type: (str, Optional[Span]) -> Optional[Any]
_deprecate_span_kwarg(span)
if span is not None and span._local_root is not None:
return span._local_root._get_ctx_item(data_key)
else:
Expand All @@ -268,6 +291,7 @@ def get_item(data_key, span=None):

def get_items(data_keys, span=None):
# type: (List[str], Optional[Span]) -> Optional[Any]
_deprecate_span_kwarg(span)
if span is not None and span._local_root is not None:
return [span._local_root._get_ctx_item(key) for key in data_keys]
else:
Expand All @@ -282,6 +306,7 @@ def set_safe(data_key, data_value):
# NB Don't call these set_* functions from `ddtrace.contrib`, only from product code!
def set_item(data_key, data_value, span=None):
# type: (str, Optional[Any], Optional[Span]) -> None
_deprecate_span_kwarg(span)
if span is not None and span._local_root is not None:
span._local_root._set_ctx_item(data_key, data_value)
else:
Expand All @@ -290,6 +315,7 @@ def set_item(data_key, data_value, span=None):

def set_items(keys_values, span=None):
# type: (Dict[str, Optional[Any]], Optional[Span]) -> None
_deprecate_span_kwarg(span)
if span is not None and span._local_root is not None:
span._local_root._set_ctx_items(keys_values)
else:
Expand Down

0 comments on commit 14c0a97

Please sign in to comment.