diff --git a/ddtrace/_hooks.py b/ddtrace/_hooks.py deleted file mode 100644 index 8aa17365b2d..00000000000 --- a/ddtrace/_hooks.py +++ /dev/null @@ -1,134 +0,0 @@ -import collections -from copy import deepcopy -from typing import Any # noqa:F401 -from typing import Callable # noqa:F401 -from typing import DefaultDict # noqa:F401 -from typing import Optional # noqa:F401 -from typing import Set # noqa:F401 - -from .internal.logger import get_logger - - -log = get_logger(__name__) - - -class Hooks: - """ - Hooks configuration object is used for registering and calling hook functions - - Example:: - - @config.falcon.hooks.on('request') - def on_request(span, request, response): - pass - """ - - _hooks: DefaultDict[str, Set] - __slots__ = ("_hooks",) - - def __init__(self): - self._hooks = collections.defaultdict(set) - - def __deepcopy__(self, memodict=None): - hooks = Hooks() - hooks._hooks = deepcopy(self._hooks, memodict) - return hooks - - def register( - self, - hook, # type: Any - func=None, # type: Optional[Callable] - ): - # type: (...) -> Optional[Callable[..., Any]] - """ - Function used to register a hook for the provided name. - - Example:: - - def on_request(span, request, response): - pass - - config.falcon.hooks.register('request', on_request) - - - If no function is provided then a decorator is returned:: - - @config.falcon.hooks.register('request') - def on_request(span, request, response): - pass - - :param hook: The name of the hook to register the function for - :type hook: object - :param func: The function to register, or ``None`` if a decorator should be returned - :type func: function, None - :returns: Either a function decorator if ``func is None``, otherwise ``None`` - :rtype: function, None - """ - # If they didn't provide a function, then return a decorator - if not func: - - def wrapper(func): - self.register(hook, func) - return func - - return wrapper - self._hooks[hook].add(func) - return None - - # Provide shorthand `on` method for `register` - # >>> @config.falcon.hooks.on('request') - # def on_request(span, request, response): - # pass - on = register - - def deregister( - self, - hook, # type: Any - func, # type: Callable - ): - # type: (...) -> None - """ - Function to deregister a function from a hook it was registered under - - Example:: - - @config.falcon.hooks.on('request') - def on_request(span, request, response): - pass - - config.falcon.hooks.deregister('request', on_request) - - :param hook: The name of the hook to register the function for - :type hook: object - :param func: Function hook to register - :type func: function - """ - if hook in self._hooks: - try: - self._hooks[hook].remove(func) - except KeyError: - pass - - def emit( - self, - hook, # type: Any - *args, # type: Any - **kwargs, # type: Any - ): - # type: (...) -> None - """ - Function used to call registered hook functions. - - :param hook: The hook to call functions for - :type hook: str - :param args: Positional arguments to pass to the hook functions - :type args: list - :param kwargs: Keyword arguments to pass to the hook functions - :type kwargs: dict - """ - # Call registered hooks - for func in self._hooks.get(hook, ()): - try: - func(*args, **kwargs) - except Exception: - log.error("Failed to run hook %s function %s", hook, func, exc_info=True) diff --git a/ddtrace/contrib/falcon.py b/ddtrace/contrib/falcon.py index c0b09c94e01..8cba178ef81 100644 --- a/ddtrace/contrib/falcon.py +++ b/ddtrace/contrib/falcon.py @@ -20,28 +20,6 @@ To disable distributed tracing when using autopatching, set the ``DD_FALCON_DISTRIBUTED_TRACING`` environment variable to ``False``. -**Supported span hooks** - -The following is a list of available tracer hooks that can be used to intercept -and modify spans created by this integration. - -- ``request`` - - Called before the response has been finished - - ``def on_falcon_request(span, request, response)`` - - -Example:: - - import ddtrace.auto - import falcon - from ddtrace import config - - app = falcon.API() - - @config.falcon.hooks.on('request') - def on_falcon_request(span, request, response): - span.set_tag('my.custom', 'tag') - :ref:`Headers tracing ` is supported for this integration. """ diff --git a/ddtrace/contrib/internal/falcon/middleware.py b/ddtrace/contrib/internal/falcon/middleware.py index 38f1c89fb3a..a689a767fe4 100644 --- a/ddtrace/contrib/internal/falcon/middleware.py +++ b/ddtrace/contrib/internal/falcon/middleware.py @@ -81,10 +81,6 @@ def process_response(self, req, resp, resource, req_succeeded=None): route = req.root_path or "" + req.uri_template - # Emit span hook for this response - # DEV: Emit before closing so they can overwrite `span.resource` if they want - config.falcon.hooks.emit("request", span, req, resp) - core.dispatch( "web.request.finish", (span, config.falcon, None, None, status, None, None, resp._headers, route, True) ) diff --git a/ddtrace/internal/settings/integration.py b/ddtrace/internal/settings/integration.py index 6cea1c33c75..f5ea0a93f43 100644 --- a/ddtrace/internal/settings/integration.py +++ b/ddtrace/internal/settings/integration.py @@ -1,8 +1,9 @@ import os from typing import Optional # noqa:F401 -from ddtrace._hooks import Hooks from ddtrace.internal.utils.attrdict import AttrDict +from ddtrace.internal.utils.deprecations import DDTraceDeprecationWarning +from ddtrace.vendor.debtcollector import deprecate from .http import HttpConfig @@ -36,8 +37,8 @@ def __init__(self, global_config, name, *args, **kwargs): # DEV: By-pass the `__setattr__` overrides from `AttrDict` to set real properties object.__setattr__(self, "global_config", global_config) object.__setattr__(self, "integration_name", name) - object.__setattr__(self, "hooks", Hooks()) object.__setattr__(self, "http", HttpConfig()) + object.__setattr__(self, "hooks", Hooks()) # Trace Analytics was removed in v3.0.0 # TODO(munir): Remove all references to analytics_enabled and analytics_sample_rate @@ -115,3 +116,42 @@ def copy(self): new_instance = self.__class__(self.global_config, self.integration_name) new_instance.update(self) return new_instance + + +class Hooks: + """Deprecated no-op Hooks class for backwards compatibility.""" + + def register(self, hook, func=None): + deprecate( + "Hooks.register() is deprecated and is currently a no-op.", + message="To interact with spans, use get_current_span() or get_current_root_span().", + removal_version="5.0.0", + category=DDTraceDeprecationWarning, + ) + if not func: + # Return a no-op decorator + def wrapper(func): + return func + + return wrapper + return None + + def on(self, hook, func=None): + return self.register(hook, func) + + def deregister(self, hook, func): + deprecate( + "Hooks.deregister() is deprecated and is currently a no-op.", + removal_version="5.0.0", + category=DDTraceDeprecationWarning, + ) + pass + + def emit(self, hook, *args, **kwargs): + deprecate( + "Hooks.emit() is deprecated", + message="Use tracer.current_span() or TraceFilters to retrieve and/or modify spans", + removal_version="5.0.0", + category=DDTraceDeprecationWarning, + ) + pass diff --git a/tests/contrib/falcon/test_suite.py b/tests/contrib/falcon/test_suite.py index 9771158e34d..3293259dd4b 100644 --- a/tests/contrib/falcon/test_suite.py +++ b/tests/contrib/falcon/test_suite.py @@ -224,27 +224,6 @@ def test_404_exception_no_stacktracer(self): assert span.get_tag("component") == "falcon" assert span.get_tag("span.kind") == "server" - def test_falcon_request_hook(self): - @config.falcon.hooks.on("request") - def on_falcon_request(span, request, response): - span.set_tag("my.custom", "tag") - - out = self.make_test_call("/200", expected_status_code=200) - assert out.content.decode("utf-8") == "Success" - - traces = self.tracer.pop_traces() - assert len(traces) == 1 - assert len(traces[0]) == 1 - span = traces[0][0] - assert span.get_tag("http.request.headers.my_header") is None - assert span.get_tag("http.response.headers.my_response_header") is None - - assert span.name == "falcon.request" - - assert span.get_tag("my.custom") == "tag" - - assert span.error == 0 - def test_http_header_tracing(self): with self.override_config("falcon", {}): config.falcon.http.trace_headers(["my-header", "my-response-header"]) diff --git a/tests/suitespec.yml b/tests/suitespec.yml index dd67e52deb9..4f9d4fcb435 100644 --- a/tests/suitespec.yml +++ b/tests/suitespec.yml @@ -114,7 +114,6 @@ components: telemetry: - ddtrace/internal/telemetry/* tracing: - - ddtrace/_hooks.py - ddtrace/_logger.py - ddtrace/_monkey.py - ddtrace/_trace/* diff --git a/tests/tracer/test_global_config.py b/tests/tracer/test_global_config.py index 0d60f2319e7..323770ee52e 100644 --- a/tests/tracer/test_global_config.py +++ b/tests/tracer/test_global_config.py @@ -1,6 +1,5 @@ from unittest import TestCase -import mock import pytest from ddtrace import config as global_config @@ -112,156 +111,6 @@ def test_settings_merge_deep(self): assert self.config.requests["a"]["b"]["c"] is True assert self.config.requests["a"]["b"]["d"] is True - def test_settings_hook(self): - """ - When calling `Hooks.emit()` - When there is a hook registered - we call the hook as expected - """ - - # Setup our hook - @self.config.web.hooks.on("request") - def on_web_request(span): - span.set_tag("web.request", "/") - - # Create our span - with self.tracer.start_span("web.request") as span: - assert "web.request" not in span.get_tags() - - # Emit the span - self.config.web.hooks.emit("request", span) - - # Assert we updated the span as expected - assert span.get_tag("web.request") == "/" - - def test_settings_hook_args(self): - """ - When calling `Hooks.emit()` with arguments - When there is a hook registered - we call the hook as expected - """ - - # Setup our hook - @self.config.web.hooks.on("request") - def on_web_request(span, request, response): - span.set_tag("web.request", request) - span.set_tag("web.response", response) - - # Create our span - with self.tracer.start_span("web.request") as span: - assert "web.request" not in span.get_tags() - - # Emit the span - # DEV: The actual values don't matter, we just want to test args + kwargs usage - self.config.web.hooks.emit("request", span, "request", response="response") - - # Assert we updated the span as expected - assert span.get_tag("web.request") == "request" - assert span.get_tag("web.response") == "response" - - def test_settings_hook_args_failure(self): - """ - When calling `Hooks.emit()` with arguments - When there is a hook registered that is missing parameters - we do not raise an exception - """ - - # Setup our hook - # DEV: We are missing the required "response" argument - @self.config.web.hooks.on("request") - def on_web_request(span, request): - span.set_tag("web.request", request) - - # Create our span - with self.tracer.start_span("web.request") as span: - assert "web.request" not in span.get_tags() - - # Emit the span - # DEV: This also asserts that no exception was raised - self.config.web.hooks.emit("request", span, "request", response="response") - - # Assert we did not update the span - assert "web.request" not in span.get_tags() - - def test_settings_multiple_hooks(self): - """ - When calling `Hooks.emit()` - When there are multiple hooks registered - we do not raise an exception - """ - - # Setup our hooks - @self.config.web.hooks.on("request") - def on_web_request(span): - span.set_tag("web.request", "/") - - @self.config.web.hooks.on("request") - def on_web_request2(span): - span.set_tag("web.status", 200) - - @self.config.web.hooks.on("request") - def on_web_request3(span): - span.set_tag("web.method", "GET") - - # Create our span - with self.tracer.start_span("web.request") as span: - assert "web.request" not in span.get_tags() - assert "web.status" not in span.get_metrics() - assert "web.method" not in span.get_tags() - - # Emit the span - self.config.web.hooks.emit("request", span) - - # Assert we updated the span as expected - assert span.get_tag("web.request") == "/" - assert span.get_metric("web.status") == 200 - assert span.get_tag("web.method") == "GET" - - def test_settings_hook_failure(self): - """ - When calling `Hooks.emit()` - When the hook raises an exception - we do not raise an exception - """ - # Setup our failing hook - on_web_request = mock.Mock(side_effect=Exception) - self.config.web.hooks.register("request")(on_web_request) - - # Create our span - with self.tracer.start_span("web.request") as span: - # Emit the span - # DEV: This is the test, to ensure no exceptions are raised - self.config.web.hooks.emit("request", span) - on_web_request.assert_called() - - def test_settings_no_hook(self): - """ - When calling `Hooks.emit()` - When no hook is registered - we do not raise an exception - """ - # Create our span - with self.tracer.start_span("web.request") as span: - # Emit the span - # DEV: This is the test, to ensure no exceptions are raised - self.config.web.hooks.emit("request", span) - - def test_settings_no_span(self): - """ - When calling `Hooks.emit()` - When no span is provided - we do not raise an exception - """ - - # Setup our hooks - @self.config.web.hooks.on("request") - def on_web_request(span): - span.set_tag("web.request", "/") - - # Emit the span - # DEV: This is the test, to ensure no exceptions are raised - self.config.web.hooks.emit("request", None) - def test_dd_version(self): c = Config() assert c.version is None diff --git a/tests/tracer/test_hooks.py b/tests/tracer/test_hooks.py index c09e496797d..882c922a227 100644 --- a/tests/tracer/test_hooks.py +++ b/tests/tracer/test_hooks.py @@ -1,29 +1,79 @@ -from ddtrace import _hooks +from unittest import TestCase +import warnings +from ddtrace.internal.settings._config import Config +from ddtrace.internal.settings.integration import IntegrationConfig -def test_deregister(): - hooks = _hooks.Hooks() - x = {} +class HooksDeprecationTestCase(TestCase): + def setUp(self): + self.config = Config() + self.config.web = IntegrationConfig(self.config, "web") - @hooks.register("key") - def do_not_call(): - x["x"] = True + def test_hooks_register_deprecation(self): + warnings.simplefilter("always") + with warnings.catch_warnings(record=True) as warns: + self.config.web.hooks.register("test_hook") + assert len(warns) == 1 + assert ( + "Hooks.register() is deprecated and is currently a no-op. and will be removed in version '5.0.0': To interact with spans, use get_current_span() or get_current_root_span()." # noqa:E501 + in str(warns[0].message) + ) - hooks.emit("key") - assert x["x"] - del x["x"] + def test_hooks_on_deprecation(self): + warnings.simplefilter("always") + with warnings.catch_warnings(record=True) as warns: + self.config.web.hooks.on("test_hook") + assert len(warns) == 1 + assert ( + "Hooks.register() is deprecated and is currently a no-op. and will be removed in version '5.0.0': To interact with spans, use get_current_span() or get_current_root_span()." # noqa:E501 + in str(warns[0].message) + ) - hooks.deregister("key", do_not_call) + def test_hooks_deregister_deprecation(self): + warnings.simplefilter("always") + with warnings.catch_warnings(record=True) as warns: + self.config.web.hooks.deregister("test_hook", lambda: None) + assert len(warns) == 1 + assert ( + "Hooks.deregister() is deprecated and is currently a no-op. and will be removed in version '5.0.0'" # noqa:E501 + in str(warns[0].message) + ) - hooks.emit("key") + def test_hooks_emit_deprecation(self): + warnings.simplefilter("always") + with warnings.catch_warnings(record=True) as warns: + self.config.web.hooks.emit("test_hook") + assert len(warns) == 1 + assert ( + "Hooks.emit() is deprecated and is currently a no-op. and will be removed in version '5.0.0': To interact with spans, use get_current_span() or get_current_root_span()." # noqa:E501 + in str(warns[0].message) + ) - assert len(x) == 0 + def test_hooks_register_decorator_deprecation(self): + warnings.simplefilter("always") + with warnings.catch_warnings(record=True) as warns: + @self.config.web.hooks.register("test_hook") + def my_hook(): + pass -def test_deregister_unknown(): - hooks = _hooks.Hooks() + assert len(warns) == 1 + assert ( + "Hooks.register() is deprecated and is currently a no-op. and will be removed in version '5.0.0': To interact with spans, use get_current_span() or get_current_root_span()." # noqa:E501 + in str(warns[0].message) + ) - hooks.deregister("key", test_deregister_unknown) + def test_hooks_on_decorator_deprecation(self): + warnings.simplefilter("always") + with warnings.catch_warnings(record=True) as warns: - hooks.emit("key") + @self.config.web.hooks.on("test_hook") + def my_hook(): + pass + + assert len(warns) == 1 + assert ( + "Hooks.register() is deprecated and is currently a no-op. and will be removed in version '5.0.0': To interact with spans, use get_current_span() or get_current_root_span()." # noqa:E501 + in str(warns[0].message) + )