From aaedab9bcd69363043aaf47602288f96faf5fcfe Mon Sep 17 00:00:00 2001 From: Yun Kim <35776586+Yun-Kim@users.noreply.github.com> Date: Mon, 27 Jan 2025 16:23:45 -0500 Subject: [PATCH 01/18] chore: update changelog for version 2.19.2 (#12088) ## Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) 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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) --------- Co-authored-by: Nicole Cybul --- CHANGELOG.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a29299f8987..6c72665472f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,21 @@ Changelogs for versions not listed here can be found at https://github.com/DataD --- +## 2.19.2 +### Bug Fixes + +- Tracing + - celery: Fixes an issue where `celery.apply` spans from Celery prerun got closed too soon leading to span tags being missing. + - openai: Fixes a patching issue where asynchronous moderation endpoint calls resulted in coroutine scheduling errors. + - openai: Ensures the OpenAI integration is compatible with Python versions 3.12 and 3.13. + - vertexai: Resolves an issue with `chat.send_message()` where the content keyword argument was not parsed correctly. +- LLM Observability + - This fix resolves an issue where annotating a span with non latin-1 (but valid utf-8) input/output values resulted in encoding errors. +- Lib-Injection + - Fixes incorrect telemetry data payload format. + +--- + ## 2.19.1 ### Bug Fixes From e3045a19a20e0348f040aa589ef3c2517db74618 Mon Sep 17 00:00:00 2001 From: Nick Ripley Date: Mon, 27 Jan 2025 16:30:29 -0500 Subject: [PATCH 02/18] fix(profiling): fix SystemError when collecting memory profiler events (#12075) We added locking to the memory profiler to address crashes. These locks are mostly "try" locks, meaning we bail out if we can't acquire them right away. This was done defensively to mitigate the possibility of deadlock until we fully understood why the locks are needed and could guarantee their correctness. But as a result of using try locks, the `iter_events` function in particular can fail if the memory profiler lock is contended when it tries to collect profiling events. The function then returns NULL, leading to SystemError exceptions because we don't set an error. Even if we set an error, returning NULL isn't the right thing to do. It'll basically mean we wait until the next profile iteration, still accumulating events in the same buffer, and try again to upload the events. So we're going to get multiple iteration's worth of events. The right thing to do is take the lock unconditionally in `iter_events`. We can allocate the new tracker outside the memory allocation profiler lock so that we don't need to worry about reentrancy/deadlock issues if we start profiling that allocation. Then, the only thing we do under the lock is swap out the global tracker, so it's safe to take the lock unconditionally. Fixes #11831 TODO - regression test? --- ddtrace/profiling/collector/_memalloc.c | 22 +++++++++++++------ ...loc-iter-events-null-780fd50bbebbf616.yaml | 4 ++++ 2 files changed, 19 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/profiling-memalloc-iter-events-null-780fd50bbebbf616.yaml diff --git a/ddtrace/profiling/collector/_memalloc.c b/ddtrace/profiling/collector/_memalloc.c index 1f2b87e0433..1e1b9dbf52e 100644 --- a/ddtrace/profiling/collector/_memalloc.c +++ b/ddtrace/profiling/collector/_memalloc.c @@ -394,20 +394,28 @@ iterevents_new(PyTypeObject* type, PyObject* Py_UNUSED(args), PyObject* Py_UNUSE } IterEventsState* iestate = (IterEventsState*)type->tp_alloc(type, 0); - if (!iestate) + if (!iestate) { + PyErr_SetString(PyExc_RuntimeError, "failed to allocate IterEventsState"); return NULL; + } memalloc_assert_gil(); - /* reset the current traceback list */ - if (memlock_trylock(&g_memalloc_lock)) { - iestate->alloc_tracker = global_alloc_tracker; - global_alloc_tracker = alloc_tracker_new(); - memlock_unlock(&g_memalloc_lock); - } else { + /* Reset the current traceback list. Do this outside lock so we can track it, + * and avoid reentrancy/deadlock problems, if we start tracking the raw + * allocator domain */ + alloc_tracker_t* tracker = alloc_tracker_new(); + if (!tracker) { + PyErr_SetString(PyExc_RuntimeError, "failed to allocate new allocation tracker"); Py_TYPE(iestate)->tp_free(iestate); return NULL; } + + memlock_lock(&g_memalloc_lock); + iestate->alloc_tracker = global_alloc_tracker; + global_alloc_tracker = tracker; + memlock_unlock(&g_memalloc_lock); + iestate->seq_index = 0; PyObject* iter_and_count = PyTuple_New(3); diff --git a/releasenotes/notes/profiling-memalloc-iter-events-null-780fd50bbebbf616.yaml b/releasenotes/notes/profiling-memalloc-iter-events-null-780fd50bbebbf616.yaml new file mode 100644 index 00000000000..52a43cbd2a1 --- /dev/null +++ b/releasenotes/notes/profiling-memalloc-iter-events-null-780fd50bbebbf616.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - | + profiling: fix SystemError from the memory profiler returning NULL when collecting events From 55767a78063ed17de0e954a6db4b9ff64b762f1d Mon Sep 17 00:00:00 2001 From: William Conti <58711692+wconti27@users.noreply.github.com> Date: Tue, 28 Jan 2025 08:44:03 -0500 Subject: [PATCH 03/18] chore(tracing): refactor web server integrations to use the core module (#12035) ## Motivation Refactors all web server integrations still using `tracer.trace` to instead use `core.context_with_data`. This is in preparation for supporting AWS API Gateway to ensure all web servers share the same code path. ## Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) 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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) --- ddtrace/_trace/trace_handlers.py | 56 +++++++++- .../contrib/internal/aiohttp/middlewares.py | 105 ++++++++---------- ddtrace/contrib/internal/bottle/trace.py | 61 +++++----- .../contrib/internal/cherrypy/middleware.py | 53 +++++---- ddtrace/contrib/internal/falcon/middleware.py | 61 ++++------ ddtrace/contrib/internal/molten/patch.py | 52 ++++----- ddtrace/contrib/internal/pyramid/trace.py | 72 ++++++------ ddtrace/contrib/internal/sanic/patch.py | 61 +++++----- ...b.cherrypy.test_middleware.test_child.json | 1 + ...b.cherrypy.test_middleware.test_error.json | 1 + ...b.cherrypy.test_middleware.test_fatal.json | 1 + ...cherrypy.test_middleware.test_success.json | 1 + ...ver.test_multiple_requests_sanic_http.json | 2 + ...multiple_requests_sanic_http_pre_21.9.json | 2 + ...c.test_sanic_server.test_sanic_errors.json | 2 + ...nic_server.test_sanic_errors_pre_21.9.json | 2 + 16 files changed, 290 insertions(+), 243 deletions(-) diff --git a/ddtrace/_trace/trace_handlers.py b/ddtrace/_trace/trace_handlers.py index dab3f743146..3ebb28f33c8 100644 --- a/ddtrace/_trace/trace_handlers.py +++ b/ddtrace/_trace/trace_handlers.py @@ -109,11 +109,14 @@ def _get_parameters_for_new_span_directly_from_context(ctx: core.ExecutionContex def _start_span(ctx: core.ExecutionContext, call_trace: bool = True, **kwargs) -> "Span": span_kwargs = _get_parameters_for_new_span_directly_from_context(ctx) call_trace = ctx.get_item("call_trace", call_trace) - tracer = (ctx.get_item("middleware") or ctx["pin"]).tracer + tracer = ctx.get_item("tracer") or (ctx.get_item("middleware") or ctx["pin"]).tracer distributed_headers_config = ctx.get_item("distributed_headers_config") if distributed_headers_config: trace_utils.activate_distributed_headers( - tracer, int_config=distributed_headers_config, request_headers=ctx["distributed_headers"] + tracer, + int_config=distributed_headers_config, + request_headers=ctx["distributed_headers"], + override=ctx.get_item("distributed_headers_config_override"), ) distributed_context = ctx.get_item("distributed_context") if distributed_context and not call_trace: @@ -126,6 +129,42 @@ def _start_span(ctx: core.ExecutionContext, call_trace: bool = True, **kwargs) - return span +def _set_web_frameworks_tags(ctx, span, int_config): + span.set_tag_str(COMPONENT, int_config.integration_name) + span.set_tag_str(SPAN_KIND, SpanKind.SERVER) + span.set_tag(_SPAN_MEASURED_KEY) + + analytics_enabled = ctx.get_item("analytics_enabled") + analytics_sample_rate = ctx.get_item("analytics_sample_rate", True) + + # Configure trace search sample rate + if (config._analytics_enabled and analytics_enabled is not False) or analytics_enabled is True: + span.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, analytics_sample_rate) + + +def _on_web_framework_start_request(ctx, int_config): + request_span = ctx.get_item("req_span") + _set_web_frameworks_tags(ctx, request_span, int_config) + + +def _on_web_framework_finish_request( + span, int_config, method, url, status_code, query, req_headers, res_headers, route, finish +): + trace_utils.set_http_meta( + span=span, + integration_config=int_config, + method=method, + url=url, + status_code=status_code, + query=query, + request_headers=req_headers, + response_headers=res_headers, + route=route, + ) + if finish: + span.finish() + + def _on_traced_request_context_started_flask(ctx): current_span = ctx["pin"].tracer.current_span() if not ctx["pin"].enabled or not current_span: @@ -761,6 +800,10 @@ def listen(): core.on("azure.functions.request_call_modifier", _on_azure_functions_request_span_modifier) core.on("azure.functions.start_response", _on_azure_functions_start_response) + # web frameworks general handlers + core.on("web.request.start", _on_web_framework_start_request) + core.on("web.request.finish", _on_web_framework_finish_request) + core.on("test_visibility.enable", _on_test_visibility_enable) core.on("test_visibility.disable", _on_test_visibility_disable) core.on("test_visibility.is_enabled", _on_test_visibility_is_enabled, "is_enabled") @@ -769,6 +812,14 @@ def listen(): core.on("rq.queue.enqueue_job", _propagate_context) for context_name in ( + # web frameworks + "aiohttp.request", + "bottle.request", + "cherrypy.request", + "falcon.request", + "molten.request", + "pyramid.request", + "sanic.request", "flask.call", "flask.jsonify", "flask.render_template", @@ -779,6 +830,7 @@ def listen(): "django.template.render", "django.process_exception", "django.func.wrapped", + # non web frameworks "botocore.instrumented_api_call", "botocore.instrumented_lib_function", "botocore.patched_kinesis_api_call", diff --git a/ddtrace/contrib/internal/aiohttp/middlewares.py b/ddtrace/contrib/internal/aiohttp/middlewares.py index ddb2d35fbc6..63a60734d3b 100644 --- a/ddtrace/contrib/internal/aiohttp/middlewares.py +++ b/ddtrace/contrib/internal/aiohttp/middlewares.py @@ -2,15 +2,10 @@ from aiohttp.web_urldispatcher import SystemRoute from ddtrace import config -from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY -from ddtrace.constants import _SPAN_MEASURED_KEY -from ddtrace.constants import SPAN_KIND -from ddtrace.contrib import trace_utils from ddtrace.contrib.asyncio import context_provider -from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes from ddtrace.ext import http -from ddtrace.internal.constants import COMPONENT +from ddtrace.internal import core from ddtrace.internal.schema import schematize_url_operation from ddtrace.internal.schema.span_attribute_schema import SpanDirection @@ -35,47 +30,42 @@ async def attach_context(request): # application configs tracer = app[CONFIG_KEY]["tracer"] service = app[CONFIG_KEY]["service"] - distributed_tracing = app[CONFIG_KEY]["distributed_tracing_enabled"] - # Create a new context based on the propagated information. - trace_utils.activate_distributed_headers( - tracer, - int_config=config.aiohttp, - request_headers=request.headers, - override=distributed_tracing, - ) - - # trace the handler - request_span = tracer.trace( - schematize_url_operation("aiohttp.request", protocol="http", direction=SpanDirection.INBOUND), - service=service, - span_type=SpanTypes.WEB, - ) - request_span.set_tag(_SPAN_MEASURED_KEY) - - request_span.set_tag_str(COMPONENT, config.aiohttp.integration_name) - - # set span.kind tag equal to type of request - request_span.set_tag_str(SPAN_KIND, SpanKind.SERVER) - - # Configure trace search sample rate # DEV: aiohttp is special case maintains separate configuration from config api analytics_enabled = app[CONFIG_KEY]["analytics_enabled"] - if (config._analytics_enabled and analytics_enabled is not False) or analytics_enabled is True: - request_span.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, app[CONFIG_KEY].get("analytics_sample_rate", True)) - - # attach the context and the root span to the request; the Context - # may be freely used by the application code - request[REQUEST_CONTEXT_KEY] = request_span.context - request[REQUEST_SPAN_KEY] = request_span - request[REQUEST_CONFIG_KEY] = app[CONFIG_KEY] - try: - response = await handler(request) - if isinstance(response, web.StreamResponse): - request.task.add_done_callback(lambda _: finish_request_span(request, response)) - return response - except Exception: - request_span.set_traceback() - raise + # Create a new context based on the propagated information. + + with core.context_with_data( + "aiohttp.request", + span_name=schematize_url_operation("aiohttp.request", protocol="http", direction=SpanDirection.INBOUND), + span_type=SpanTypes.WEB, + service=service, + tags={}, + tracer=tracer, + distributed_headers=request.headers, + distributed_headers_config=config.aiohttp, + distributed_headers_config_override=app[CONFIG_KEY]["distributed_tracing_enabled"], + headers_case_sensitive=True, + analytics_enabled=analytics_enabled, + analytics_sample_rate=app[CONFIG_KEY].get("analytics_sample_rate", True), + ) as ctx: + req_span = ctx.span + + ctx.set_item("req_span", req_span) + core.dispatch("web.request.start", (ctx, config.aiohttp)) + + # attach the context and the root span to the request; the Context + # may be freely used by the application code + request[REQUEST_CONTEXT_KEY] = req_span.context + request[REQUEST_SPAN_KEY] = req_span + request[REQUEST_CONFIG_KEY] = app[CONFIG_KEY] + try: + response = await handler(request) + if isinstance(response, web.StreamResponse): + request.task.add_done_callback(lambda _: finish_request_span(request, response)) + return response + except Exception: + req_span.set_traceback() + raise return attach_context @@ -122,19 +112,22 @@ def finish_request_span(request, response): # SystemRoute objects exist to throw HTTP errors and have no path route = aiohttp_route.resource.canonical - trace_utils.set_http_meta( - request_span, - config.aiohttp, - method=request.method, - url=str(request.url), # DEV: request.url is a yarl's URL object - status_code=response.status, - request_headers=request.headers, - response_headers=response.headers, - route=route, + core.dispatch( + "web.request.finish", + ( + request_span, + config.aiohttp, + request.method, + str(request.url), # DEV: request.url is a yarl's URL object + response.status, + None, # query arg = None + request.headers, + response.headers, + route, + True, + ), ) - request_span.finish() - async def on_prepare(request, response): """ diff --git a/ddtrace/contrib/internal/bottle/trace.py b/ddtrace/contrib/internal/bottle/trace.py index 3aabb4ccc81..58778a36ee1 100644 --- a/ddtrace/contrib/internal/bottle/trace.py +++ b/ddtrace/contrib/internal/bottle/trace.py @@ -5,13 +5,8 @@ import ddtrace from ddtrace import config -from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY -from ddtrace.constants import _SPAN_MEASURED_KEY -from ddtrace.constants import SPAN_KIND -from ddtrace.contrib import trace_utils -from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes -from ddtrace.internal.constants import COMPONENT +from ddtrace.internal import core from ddtrace.internal.schema import schematize_url_operation from ddtrace.internal.schema.span_attribute_schema import SpanDirection from ddtrace.internal.utils.formats import asbool @@ -42,24 +37,21 @@ def wrapped(*args, **kwargs): resource = "{} {}".format(request.method, route.rule) - trace_utils.activate_distributed_headers( - self.tracer, int_config=config.bottle, request_headers=request.headers - ) - - with self.tracer.trace( - schematize_url_operation("bottle.request", protocol="http", direction=SpanDirection.INBOUND), + with core.context_with_data( + "bottle.request", + span_name=schematize_url_operation("bottle.request", protocol="http", direction=SpanDirection.INBOUND), + span_type=SpanTypes.WEB, service=self.service, resource=resource, - span_type=SpanTypes.WEB, - ) as s: - s.set_tag_str(COMPONENT, config.bottle.integration_name) - - # set span.kind to the type of request being performed - s.set_tag_str(SPAN_KIND, SpanKind.SERVER) - - s.set_tag(_SPAN_MEASURED_KEY) - # set analytics sample rate with global config enabled - s.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, config.bottle.get_analytics_sample_rate(use_global_config=True)) + tags={}, + tracer=self.tracer, + distributed_headers=request.headers, + distributed_headers_config=config.bottle, + headers_case_sensitive=True, + analytics_sample_rate=config.bottle.get_analytics_sample_rate(use_global_config=True), + ) as ctx, ctx.span as req_span: + ctx.set_item("req_span", req_span) + core.dispatch("web.request.start", (ctx, config.bottle)) code = None result = None @@ -91,16 +83,21 @@ def wrapped(*args, **kwargs): method = request.method url = request.urlparts._replace(query="").geturl() full_route = "/".join([request.script_name.rstrip("/"), route.rule.lstrip("/")]) - trace_utils.set_http_meta( - s, - config.bottle, - method=method, - url=url, - status_code=response_code, - query=request.query_string, - request_headers=request.headers, - response_headers=response.headers, - route=full_route, + + core.dispatch( + "web.request.finish", + ( + req_span, + config.bottle, + method, + url, + response_code, + request.query_string, + request.headers, + response.headers, + full_route, + False, + ), ) return wrapped diff --git a/ddtrace/contrib/internal/cherrypy/middleware.py b/ddtrace/contrib/internal/cherrypy/middleware.py index 909a043175f..70f43059905 100644 --- a/ddtrace/contrib/internal/cherrypy/middleware.py +++ b/ddtrace/contrib/internal/cherrypy/middleware.py @@ -11,12 +11,10 @@ from ddtrace.constants import ERROR_MSG from ddtrace.constants import ERROR_STACK from ddtrace.constants import ERROR_TYPE -from ddtrace.constants import SPAN_KIND from ddtrace.contrib import trace_utils -from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes from ddtrace.internal import compat -from ddtrace.internal.constants import COMPONENT +from ddtrace.internal import core from ddtrace.internal.schema import SpanDirection from ddtrace.internal.schema import schematize_service_name from ddtrace.internal.schema import schematize_url_operation @@ -77,20 +75,23 @@ def _setup(self): cherrypy.request.hooks.attach("after_error_response", self._after_error_response, priority=5) def _on_start_resource(self): - trace_utils.activate_distributed_headers( - self._tracer, int_config=config.cherrypy, request_headers=cherrypy.request.headers - ) - - cherrypy.request._datadog_span = self._tracer.trace( - SPAN_NAME, - service=trace_utils.int_service(None, config.cherrypy, default="cherrypy"), + with core.context_with_data( + "cherrypy.request", + span_name=SPAN_NAME, span_type=SpanTypes.WEB, - ) + service=trace_utils.int_service(None, config.cherrypy, default="cherrypy"), + tags={}, + tracer=self._tracer, + distributed_headers=cherrypy.request.headers, + distributed_headers_config=config.cherrypy, + headers_case_sensitive=True, + ) as ctx: + req_span = ctx.span - cherrypy.request._datadog_span.set_tag_str(COMPONENT, config.cherrypy.integration_name) + ctx.set_item("req_span", req_span) + core.dispatch("web.request.start", (ctx, config.cherrypy)) - # set span.kind to the type of request being performed - cherrypy.request._datadog_span.set_tag_str(SPAN_KIND, SpanKind.SERVER) + cherrypy.request._datadog_span = req_span def _after_error_response(self): span = getattr(cherrypy.request, "_datadog_span", None) @@ -135,18 +136,22 @@ def _close_span(self, span): url = compat.to_unicode(cherrypy.request.base + cherrypy.request.path_info) status_code, _, _ = valid_status(cherrypy.response.status) - trace_utils.set_http_meta( - span, - config.cherrypy, - method=cherrypy.request.method, - url=url, - status_code=status_code, - request_headers=cherrypy.request.headers, - response_headers=cherrypy.response.headers, + core.dispatch( + "web.request.finish", + ( + span, + config.cherrypy, + cherrypy.request.method, + url, + status_code, + None, + cherrypy.request.headers, + cherrypy.response.headers, + None, + True, + ), ) - span.finish() - # Clear our span just in case. cherrypy.request._datadog_span = None diff --git a/ddtrace/contrib/internal/falcon/middleware.py b/ddtrace/contrib/internal/falcon/middleware.py index b4ec5434777..513ce6cce39 100644 --- a/ddtrace/contrib/internal/falcon/middleware.py +++ b/ddtrace/contrib/internal/falcon/middleware.py @@ -1,14 +1,8 @@ import sys from ddtrace import config -from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY -from ddtrace.constants import _SPAN_MEASURED_KEY -from ddtrace.constants import SPAN_KIND -from ddtrace.contrib import trace_utils -from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes -from ddtrace.ext import http as httpx -from ddtrace.internal.constants import COMPONENT +from ddtrace.internal import core from ddtrace.internal.schema import SpanDirection from ddtrace.internal.schema import schematize_service_name from ddtrace.internal.schema import schematize_url_operation @@ -27,26 +21,27 @@ def __init__(self, tracer, service=None, distributed_tracing=None): def process_request(self, req, resp): # Falcon uppercases all header names. headers = dict((k.lower(), v) for k, v in req.headers.items()) - trace_utils.activate_distributed_headers(self.tracer, int_config=config.falcon, request_headers=headers) - span = self.tracer.trace( - schematize_url_operation("falcon.request", protocol="http", direction=SpanDirection.INBOUND), - service=self.service, + with core.context_with_data( + "falcon.request", + span_name=schematize_url_operation("falcon.request", protocol="http", direction=SpanDirection.INBOUND), span_type=SpanTypes.WEB, - ) - span.set_tag_str(COMPONENT, config.falcon.integration_name) - - # set span.kind to the type of operation being performed - span.set_tag_str(SPAN_KIND, SpanKind.SERVER) - - span.set_tag(_SPAN_MEASURED_KEY) - - # set analytics sample rate with global config enabled - span.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, config.falcon.get_analytics_sample_rate(use_global_config=True)) - - trace_utils.set_http_meta( - span, config.falcon, method=req.method, url=req.url, query=req.query_string, request_headers=req.headers - ) + service=self.service, + tags={}, + tracer=self.tracer, + distributed_headers=headers, + distributed_headers_config=config.falcon, + headers_case_sensitive=True, + analytics_sample_rate=config.falcon.get_analytics_sample_rate(use_global_config=True), + ) as ctx: + req_span = ctx.span + ctx.set_item("req_span", req_span) + core.dispatch("web.request.start", (ctx, config.falcon)) + + core.dispatch( + "web.request.finish", + (req_span, config.falcon, req.method, req.url, None, req.query_string, req.headers, None, None, False), + ) def process_resource(self, req, resp, resource, params): span = self.tracer.current_span() @@ -69,8 +64,7 @@ def process_response(self, req, resp, resource, req_succeeded=None): if resource is None: status = "404" span.resource = "%s 404" % req.method - span.set_tag(httpx.STATUS_CODE, status) - span.finish() + core.dispatch("web.request.finish", (span, config.falcon, None, None, status, None, None, None, None, True)) return err_type = sys.exc_info()[0] @@ -87,20 +81,13 @@ def process_response(self, req, resp, resource, req_succeeded=None): route = req.root_path or "" + req.uri_template - trace_utils.set_http_meta( - span, - config.falcon, - status_code=status, - response_headers=resp._headers, - route=route, - ) - # 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) - # Close the span - span.finish() + core.dispatch( + "web.request.finish", (span, config.falcon, None, None, status, None, None, resp._headers, route, True) + ) def _is_404(err_type): diff --git a/ddtrace/contrib/internal/molten/patch.py b/ddtrace/contrib/internal/molten/patch.py index 38fa949243c..dfec47eb17d 100644 --- a/ddtrace/contrib/internal/molten/patch.py +++ b/ddtrace/contrib/internal/molten/patch.py @@ -5,15 +5,11 @@ from wrapt import wrap_function_wrapper as _w from ddtrace import config -from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY -from ddtrace.constants import _SPAN_MEASURED_KEY -from ddtrace.constants import SPAN_KIND from ddtrace.contrib import trace_utils from ddtrace.contrib.internal.trace_utils import unwrap as _u -from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes +from ddtrace.internal import core from ddtrace.internal.compat import urlencode -from ddtrace.internal.constants import COMPONENT from ddtrace.internal.schema import schematize_service_name from ddtrace.internal.schema import schematize_url_operation from ddtrace.internal.schema.span_attribute_schema import SpanDirection @@ -89,25 +85,21 @@ def patch_app_call(wrapped, instance, args, kwargs): request = molten.http.Request.from_environ(environ) resource = func_name(wrapped) - # request.headers is type Iterable[Tuple[str, str]] - trace_utils.activate_distributed_headers( - pin.tracer, int_config=config.molten, request_headers=dict(request.headers) - ) - - with pin.tracer.trace( - schematize_url_operation("molten.request", protocol="http", direction=SpanDirection.INBOUND), + with core.context_with_data( + "molten.request", + span_name=schematize_url_operation("molten.request", protocol="http", direction=SpanDirection.INBOUND), + span_type=SpanTypes.WEB, service=trace_utils.int_service(pin, config.molten), resource=resource, - span_type=SpanTypes.WEB, - ) as span: - span.set_tag_str(COMPONENT, config.molten.integration_name) - - # set span.kind tag equal to type of operation being performed - span.set_tag_str(SPAN_KIND, SpanKind.SERVER) - - span.set_tag(_SPAN_MEASURED_KEY) - # set analytics sample rate with global config enabled - span.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, config.molten.get_analytics_sample_rate(use_global_config=True)) + tags={}, + tracer=pin.tracer, + distributed_headers=dict(request.headers), # request.headers is type Iterable[Tuple[str, str]] + distributed_headers_config=config.molten, + headers_case_sensitive=True, + analytics_sample_rate=config.molten.get_analytics_sample_rate(use_global_config=True), + ) as ctx, ctx.span as req_span: + ctx.set_item("req_span", req_span) + core.dispatch("web.request.start", (ctx, config.molten)) @wrapt.function_wrapper def _w_start_response(wrapped, instance, args, kwargs): @@ -125,11 +117,13 @@ def _w_start_response(wrapped, instance, args, kwargs): except ValueError: pass - if not span.get_tag(MOLTEN_ROUTE): + if not req_span.get_tag(MOLTEN_ROUTE): # if route never resolve, update root resource - span.resource = "{} {}".format(request.method, code) + req_span.resource = "{} {}".format(request.method, code) - trace_utils.set_http_meta(span, config.molten, status_code=code) + core.dispatch( + "web.request.finish", (req_span, config.molten, None, None, code, None, None, None, None, False) + ) return wrapped(*args, **kwargs) @@ -143,11 +137,13 @@ def _w_start_response(wrapped, instance, args, kwargs): request.path, ) query = urlencode(dict(request.params)) - trace_utils.set_http_meta( - span, config.molten, method=request.method, url=url, query=query, request_headers=request.headers + + core.dispatch( + "web.request.finish", + (req_span, config.molten, request.method, url, None, query, request.headers, None, None, False), ) - span.set_tag_str("molten.version", molten.__version__) + req_span.set_tag_str("molten.version", molten.__version__) return wrapped(environ, start_response, **kwargs) diff --git a/ddtrace/contrib/internal/pyramid/trace.py b/ddtrace/contrib/internal/pyramid/trace.py index 9942c673d4e..9b221bd3f09 100644 --- a/ddtrace/contrib/internal/pyramid/trace.py +++ b/ddtrace/contrib/internal/pyramid/trace.py @@ -6,12 +6,8 @@ # project import ddtrace from ddtrace import config -from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY -from ddtrace.constants import _SPAN_MEASURED_KEY -from ddtrace.constants import SPAN_KIND -from ddtrace.contrib import trace_utils -from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes +from ddtrace.internal import core from ddtrace.internal.constants import COMPONENT from ddtrace.internal.logger import get_logger from ddtrace.internal.schema import schematize_service_name @@ -67,29 +63,30 @@ def trace_tween_factory(handler, registry): service = settings.get(SETTINGS_SERVICE) or schematize_service_name("pyramid") tracer = settings.get(SETTINGS_TRACER) or ddtrace.tracer enabled = asbool(settings.get(SETTINGS_TRACE_ENABLED, tracer.enabled)) - distributed_tracing = asbool(settings.get(SETTINGS_DISTRIBUTED_TRACING, True)) + + # ensure distributed tracing within pyramid settings matches config + config.pyramid.distributed_tracing_enabled = asbool(settings.get(SETTINGS_DISTRIBUTED_TRACING, True)) if enabled: # make a request tracing function def trace_tween(request): - trace_utils.activate_distributed_headers( - tracer, int_config=config.pyramid, request_headers=request.headers, override=distributed_tracing - ) - - span_name = schematize_url_operation("pyramid.request", protocol="http", direction=SpanDirection.INBOUND) - with tracer.trace(span_name, service=service, resource="404", span_type=SpanTypes.WEB) as span: - span.set_tag_str(COMPONENT, config.pyramid.integration_name) - - # set span.kind to the type of operation being performed - span.set_tag_str(SPAN_KIND, SpanKind.SERVER) - - span.set_tag(_SPAN_MEASURED_KEY) - # Configure trace search sample rate + with core.context_with_data( + "pyramid.request", + span_name=schematize_url_operation("pyramid.request", protocol="http", direction=SpanDirection.INBOUND), + span_type=SpanTypes.WEB, + service=service, + resource="404", + tags={}, + tracer=tracer, + distributed_headers=request.headers, + distributed_headers_config=config.pyramid, + headers_case_sensitive=True, # DEV: pyramid is special case maintains separate configuration from config api - analytics_enabled = settings.get(SETTINGS_ANALYTICS_ENABLED) - - if (config._analytics_enabled and analytics_enabled is not False) or analytics_enabled is True: - span.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, settings.get(SETTINGS_ANALYTICS_SAMPLE_RATE, True)) + analytics_enabled=settings.get(SETTINGS_ANALYTICS_ENABLED), + analytics_sample_rate=settings.get(SETTINGS_ANALYTICS_SAMPLE_RATE, True), + ) as ctx, ctx.span as req_span: + ctx.set_item("req_span", req_span) + core.dispatch("web.request.start", (ctx, config.pyramid)) setattr(request, DD_TRACER, tracer) # used to find the tracer in templates response = None @@ -110,8 +107,8 @@ def trace_tween(request): finally: # set request tags if request.matched_route: - span.resource = "{} {}".format(request.method, request.matched_route.name) - span.set_tag_str("pyramid.route.name", request.matched_route.name) + req_span.resource = "{} {}".format(request.method, request.matched_route.name) + req_span.set_tag_str("pyramid.route.name", request.matched_route.name) # set response tags if response: status = response.status_code @@ -119,17 +116,22 @@ def trace_tween(request): else: response_headers = None - trace_utils.set_http_meta( - span, - config.pyramid, - method=request.method, - url=request.path_url, - status_code=status, - query=request.query_string, - request_headers=request.headers, - response_headers=response_headers, - route=request.matched_route.pattern if request.matched_route else None, + core.dispatch( + "web.request.finish", + ( + req_span, + config.pyramid, + request.method, + request.path_url, + status, + request.query_string, + request.headers, + response_headers, + request.matched_route.pattern if request.matched_route else None, + False, + ), ) + return response return trace_tween diff --git a/ddtrace/contrib/internal/sanic/patch.py b/ddtrace/contrib/internal/sanic/patch.py index 8e53ed41dc8..1babf7d44a0 100644 --- a/ddtrace/contrib/internal/sanic/patch.py +++ b/ddtrace/contrib/internal/sanic/patch.py @@ -4,14 +4,10 @@ import wrapt from wrapt import wrap_function_wrapper as _w -import ddtrace from ddtrace import config -from ddtrace.constants import _ANALYTICS_SAMPLE_RATE_KEY -from ddtrace.constants import SPAN_KIND from ddtrace.contrib import trace_utils -from ddtrace.ext import SpanKind from ddtrace.ext import SpanTypes -from ddtrace.internal.constants import COMPONENT +from ddtrace.internal import core from ddtrace.internal.logger import get_logger from ddtrace.internal.schema import schematize_service_name from ddtrace.internal.schema import schematize_url_operation @@ -47,7 +43,10 @@ def update_span(span, response): # and so use 500 status_code = getattr(response, "status", 500) response_headers = getattr(response, "headers", None) - trace_utils.set_http_meta(span, config.sanic, status_code=status_code, response_headers=response_headers) + + core.dispatch( + "web.request.finish", (span, config.sanic, None, None, status_code, None, None, response_headers, None, False) + ) def _wrap_response_callback(span, callback): @@ -200,31 +199,35 @@ def _create_sanic_request_span(request): headers = request.headers.copy() - trace_utils.activate_distributed_headers(ddtrace.tracer, int_config=config.sanic, request_headers=headers) - - span = pin.tracer.trace( - schematize_url_operation("sanic.request", protocol="http", direction=SpanDirection.INBOUND), + with core.context_with_data( + "sanic.request", + span_name=schematize_url_operation("sanic.request", protocol="http", direction=SpanDirection.INBOUND), + span_type=SpanTypes.WEB, service=trace_utils.int_service(None, config.sanic), resource=resource, - span_type=SpanTypes.WEB, - ) - span.set_tag_str(COMPONENT, config.sanic.integration_name) - - # set span.kind to the type of operation being performed - span.set_tag_str(SPAN_KIND, SpanKind.SERVER) - - sample_rate = config.sanic.get_analytics_sample_rate(use_global_config=True) - if sample_rate is not None: - span.set_tag(_ANALYTICS_SAMPLE_RATE_KEY, sample_rate) - - method = request.method - url = "{scheme}://{host}{path}".format(scheme=request.scheme, host=request.host, path=request.path) - query_string = request.query_string - if isinstance(query_string, bytes): - query_string = query_string.decode() - trace_utils.set_http_meta(span, config.sanic, method=method, url=url, query=query_string, request_headers=headers) - - return span + tags={}, + pin=pin, + distributed_headers=headers, + distributed_headers_config=config.sanic, + headers_case_sensitive=True, + analytics_sample_rate=config.sanic.get_analytics_sample_rate(use_global_config=True), + ) as ctx: + req_span = ctx.span + + ctx.set_item("req_span", req_span) + core.dispatch("web.request.start", (ctx, config.sanic)) + + method = request.method + url = "{scheme}://{host}{path}".format(scheme=request.scheme, host=request.host, path=request.path) + query_string = request.query_string + if isinstance(query_string, bytes): + query_string = query_string.decode() + + core.dispatch( + "web.request.finish", (req_span, config.sanic, method, url, None, query_string, headers, None, None, False) + ) + + return req_span async def sanic_http_lifecycle_handle(request): diff --git a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_child.json b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_child.json index 1887282b06f..933eda3e420 100644 --- a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_child.json +++ b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_child.json @@ -22,6 +22,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_error.json b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_error.json index 49759fee3d3..58fde524828 100644 --- a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_error.json +++ b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_error.json @@ -25,6 +25,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_fatal.json b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_fatal.json index 4153c8c2b18..a5a7c1ed14f 100644 --- a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_fatal.json +++ b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_fatal.json @@ -25,6 +25,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_success.json b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_success.json index d1eb9fc5bee..ce13988add0 100644 --- a/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_success.json +++ b/tests/snapshots/tests.contrib.cherrypy.test_middleware.test_success.json @@ -22,6 +22,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http.json b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http.json index 3803d07f360..66ca28d9606 100644 --- a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http.json +++ b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http.json @@ -23,6 +23,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, @@ -72,6 +73,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http_pre_21.9.json b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http_pre_21.9.json index 4aac2721c02..aca376a974a 100644 --- a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http_pre_21.9.json +++ b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_multiple_requests_sanic_http_pre_21.9.json @@ -22,6 +22,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, @@ -70,6 +71,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors.json b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors.json index 435bbbc7b23..ab2aeaec920 100644 --- a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors.json +++ b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors.json @@ -22,6 +22,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, @@ -58,6 +59,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, diff --git a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors_pre_21.9.json b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors_pre_21.9.json index c03813a43d6..ceabecd1ae7 100644 --- a/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors_pre_21.9.json +++ b/tests/snapshots/tests.contrib.sanic.test_sanic_server.test_sanic_errors_pre_21.9.json @@ -22,6 +22,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, @@ -54,6 +55,7 @@ "span.kind": "server" }, "metrics": { + "_dd.measured": 1, "_dd.top_level": 1, "_dd.tracer_kr": 1.0, "_sampling_priority_v1": 1, From 16d5280956bc259e77691536229824d3183a4181 Mon Sep 17 00:00:00 2001 From: Christophe Papazian <114495376+christophe-papazian@users.noreply.github.com> Date: Tue, 28 Jan 2025 15:48:22 +0100 Subject: [PATCH 04/18] ci(tracer): make serverless test unrequired (#12121) ## Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) 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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) --- .gitlab/benchmarks.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitlab/benchmarks.yml b/.gitlab/benchmarks.yml index e922d315444..692a61ea93f 100644 --- a/.gitlab/benchmarks.yml +++ b/.gitlab/benchmarks.yml @@ -96,6 +96,7 @@ benchmark-serverless: tags: ["arch:amd64"] when: on_success needs: [ "benchmark-serverless-trigger" ] + allow_failure: true script: - git clone https://gitlab-ci-token:${CI_JOB_TOKEN}@gitlab.ddbuild.io/DataDog/serverless-tools.git ./serverless-tools && cd ./serverless-tools - ./ci/check_trigger_status.sh From 4f0bcb5a59378fb6015230f9f9ad04199fc380b5 Mon Sep 17 00:00:00 2001 From: Christophe Papazian <114495376+christophe-papazian@users.noreply.github.com> Date: Tue, 28 Jan 2025 17:28:20 +0100 Subject: [PATCH 05/18] chore(asm): clean libddwaf loading (#12102) Depending of the timing, libddwaf loading process could create triggers that would create loops in our instrumentation. From what I investigated: - if loaded too early, it could have bad interactions with gevent. - if loaded too late, it could be self instrumented by the tracer, creating a loop, as ctypes is using Popen and subprocess. while keeping the late loading introduced by 2 previous PRs - https://github.com/DataDog/dd-trace-py/pull/11987 - https://github.com/DataDog/dd-trace-py/pull/12013 this PR introduced a mechanism to bypass tracer instrumentation during ctypes loading, to avoid a possible loop that would prevent the WAF to be loaded. ## Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) 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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) --- ddtrace/appsec/_ddwaf/ddwaf_types.py | 3 +++ ddtrace/appsec/_processor.py | 26 +++++++++++--------- ddtrace/contrib/internal/subprocess/patch.py | 6 +++++ ddtrace/settings/asm.py | 1 + 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/ddtrace/appsec/_ddwaf/ddwaf_types.py b/ddtrace/appsec/_ddwaf/ddwaf_types.py index e998b6048f6..5942a2fa184 100644 --- a/ddtrace/appsec/_ddwaf/ddwaf_types.py +++ b/ddtrace/appsec/_ddwaf/ddwaf_types.py @@ -22,9 +22,12 @@ if system() == "Linux": try: + asm_config._bypass_instrumentation_for_waf = True ctypes.CDLL(ctypes.util.find_library("rt"), mode=ctypes.RTLD_GLOBAL) except Exception: # nosec pass + finally: + asm_config._bypass_instrumentation_for_waf = False ARCHI = machine().lower() diff --git a/ddtrace/appsec/_processor.py b/ddtrace/appsec/_processor.py index 6102ba1ded2..83b65584a66 100644 --- a/ddtrace/appsec/_processor.py +++ b/ddtrace/appsec/_processor.py @@ -4,6 +4,7 @@ from json.decoder import JSONDecodeError import os import os.path +from typing import TYPE_CHECKING from typing import Any from typing import Dict from typing import List @@ -11,6 +12,11 @@ from typing import Set from typing import Tuple from typing import Union + + +if TYPE_CHECKING: + import ddtrace.appsec._ddwaf as ddwaf + import weakref from ddtrace._trace.processor import SpanProcessor @@ -167,14 +173,17 @@ def __post_init__(self) -> None: def delayed_init(self) -> None: try: if self._rules is not None and not hasattr(self, "_ddwaf"): - self._ddwaf = ddwaf.DDWaf( + from ddtrace.appsec._ddwaf import DDWaf # noqa: E402 + import ddtrace.appsec._metrics as metrics # noqa: E402 + + self.metrics = metrics + self._ddwaf = DDWaf( self._rules, self.obfuscation_parameter_key_regexp, self.obfuscation_parameter_value_regexp ) - _set_waf_init_metric(self._ddwaf.info) + self.metrics._set_waf_init_metric(self._ddwaf.info) except Exception: # Partial of DDAS-0005-00 log.warning("[DDAS-0005-00] WAF initialization failed") - raise self._update_required() def _update_required(self): @@ -193,7 +202,7 @@ def _update_rules(self, new_rules: Dict[str, Any]) -> bool: if asm_config._asm_static_rule_file is not None: return result result = self._ddwaf.update_rules(new_rules) - _set_waf_updates_metric(self._ddwaf.info) + self.metrics._set_waf_updates_metric(self._ddwaf.info) self._update_required() return result @@ -241,7 +250,7 @@ def waf_callable(custom_data=None, **kwargs): return self._waf_action(span._local_root or span, ctx, custom_data, **kwargs) _asm_request_context.set_waf_callback(waf_callable) - _asm_request_context.add_context_callback(_set_waf_request_metrics) + _asm_request_context.add_context_callback(self.metrics._set_waf_request_metrics) if headers is not None: _asm_request_context.set_waf_address(SPAN_DATA_NAMES.REQUEST_HEADERS_NO_COOKIES, headers) _asm_request_context.set_waf_address( @@ -436,10 +445,3 @@ def on_span_finish(self, span: Span) -> None: del self._span_to_waf_ctx[s] except Exception: # nosec B110 pass - - -# load waf at the end only to avoid possible circular imports with gevent -import ddtrace.appsec._ddwaf as ddwaf # noqa: E402 -from ddtrace.appsec._metrics import _set_waf_init_metric # noqa: E402 -from ddtrace.appsec._metrics import _set_waf_request_metrics # noqa: E402 -from ddtrace.appsec._metrics import _set_waf_updates_metric # noqa: E402 diff --git a/ddtrace/contrib/internal/subprocess/patch.py b/ddtrace/contrib/internal/subprocess/patch.py index 80d05b107bb..2d66edd4737 100644 --- a/ddtrace/contrib/internal/subprocess/patch.py +++ b/ddtrace/contrib/internal/subprocess/patch.py @@ -327,6 +327,8 @@ def unpatch() -> None: @trace_utils.with_traced_module def _traced_ossystem(module, pin, wrapped, instance, args, kwargs): try: + if asm_config._bypass_instrumentation_for_waf: + return wrapped(*args, **kwargs) if isinstance(args[0], str): for callback in _STR_CALLBACKS.values(): callback(args[0]) @@ -393,6 +395,8 @@ def _traced_osspawn(module, pin, wrapped, instance, args, kwargs): @trace_utils.with_traced_module def _traced_subprocess_init(module, pin, wrapped, instance, args, kwargs): try: + if asm_config._bypass_instrumentation_for_waf: + return wrapped(*args, **kwargs) cmd_args = args[0] if len(args) else kwargs["args"] if isinstance(cmd_args, (list, tuple, str)): if kwargs.get("shell", False): @@ -425,6 +429,8 @@ def _traced_subprocess_init(module, pin, wrapped, instance, args, kwargs): @trace_utils.with_traced_module def _traced_subprocess_wait(module, pin, wrapped, instance, args, kwargs): try: + if asm_config._bypass_instrumentation_for_waf: + return wrapped(*args, **kwargs) binary = core.get_item("subprocess_popen_binary") with pin.tracer.trace(COMMANDS.SPAN_NAME, resource=binary, span_type=SpanTypes.SYSTEM) as span: diff --git a/ddtrace/settings/asm.py b/ddtrace/settings/asm.py index 16937399501..fc2c9aa13fb 100644 --- a/ddtrace/settings/asm.py +++ b/ddtrace/settings/asm.py @@ -245,6 +245,7 @@ class ASMConfig(Env): default=r"^[+-]?((0b[01]+)|(0x[0-9A-Fa-f]+)|(\d+\.?\d*(?:[Ee][+-]?\d+)?|\.\d+(?:[Ee][+-]" + r"?\d+)?)|(X\'[0-9A-Fa-f]+\')|(B\'[01]+\'))$", ) + _bypass_instrumentation_for_waf = False def __init__(self): super().__init__() From c4448ea4e5580cfa55684ef108a5dafafe1f6f75 Mon Sep 17 00:00:00 2001 From: Yun Kim <35776586+Yun-Kim@users.noreply.github.com> Date: Tue, 28 Jan 2025 17:11:42 -0500 Subject: [PATCH 06/18] fix(llmobs): propagate distributed headers via signal dispatching, not config (#12089) This PR makes a change to our shared distributed tracing header injection method to dispatch signals/events instead of relying on the global config settings, which is only modifiable via env vars. This fixes distributed tracing for users that might rely solely on the `LLMObs.enable()` setup config. Programmatic `LLMObs.enable()/disable()` calls do not set the global `config._llmobs_enabled` boolean setting, which is only controlled by the `DD_LLMOBS_ENABLED` env var. This was problematic for users that relied on manual `LLMObs.enable()` setup (i.e. no env vars) because our distributed tracing injection code only checks the global config to inject llmobs parent IDs into request headers. If users manually enabled LLMObs without any env vars, then this would not be reflected in the global config value and thus LLMObs parent IDs would never be injected into the request headers. We can't check directly if LLMObs is enabled in the http injection module because: 1. This would require us to import significant product-specific LLMObs-code into the shared http injector helper module which would impact non-LLMObs users' app performance 2. Circular imports in LLMObs which imports http injector logic to use in its own helpers Instead of doing our check based on the global `config._llmobs_enabled` setting, we now send a tracing event to our shared product listeners, and register a corresponding `LLMObs._inject_llmobs_context()` hook to be called for all inject() calls if LLMObs is enabled (we check the LLMObs instance, not the global config setting value). ~One risk and why I don't like changing global config settings is because this then implies that it is no longer global or tied to an env var (I want to push for env var configuration where possible over manual overriding/enabling). If a global enabled config can be toggled indiscriminately then this could open a can of worms for enabling/disabling logic in our LLMObs service, which isn't really designed to be toggled on/off multiple times in the app's lifespan. However if some users cannot rely on env vars, then I don't see any other solution that does not couple tracer internal code with LLMObs code which is a no-option.~ (UPDATE: we avoided this issue by using signal dispatching) ## Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) 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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) --- ddtrace/llmobs/_llmobs.py | 8 ++- ddtrace/propagation/http.py | 7 +-- ...nable-updates-config-45379a7a30e2e0e3.yaml | 5 ++ tests/llmobs/test_llmobs_service.py | 1 - tests/llmobs/test_propagation.py | 54 ++++++++++++------- tests/tracer/test_propagation.py | 24 --------- 6 files changed, 49 insertions(+), 50 deletions(-) create mode 100644 releasenotes/notes/fix-llmobs-enable-updates-config-45379a7a30e2e0e3.yaml diff --git a/ddtrace/llmobs/_llmobs.py b/ddtrace/llmobs/_llmobs.py index 768f4bdb292..bc5ddabcf00 100644 --- a/ddtrace/llmobs/_llmobs.py +++ b/ddtrace/llmobs/_llmobs.py @@ -285,6 +285,7 @@ def _stop_service(self) -> None: # Remove listener hooks for span events core.reset_listeners("trace.span_start", self._on_span_start) core.reset_listeners("trace.span_finish", self._on_span_finish) + core.reset_listeners("http.span_inject", self._inject_llmobs_context) forksafe.unregister(self._child_after_fork) @@ -369,6 +370,7 @@ def enable( # Register hooks for span events core.on("trace.span_start", cls._instance._on_span_start) core.on("trace.span_finish", cls._instance._on_span_finish) + core.on("http.span_inject", cls._instance._inject_llmobs_context) atexit.register(cls.disable) telemetry_writer.product_activated(TELEMETRY_APM_PRODUCT.LLMOBS, True) @@ -1162,6 +1164,11 @@ def submit_evaluation( cls._instance._llmobs_eval_metric_writer.enqueue(evaluation_metric) + def _inject_llmobs_context(self, span_context: Context, request_headers: Dict[str, str]) -> None: + if self.enabled is False: + return + _inject_llmobs_parent_id(span_context) + @classmethod def inject_distributed_headers(cls, request_headers: Dict[str, str], span: Optional[Span] = None) -> Dict[str, str]: """Injects the span's distributed context into the given request headers.""" @@ -1179,7 +1186,6 @@ def inject_distributed_headers(cls, request_headers: Dict[str, str], span: Optio if span is None: log.warning("No span provided and no currently active span found.") return request_headers - _inject_llmobs_parent_id(span.context) HTTPPropagator.inject(span.context, request_headers) return request_headers diff --git a/ddtrace/propagation/http.py b/ddtrace/propagation/http.py index fdaf97410ad..381acabb1bc 100644 --- a/ddtrace/propagation/http.py +++ b/ddtrace/propagation/http.py @@ -28,6 +28,7 @@ from ddtrace._trace.span import _get_64_lowest_order_bits_as_int from ddtrace._trace.span import _MetaDictType from ddtrace.appsec._constants import APPSEC +from ddtrace.internal.core import dispatch from ddtrace.settings.asm import config as asm_config from ..constants import AUTO_KEEP @@ -1052,6 +1053,7 @@ def parent_call(): :param dict headers: HTTP headers to extend with tracing attributes. :param Span non_active_span: Only to be used if injecting a non-active span. """ + dispatch("http.span_inject", (span_context, headers)) if not config._propagation_style_inject: return if non_active_span is not None and non_active_span.context is not span_context: @@ -1089,11 +1091,6 @@ def parent_call(): for key in span_context._baggage: headers[_HTTP_BAGGAGE_PREFIX + key] = span_context._baggage[key] - if config._llmobs_enabled: - from ddtrace.llmobs._utils import _inject_llmobs_parent_id - - _inject_llmobs_parent_id(span_context) - if PROPAGATION_STYLE_DATADOG in config._propagation_style_inject: _DatadogMultiHeader._inject(span_context, headers) if PROPAGATION_STYLE_B3_MULTI in config._propagation_style_inject: diff --git a/releasenotes/notes/fix-llmobs-enable-updates-config-45379a7a30e2e0e3.yaml b/releasenotes/notes/fix-llmobs-enable-updates-config-45379a7a30e2e0e3.yaml new file mode 100644 index 00000000000..4bf312f4680 --- /dev/null +++ b/releasenotes/notes/fix-llmobs-enable-updates-config-45379a7a30e2e0e3.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + LLM Observability: Resolves an issue where explicitly only using ``LLMObs.enable()`` to configure LLM Observability + without environment variables would not automatically propagate distributed tracing headers. diff --git a/tests/llmobs/test_llmobs_service.py b/tests/llmobs/test_llmobs_service.py index de428999147..744fde885e9 100644 --- a/tests/llmobs/test_llmobs_service.py +++ b/tests/llmobs/test_llmobs_service.py @@ -60,7 +60,6 @@ def test_service_enable_proxy_default(): assert llmobs_instance.tracer == dummy_tracer assert isinstance(llmobs_instance._llmobs_span_writer._clients[0], LLMObsProxiedEventClient) assert run_llmobs_trace_filter(dummy_tracer) is not None - llmobs_service.disable() diff --git a/tests/llmobs/test_propagation.py b/tests/llmobs/test_propagation.py index e3ab9c80d66..27421e6b12f 100644 --- a/tests/llmobs/test_propagation.py +++ b/tests/llmobs/test_propagation.py @@ -57,20 +57,24 @@ def test_propagate_correct_llmobs_parent_id_simple(run_python_code_in_subprocess """ code = """ import json +import mock -from ddtrace import tracer -from ddtrace.ext import SpanTypes +from ddtrace.internal.utils.http import Response +from ddtrace.llmobs import LLMObs from ddtrace.propagation.http import HTTPPropagator -with tracer.trace("LLMObs span", span_type=SpanTypes.LLM) as root_span: - with tracer.trace("Non-LLMObs span") as child_span: - headers = {"_DD_LLMOBS_SPAN_ID": str(root_span.span_id)} - HTTPPropagator.inject(child_span.context, headers) +with mock.patch( + "ddtrace.internal.writer.HTTPWriter._send_payload", return_value=Response(status=200, body="{}"), +): + LLMObs.enable(ml_app="test-app", api_key="", agentless_enabled=True) + with LLMObs.workflow("LLMObs span") as root_span: + with LLMObs._instance.tracer.trace("Non-LLMObs span") as child_span: + headers = {"_DD_LLMOBS_SPAN_ID": str(root_span.span_id)} + HTTPPropagator.inject(child_span.context, headers) print(json.dumps(headers)) """ env = os.environ.copy() - env["DD_LLMOBS_ENABLED"] = "1" env["DD_TRACE_ENABLED"] = "0" stdout, stderr, status, _ = run_python_code_in_subprocess(code=code, env=env) assert status == 0, (stdout, stderr) @@ -93,21 +97,33 @@ def test_propagate_llmobs_parent_id_complex(run_python_code_in_subprocess): """ code = """ import json +import mock -from ddtrace import tracer -from ddtrace.ext import SpanTypes +from ddtrace.internal.utils.http import Response +from ddtrace.llmobs import LLMObs from ddtrace.propagation.http import HTTPPropagator -with tracer.trace("LLMObs span", span_type=SpanTypes.LLM) as root_span: - with tracer.trace("Non-LLMObs span") as child_span: - headers = {"_DD_LLMOBS_SPAN_ID": str(root_span.span_id)} - HTTPPropagator.inject(child_span.context, headers) +with mock.patch( + "ddtrace.internal.writer.HTTPWriter._send_payload", return_value=Response(status=200, body="{}"), +): + from ddtrace import auto # simulate ddtrace-run startup to ensure env var configs also propagate + with LLMObs.workflow("LLMObs span") as root_span: + with LLMObs._instance.tracer.trace("Non-LLMObs span") as child_span: + headers = {"_DD_LLMOBS_SPAN_ID": str(root_span.span_id)} + HTTPPropagator.inject(child_span.context, headers) print(json.dumps(headers)) """ env = os.environ.copy() - env["DD_LLMOBS_ENABLED"] = "1" - env["DD_TRACE_ENABLED"] = "0" + env.update( + { + "DD_LLMOBS_ENABLED": "1", + "DD_TRACE_ENABLED": "0", + "DD_AGENTLESS_ENABLED": "1", + "DD_API_KEY": "", + "DD_LLMOBS_ML_APP": "test-app", + } + ) stdout, stderr, status, _ = run_python_code_in_subprocess(code=code, env=env) assert status == 0, (stdout, stderr) assert stderr == b"", (stdout, stderr) @@ -124,7 +140,7 @@ def test_propagate_llmobs_parent_id_complex(run_python_code_in_subprocess): def test_no_llmobs_parent_id_propagated_if_no_llmobs_spans(run_python_code_in_subprocess): - """Test that the correct LLMObs parent ID (None) is extracted from the headers in a simple distributed scenario. + """Test that the correct LLMObs parent ID ('undefined') is extracted from headers in a simple distributed scenario. Service A (subprocess) has spans, but none are LLMObs spans. Service B (outside subprocess) has a LLMObs span. Service B's span should have no LLMObs parent ID as there are no LLMObs spans from service A. @@ -132,17 +148,17 @@ def test_no_llmobs_parent_id_propagated_if_no_llmobs_spans(run_python_code_in_su code = """ import json -from ddtrace import tracer +from ddtrace.llmobs import LLMObs from ddtrace.propagation.http import HTTPPropagator -with tracer.trace("Non-LLMObs span") as root_span: +LLMObs.enable(ml_app="ml-app", agentless_enabled=True, api_key="") +with LLMObs._instance.tracer.trace("Non-LLMObs span") as root_span: headers = {} HTTPPropagator.inject(root_span.context, headers) print(json.dumps(headers)) """ env = os.environ.copy() - env["DD_LLMOBS_ENABLED"] = "1" env["DD_TRACE_ENABLED"] = "0" stdout, stderr, status, _ = run_python_code_in_subprocess(code=code, env=env) assert status == 0, (stdout, stderr) diff --git a/tests/tracer/test_propagation.py b/tests/tracer/test_propagation.py index 533e4974250..b073cd72e3b 100644 --- a/tests/tracer/test_propagation.py +++ b/tests/tracer/test_propagation.py @@ -4,7 +4,6 @@ import os import pickle -import mock import pytest import ddtrace @@ -3387,29 +3386,6 @@ def test_DD_TRACE_PROPAGATION_STYLE_INJECT_overrides_DD_TRACE_PROPAGATION_STYLE( assert result == expected_headers -def test_llmobs_enabled_injects_llmobs_parent_id(): - with override_global_config(dict(_llmobs_enabled=True)): - with mock.patch("ddtrace.llmobs._utils._inject_llmobs_parent_id") as mock_llmobs_inject: - context = Context(trace_id=1, span_id=2) - HTTPPropagator.inject(context, {}) - mock_llmobs_inject.assert_called_once_with(context) - - -def test_llmobs_disabled_does_not_inject_parent_id(): - with override_global_config(dict(_llmobs_enabled=False)): - with mock.patch("ddtrace.llmobs._utils._inject_llmobs_parent_id") as mock_llmobs_inject: - context = Context(trace_id=1, span_id=2) - HTTPPropagator.inject(context, {}) - mock_llmobs_inject.assert_not_called() - - -def test_llmobs_parent_id_not_injected_by_default(): - with mock.patch("ddtrace.llmobs._utils._inject_llmobs_parent_id") as mock_llmobs_inject: - context = Context(trace_id=1, span_id=2) - HTTPPropagator.inject(context, {}) - mock_llmobs_inject.assert_not_called() - - @pytest.mark.parametrize( "span_context,expected_headers", [ From cb41f8e77aa71bfffbb63f7ee1a9808e659189d2 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Wed, 29 Jan 2025 08:39:44 -0500 Subject: [PATCH 07/18] feat(provider): expose context provider in ddtrace.trace (#12135) ddtrace v3.0 is set to remove `ddtrace.providers` module from the public API. However we should still allow users to use their own ContextProviders. This PR ensures the BaseContextProvider remains in the public API and can be used in `tracer.configure(...)`. ## Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) 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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) --- .github/CODEOWNERS | 2 ++ ddtrace/provider.py | 5 +++-- ddtrace/trace/__init__.py | 2 ++ docs/advanced_usage.rst | 2 +- .../feat-expose-base-context-provider-530ebec2225f6c8d.yaml | 5 +++++ 5 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/feat-expose-base-context-provider-530ebec2225f6c8d.yaml diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 84e6c873642..b1162da2348 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -166,12 +166,14 @@ ddtrace/internal/remoteconfig @DataDog/remote-config @DataDog/apm-core-pyt tests/internal/remoteconfig @DataDog/remote-config @DataDog/apm-core-python # API SDK +ddtrace/trace/ @DataDog/apm-sdk-api-python ddtrace/_trace/ @DataDog/apm-sdk-api-python ddtrace/opentelemetry/ @DataDog/apm-sdk-api-python ddtrace/internal/opentelemetry @DataDog/apm-sdk-api-python ddtrace/opentracer/ @DataDog/apm-sdk-api-python ddtrace/propagation/ @DataDog/apm-sdk-api-python ddtrace/filters.py @DataDog/apm-sdk-api-python +ddtrace/provider.py @DataDog/apm-sdk-api-python ddtrace/pin.py @DataDog/apm-sdk-api-python ddtrace/sampler.py @DataDog/apm-sdk-api-python ddtrace/sampling_rule.py @DataDog/apm-sdk-api-python diff --git a/ddtrace/provider.py b/ddtrace/provider.py index 4a275ece8c8..7b9867de01a 100644 --- a/ddtrace/provider.py +++ b/ddtrace/provider.py @@ -7,7 +7,8 @@ deprecate( - "The context provider interface is deprecated.", - message="The trace context is an internal interface and will no longer be supported.", + "The context provider interface is deprecated", + message="Import BaseContextProvider from `ddtrace.trace` instead.", category=DDTraceDeprecationWarning, + removal_version="3.0.0", ) diff --git a/ddtrace/trace/__init__.py b/ddtrace/trace/__init__.py index f709310d589..8473c0b99f0 100644 --- a/ddtrace/trace/__init__.py +++ b/ddtrace/trace/__init__.py @@ -1,6 +1,7 @@ from ddtrace._trace.context import Context from ddtrace._trace.filters import TraceFilter from ddtrace._trace.pin import Pin +from ddtrace._trace.provider import BaseContextProvider from ddtrace._trace.span import Span from ddtrace._trace.tracer import Tracer @@ -15,4 +16,5 @@ "Tracer", "Span", "tracer", + "BaseContextProvider", ] diff --git a/docs/advanced_usage.rst b/docs/advanced_usage.rst index 05879acac37..392e1406fb7 100644 --- a/docs/advanced_usage.rst +++ b/docs/advanced_usage.rst @@ -221,7 +221,7 @@ context management. If there is a case where the default is insufficient then a custom context provider can be used. It must implement the -:class:`ddtrace.provider.BaseContextProvider` interface and can be configured +:class:`ddtrace.trace.BaseContextProvider` interface and can be configured with:: tracer.configure(context_provider=MyContextProvider) diff --git a/releasenotes/notes/feat-expose-base-context-provider-530ebec2225f6c8d.yaml b/releasenotes/notes/feat-expose-base-context-provider-530ebec2225f6c8d.yaml new file mode 100644 index 00000000000..010d2bf52bd --- /dev/null +++ b/releasenotes/notes/feat-expose-base-context-provider-530ebec2225f6c8d.yaml @@ -0,0 +1,5 @@ +--- +deprecations: + - | + tracing: Moves ``ddtrace.provider.BaseContextProvider`` to ``ddtrace.trace.BaseContextProvider``. + The ``ddtrace.provider`` module is deprecated and will be removed in v3.0.0. From af9098c4343831d4b8771fe5debd6f99d021eac6 Mon Sep 17 00:00:00 2001 From: Laplie Anderson Date: Thu, 30 Jan 2025 05:34:25 -0500 Subject: [PATCH 08/18] chore(ci): skip non-linux OCI package creation (#12036) The shared pipeline is introducing support for windows OCI packages. This PR ensure dd-trace-py doesn't generate nonsensical packages. Windows support can be added later. This was tested by using the WIP branch of one-pipeline ## Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] 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](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) 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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) --------- Co-authored-by: Brett Langdon --- .gitlab/prepare-oci-package.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.gitlab/prepare-oci-package.sh b/.gitlab/prepare-oci-package.sh index 5958c31e731..7ee7b7d6e77 100755 --- a/.gitlab/prepare-oci-package.sh +++ b/.gitlab/prepare-oci-package.sh @@ -1,6 +1,11 @@ #!/bin/bash set -eo pipefail +if [ "$OS" != "linux" ]; then + echo "Only linux packages are supported. Exiting" + exit 0 +fi + if [ -n "$CI_COMMIT_TAG" ] && [ -z "$PYTHON_PACKAGE_VERSION" ]; then PYTHON_PACKAGE_VERSION=${CI_COMMIT_TAG##v} fi From c93ff44092a1e5452aca7695162fa7fe2d753262 Mon Sep 17 00:00:00 2001 From: Louis Tricot Date: Fri, 31 Jan 2025 10:46:26 +0100 Subject: [PATCH 09/18] feat: add a record_exception method to the tracer --- ddtrace/_trace/tracer.py | 48 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/ddtrace/_trace/tracer.py b/ddtrace/_trace/tracer.py index 6e595bbe7c1..9cd30ab82c6 100644 --- a/ddtrace/_trace/tracer.py +++ b/ddtrace/_trace/tracer.py @@ -1,10 +1,13 @@ import functools +from io import StringIO from itertools import chain import logging import os from os import environ from os import getpid from threading import RLock +from time import time_ns +import traceback from typing import Any from typing import Callable from typing import Dict @@ -1244,6 +1247,51 @@ def set_tags(self, tags: Dict[str, str]) -> None: """ self._tags.update(tags) + def record_exception( + self, + exception: Exception, + attributes: Optional[Dict[str, str]] = None, + timestamp: Optional[int] = None, + escaped=False, + ) -> None: + """ + Records an exception as an event if the exception is handled (escaped=False) + If the exception is not handled (escaped=True), tag the span with an error tuple + + There is no real use case of recording an unhandled exceptions as the tracer does it + automatically. It is just to comply with otel spec. + """ + current_span = self.current_span() + if current_span is None: + return + + if timestamp is None: + timestamp = time_ns() + + exc_type, exc_val, exc_tb = type(exception), exception, exception.__traceback__ + + if escaped is True: + current_span.set_exc_info(exc_type, exc_val, exc_tb) + + # get the traceback + buff = StringIO() + traceback.print_exception(exc_type, exc_val, exc_tb, file=buff, limit=config._span_traceback_max_size) + tb = buff.getvalue() + + # Set exception attributes in a manner that is consistent with the opentelemetry sdk + # https://github.com/open-telemetry/opentelemetry-python/blob/v1.24.0/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py#L998 + attrs = { + "exception.type": "%s.%s" % (exception.__class__.__module__, exception.__class__.__name__), + "exception.message": str(exception), + "exception.escaped": str(escaped), + "exception.stacktrace": tb, + } + if attributes: + # User provided attributes must take precedence over attrs + attrs.update(attributes) + + current_span._add_event(name="recorded exception", attributes=attrs, timestamp=timestamp) + def shutdown(self, timeout: Optional[float] = None) -> None: """Shutdown the tracer and flush finished traces. Avoid calling shutdown multiple times. From 9943d70901f08fb72921c1c8460423aa96376133 Mon Sep 17 00:00:00 2001 From: Louis Tricot Date: Fri, 31 Jan 2025 11:27:10 +0100 Subject: [PATCH 10/18] chore: add record_exception_tests --- tests/tracer/test_tracer.py | 75 +++++++++++++++++++++++++++++++++++++ tests/utils.py | 13 +++++++ 2 files changed, 88 insertions(+) diff --git a/tests/tracer/test_tracer.py b/tests/tracer/test_tracer.py index 1c45f424679..01b3a2dd021 100644 --- a/tests/tracer/test_tracer.py +++ b/tests/tracer/test_tracer.py @@ -164,6 +164,81 @@ def f(): ), ) + def test_tracer_record_exception(self): + @self.tracer.wrap() + def f(): + try: + raise Exception("bim") + except Exception as e: + self.tracer.record_exception(e) + + f() + self.assert_structure( + dict( + name="tests.tracer.test_tracer.f", + error=0, + ), + ) + self.assert_span_event_count(1) + self.assert_span_event_attributes( + 0, {"exception.type": "builtins.Exception", "exception.message": "bim", "exception.escaped": "False"} + ) + + def test_tracer_record_multiple_exceptions(self): + @self.tracer.wrap() + def f(): + try: + raise Exception("bim") + except Exception as e: + self.tracer.record_exception(e) + + try: + raise Exception("bam") + except Exception as e: + self.tracer.record_exception(e) + + f() + self.assert_span_event_count(2) + self.assert_structure( + dict( + name="tests.tracer.test_tracer.f", + error=0, + ) + ) + self.assert_span_event_attributes( + 0, {"exception.type": "builtins.Exception", "exception.message": "bim", "exception.escaped": "False"} + ) + self.assert_span_event_attributes( + 1, {"exception.type": "builtins.Exception", "exception.message": "bam", "exception.escaped": "False"} + ) + + def test_tracer_record_escaped_exception(self): + exc = Exception("bim") + + @self.tracer.wrap() + def f(): + try: + raise exc + except Exception as e: + self.tracer.record_exception(e, escaped=True) + + f() + + self.assert_structure( + dict( + name="tests.tracer.test_tracer.f", + error=1, + meta={ + "error.message": str(exc), + "error.type": "%s.%s" % (exc.__class__.__module__, exc.__class__.__name__), + }, + ), + ) + self.assert_span_event_count(1) + self.assert_span_event_attributes( + 0, {"exception.type": "builtins.Exception", "exception.message": "bim", "exception.escaped": "True"} + ) + def test_tracer_wrap_multiple_calls(self): @self.tracer.wrap() def f(): diff --git a/tests/utils.py b/tests/utils.py index 5283e27e7cf..13182d51d88 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -519,6 +519,19 @@ def assert_structure(self, root, children=NO_CHILDREN): root_span = self.get_root_span() root_span.assert_structure(root, children) + def assert_span_event_count(self, count): + "Assert the r" + root_span = self.get_root_span() + assert len(root_span._events) == count, "Span count {0} != {1}".format(len(root_span._events), count) + + def assert_span_event_attributes(self, event_idx, attrs): + span_event_attrs = self.get_root_span()._events[event_idx].attributes + for name, value in attrs.items(): + assert name in span_event_attrs, "{0!r} does not have property {1!r}".format(span_event_attrs, name) + assert span_event_attrs[name] == value, "{0!r} property {1}: {2!r} != {3!r}".format( + span_event_attrs, name, span_event_attrs[name], value + ) + @contextlib.contextmanager def override_global_tracer(self, tracer=None): original = ddtrace.tracer From 296cff288a82b5b7f83afcef89e9d3f882ac6ace Mon Sep 17 00:00:00 2001 From: Louis Tricot Date: Fri, 31 Jan 2025 15:59:52 +0100 Subject: [PATCH 11/18] chore: enrich documentation and add release note --- ddtrace/_trace/tracer.py | 14 +++++---- ...-dd-record-exception-033fd0436dfd2723.yaml | 30 +++++++++++++++++++ 2 files changed, 39 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/feat-add-dd-record-exception-033fd0436dfd2723.yaml diff --git a/ddtrace/_trace/tracer.py b/ddtrace/_trace/tracer.py index 9cd30ab82c6..788730914f4 100644 --- a/ddtrace/_trace/tracer.py +++ b/ddtrace/_trace/tracer.py @@ -1255,11 +1255,15 @@ def record_exception( escaped=False, ) -> None: """ - Records an exception as an event if the exception is handled (escaped=False) - If the exception is not handled (escaped=True), tag the span with an error tuple - - There is no real use case of recording an unhandled exceptions as the tracer does it - automatically. It is just to comply with otel spec. + Records an exception as span event. + If the exception is unhandled, :obj:`escaped` should be set :obj:`True`. It + will tag the span with an error tuple. + + :param Exception exception: the exception to record< + :param dict attributes: optional attributes to add to the span event. It will override + the base attributes if :obj:`attributes` contains existing keys. + :param int timestamp: the timestamp of the span event. Will be set to now() if timestamp is :obj:`None`. + :param bool escaped: sets to :obj:`False` for a handled exception and :obj:`True` for an unhandled exception. """ current_span = self.current_span() if current_span is None: diff --git a/releasenotes/notes/feat-add-dd-record-exception-033fd0436dfd2723.yaml b/releasenotes/notes/feat-add-dd-record-exception-033fd0436dfd2723.yaml new file mode 100644 index 00000000000..e5f202e39fb --- /dev/null +++ b/releasenotes/notes/feat-add-dd-record-exception-033fd0436dfd2723.yaml @@ -0,0 +1,30 @@ +--- +#instructions: +# The style guide below provides explanations, instructions, and templates to write your own release note. +# Once finished, all irrelevant sections (including this instruction section) should be removed, +# and the release note should be committed with the rest of the changes. +# +# The main goal of a release note is to provide a brief overview of a change and provide actionable steps to the user. +# The release note should clearly communicate what the change is, why the change was made, and how a user can migrate their code. +# +# The release note should also clearly distinguish between announcements and user instructions. Use: +# * Past tense for previous/existing behavior (ex: ``resulted, caused, failed``) +# * Third person present tense for the change itself (ex: ``adds, fixes, upgrades``) +# * Active present infinitive for user instructions (ex: ``set, use, add``) +# +# Release notes should: +# * Use plain language +# * Be concise +# * Include actionable steps with the necessary code changes +# * Include relevant links (bug issues, upstream issues or release notes, documentation pages) +# * Use full sentences with sentence-casing and punctuation. +# * Before using Datadog specific acronyms/terminology, a release note must first introduce them with a definition. +# +# Release notes should not: +# * Be vague. Example: ``fixes an issue in tracing``. +# * Use overly technical language +# * Use dynamic links (``stable/latest/1.x`` URLs). Instead, use static links (specific version, commit hash) whenever possible so that they don't break in the future. +features: + - | + tracing: This introduces the record_exception tracer api. It allows to manually add an exception as a span event. + It also allows to tag a span with an error tuple of the recorded exception by setting the escaped parameter as true. \ No newline at end of file From 8cefbef6845b668f41bf46c774f014b057953df0 Mon Sep 17 00:00:00 2001 From: Louis Tricot Date: Fri, 31 Jan 2025 16:35:01 +0100 Subject: [PATCH 12/18] fix: unhappy dd-bot --- ddtrace/_trace/tracer.py | 2 +- tests/tracer/test_tracer.py | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/ddtrace/_trace/tracer.py b/ddtrace/_trace/tracer.py index 788730914f4..b0429b65690 100644 --- a/ddtrace/_trace/tracer.py +++ b/ddtrace/_trace/tracer.py @@ -1274,7 +1274,7 @@ def record_exception( exc_type, exc_val, exc_tb = type(exception), exception, exception.__traceback__ - if escaped is True: + if escaped: current_span.set_exc_info(exc_type, exc_val, exc_tb) # get the traceback diff --git a/tests/tracer/test_tracer.py b/tests/tracer/test_tracer.py index 01b3a2dd021..5a64b12148a 100644 --- a/tests/tracer/test_tracer.py +++ b/tests/tracer/test_tracer.py @@ -168,8 +168,8 @@ def test_tracer_record_exception(self): @self.tracer.wrap() def f(): try: - raise Exception("bim") - except Exception as e: + raise RuntimeError("bim") + except RuntimeError as e: self.tracer.record_exception(e) f() @@ -181,20 +181,20 @@ def f(): ) self.assert_span_event_count(1) self.assert_span_event_attributes( - 0, {"exception.type": "builtins.Exception", "exception.message": "bim", "exception.escaped": "False"} + 0, {"exception.type": "builtins.RuntimeError", "exception.message": "bim", "exception.escaped": "False"} ) def test_tracer_record_multiple_exceptions(self): @self.tracer.wrap() def f(): try: - raise Exception("bim") - except Exception as e: + raise RuntimeError("bim") + except RuntimeError as e: self.tracer.record_exception(e) try: - raise Exception("bam") - except Exception as e: + raise RuntimeError("bam") + except RuntimeError as e: self.tracer.record_exception(e) f() @@ -206,20 +206,20 @@ def f(): ) ) self.assert_span_event_attributes( - 0, {"exception.type": "builtins.Exception", "exception.message": "bim", "exception.escaped": "False"} + 0, {"exception.type": "builtins.RuntimeError", "exception.message": "bim", "exception.escaped": "False"} ) self.assert_span_event_attributes( - 1, {"exception.type": "builtins.Exception", "exception.message": "bam", "exception.escaped": "False"} + 1, {"exception.type": "builtins.RuntimeError", "exception.message": "bam", "exception.escaped": "False"} ) def test_tracer_record_escaped_exception(self): - exc = Exception("bim") + exc = RuntimeError("bim") @self.tracer.wrap() def f(): try: raise exc - except Exception as e: + except RuntimeError as e: self.tracer.record_exception(e, escaped=True) f() @@ -236,7 +236,7 @@ def f(): ) self.assert_span_event_count(1) self.assert_span_event_attributes( - 0, {"exception.type": "builtins.Exception", "exception.message": "bim", "exception.escaped": "True"} + 0, {"exception.type": "builtins.RuntimeError", "exception.message": "bim", "exception.escaped": "True"} ) def test_tracer_wrap_multiple_calls(self): From 349990fb962ee973138229f918a928c120fb26c4 Mon Sep 17 00:00:00 2001 From: Louis Tricot Date: Mon, 3 Feb 2025 09:22:08 +0100 Subject: [PATCH 13/18] fix: sphinx does not know the word unhandled --- ddtrace/_trace/tracer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ddtrace/_trace/tracer.py b/ddtrace/_trace/tracer.py index b0429b65690..406f01a1654 100644 --- a/ddtrace/_trace/tracer.py +++ b/ddtrace/_trace/tracer.py @@ -1256,14 +1256,14 @@ def record_exception( ) -> None: """ Records an exception as span event. - If the exception is unhandled, :obj:`escaped` should be set :obj:`True`. It + If the exception is uncaught, :obj:`escaped` should be set :obj:`True`. It will tag the span with an error tuple. :param Exception exception: the exception to record< :param dict attributes: optional attributes to add to the span event. It will override the base attributes if :obj:`attributes` contains existing keys. :param int timestamp: the timestamp of the span event. Will be set to now() if timestamp is :obj:`None`. - :param bool escaped: sets to :obj:`False` for a handled exception and :obj:`True` for an unhandled exception. + :param bool escaped: sets to :obj:`False` for a handled exception and :obj:`True` for a uncaught exception. """ current_span = self.current_span() if current_span is None: From 8202b1b1b2267d6bfec53d1e6207c21c3bdd4644 Mon Sep 17 00:00:00 2001 From: Louis Tricot Date: Wed, 5 Feb 2025 11:55:52 +0100 Subject: [PATCH 14/18] chore: change place of assert_span_event_count/_attributes --- tests/tracer/test_tracer.py | 14 +++++++------- tests/utils.py | 36 +++++++++++++++++++++++------------- 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/tests/tracer/test_tracer.py b/tests/tracer/test_tracer.py index 5a64b12148a..5e68e6bc588 100644 --- a/tests/tracer/test_tracer.py +++ b/tests/tracer/test_tracer.py @@ -179,8 +179,8 @@ def f(): error=0, ), ) - self.assert_span_event_count(1) - self.assert_span_event_attributes( + self.spans[0].assert_span_event_count(1) + self.spans[0].assert_span_event_attributes( 0, {"exception.type": "builtins.RuntimeError", "exception.message": "bim", "exception.escaped": "False"} ) @@ -198,17 +198,17 @@ def f(): self.tracer.record_exception(e) f() - self.assert_span_event_count(2) self.assert_structure( dict( name="tests.tracer.test_tracer.f", error=0, ) ) - self.assert_span_event_attributes( + self.spans[0].assert_span_event_count(2) + self.spans[0].assert_span_event_attributes( 0, {"exception.type": "builtins.RuntimeError", "exception.message": "bim", "exception.escaped": "False"} ) - self.assert_span_event_attributes( + self.spans[0].assert_span_event_attributes( 1, {"exception.type": "builtins.RuntimeError", "exception.message": "bam", "exception.escaped": "False"} ) @@ -234,8 +234,8 @@ def f(): }, ), ) - self.assert_span_event_count(1) - self.assert_span_event_attributes( + self.spans[0].assert_span_event_count(1) + self.spans[0].assert_span_event_attributes( 0, {"exception.type": "builtins.RuntimeError", "exception.message": "bim", "exception.escaped": "True"} ) diff --git a/tests/utils.py b/tests/utils.py index 13182d51d88..1c29bc421d3 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -519,19 +519,6 @@ def assert_structure(self, root, children=NO_CHILDREN): root_span = self.get_root_span() root_span.assert_structure(root, children) - def assert_span_event_count(self, count): - "Assert the r" - root_span = self.get_root_span() - assert len(root_span._events) == count, "Span count {0} != {1}".format(len(root_span._events), count) - - def assert_span_event_attributes(self, event_idx, attrs): - span_event_attrs = self.get_root_span()._events[event_idx].attributes - for name, value in attrs.items(): - assert name in span_event_attrs, "{0!r} does not have property {1!r}".format(span_event_attrs, name) - assert span_event_attrs[name] == value, "{0!r} property {1}: {2!r} != {3!r}".format( - span_event_attrs, name, span_event_attrs[name], value - ) - @contextlib.contextmanager def override_global_tracer(self, tracer=None): original = ddtrace.tracer @@ -860,6 +847,29 @@ def assert_metrics(self, metrics, exact=False): self, key, self._metrics[key], value ) + def assert_span_event_count(self, count): + """Assert this span has the expected number of span_events""" + assert len(self._events) == count, "Span count {0} != {1}".format(len(self._events), count) + + def assert_span_event_attributes(self, event_idx, attrs): + """ + Assertion method to ensure this span's span event match as expected + + Example:: + + span = TestSpan(span) + span.assert_span_event(0, {"exception.type": "builtins.RuntimeError"}) + + :param event_idx: id of the span event + :type event_idx: integer + """ + span_event_attrs = self._events[event_idx].attributes + for name, value in attrs.items(): + assert name in span_event_attrs, "{0!r} does not have property {1!r}".format(span_event_attrs, name) + assert span_event_attrs[name] == value, "{0!r} property {1}: {2!r} != {3!r}".format( + span_event_attrs, name, span_event_attrs[name], value + ) + class TracerSpanContainer(TestSpanContainer): """ From 9ae4725313e926580472292abd86c12d4196b6ec Mon Sep 17 00:00:00 2001 From: Louis Tricot Date: Thu, 6 Feb 2025 10:24:51 +0100 Subject: [PATCH 15/18] changes according to review --- ddtrace/_trace/span.py | 45 +++++++++++ ddtrace/_trace/tracer.py | 52 ------------- ...-dd-record-exception-033fd0436dfd2723.yaml | 30 +------- tests/tracer/test_span.py | 55 ++++++++++++++ tests/tracer/test_tracer.py | 75 ------------------- 5 files changed, 103 insertions(+), 154 deletions(-) diff --git a/ddtrace/_trace/span.py b/ddtrace/_trace/span.py index 446239a8091..dba0df89c6c 100644 --- a/ddtrace/_trace/span.py +++ b/ddtrace/_trace/span.py @@ -552,6 +552,51 @@ def set_exc_info( core.dispatch("span.exception", (self, exc_type, exc_val, exc_tb)) + def record_exception( + self, + exception: Exception, + attributes: Optional[Dict[str, str]] = None, + timestamp: Optional[int] = None, + escaped=False, + ) -> None: + """ + Records an exception as span event. + If the exception is uncaught, :obj:`escaped` should be set :obj:`True`. It + will tag the span with an error tuple. + + :param Exception exception: the exception to record< + :param dict attributes: optional attributes to add to the span event. It will override + the base attributes if :obj:`attributes` contains existing keys. + :param int timestamp: the timestamp of the span event. Will be set to now() if timestamp is :obj:`None`. + :param bool escaped: sets to :obj:`False` for a handled exception and :obj:`True` for a uncaught exception. + """ + if timestamp is None: + timestamp = time_ns() + + exc_type, exc_val, exc_tb = type(exception), exception, exception.__traceback__ + + if escaped: + self.set_exc_info(exc_type, exc_val, exc_tb) + + # get the traceback + buff = StringIO() + traceback.print_exception(exc_type, exc_val, exc_tb, file=buff, limit=config._span_traceback_max_size) + tb = buff.getvalue() + + # Set exception attributes in a manner that is consistent with the opentelemetry sdk + # https://github.com/open-telemetry/opentelemetry-python/blob/v1.24.0/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py#L998 + attrs = { + "exception.type": "%s.%s" % (exception.__class__.__module__, exception.__class__.__name__), + "exception.message": str(exception), + "exception.escaped": str(escaped), + "exception.stacktrace": tb, + } + if attributes: + # User provided attributes must take precedence over attrs + attrs.update(attributes) + + self._add_event(name="recorded exception", attributes=attrs, timestamp=timestamp) + def _pprint(self) -> str: """Return a human readable version of the span.""" data = [ diff --git a/ddtrace/_trace/tracer.py b/ddtrace/_trace/tracer.py index 406f01a1654..6e595bbe7c1 100644 --- a/ddtrace/_trace/tracer.py +++ b/ddtrace/_trace/tracer.py @@ -1,13 +1,10 @@ import functools -from io import StringIO from itertools import chain import logging import os from os import environ from os import getpid from threading import RLock -from time import time_ns -import traceback from typing import Any from typing import Callable from typing import Dict @@ -1247,55 +1244,6 @@ def set_tags(self, tags: Dict[str, str]) -> None: """ self._tags.update(tags) - def record_exception( - self, - exception: Exception, - attributes: Optional[Dict[str, str]] = None, - timestamp: Optional[int] = None, - escaped=False, - ) -> None: - """ - Records an exception as span event. - If the exception is uncaught, :obj:`escaped` should be set :obj:`True`. It - will tag the span with an error tuple. - - :param Exception exception: the exception to record< - :param dict attributes: optional attributes to add to the span event. It will override - the base attributes if :obj:`attributes` contains existing keys. - :param int timestamp: the timestamp of the span event. Will be set to now() if timestamp is :obj:`None`. - :param bool escaped: sets to :obj:`False` for a handled exception and :obj:`True` for a uncaught exception. - """ - current_span = self.current_span() - if current_span is None: - return - - if timestamp is None: - timestamp = time_ns() - - exc_type, exc_val, exc_tb = type(exception), exception, exception.__traceback__ - - if escaped: - current_span.set_exc_info(exc_type, exc_val, exc_tb) - - # get the traceback - buff = StringIO() - traceback.print_exception(exc_type, exc_val, exc_tb, file=buff, limit=config._span_traceback_max_size) - tb = buff.getvalue() - - # Set exception attributes in a manner that is consistent with the opentelemetry sdk - # https://github.com/open-telemetry/opentelemetry-python/blob/v1.24.0/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py#L998 - attrs = { - "exception.type": "%s.%s" % (exception.__class__.__module__, exception.__class__.__name__), - "exception.message": str(exception), - "exception.escaped": str(escaped), - "exception.stacktrace": tb, - } - if attributes: - # User provided attributes must take precedence over attrs - attrs.update(attributes) - - current_span._add_event(name="recorded exception", attributes=attrs, timestamp=timestamp) - def shutdown(self, timeout: Optional[float] = None) -> None: """Shutdown the tracer and flush finished traces. Avoid calling shutdown multiple times. diff --git a/releasenotes/notes/feat-add-dd-record-exception-033fd0436dfd2723.yaml b/releasenotes/notes/feat-add-dd-record-exception-033fd0436dfd2723.yaml index e5f202e39fb..8a6231e3ce8 100644 --- a/releasenotes/notes/feat-add-dd-record-exception-033fd0436dfd2723.yaml +++ b/releasenotes/notes/feat-add-dd-record-exception-033fd0436dfd2723.yaml @@ -1,30 +1,6 @@ --- -#instructions: -# The style guide below provides explanations, instructions, and templates to write your own release note. -# Once finished, all irrelevant sections (including this instruction section) should be removed, -# and the release note should be committed with the rest of the changes. -# -# The main goal of a release note is to provide a brief overview of a change and provide actionable steps to the user. -# The release note should clearly communicate what the change is, why the change was made, and how a user can migrate their code. -# -# The release note should also clearly distinguish between announcements and user instructions. Use: -# * Past tense for previous/existing behavior (ex: ``resulted, caused, failed``) -# * Third person present tense for the change itself (ex: ``adds, fixes, upgrades``) -# * Active present infinitive for user instructions (ex: ``set, use, add``) -# -# Release notes should: -# * Use plain language -# * Be concise -# * Include actionable steps with the necessary code changes -# * Include relevant links (bug issues, upstream issues or release notes, documentation pages) -# * Use full sentences with sentence-casing and punctuation. -# * Before using Datadog specific acronyms/terminology, a release note must first introduce them with a definition. -# -# Release notes should not: -# * Be vague. Example: ``fixes an issue in tracing``. -# * Use overly technical language -# * Use dynamic links (``stable/latest/1.x`` URLs). Instead, use static links (specific version, commit hash) whenever possible so that they don't break in the future. features: - | - tracing: This introduces the record_exception tracer api. It allows to manually add an exception as a span event. - It also allows to tag a span with an error tuple of the recorded exception by setting the escaped parameter as true. \ No newline at end of file + tracing: tracing: Introduces a record_exception method that adds an exception to a Span as a span event. + Refer to [Span.record_exception](https://ddtrace.readthedocs.io/en/stable/api.html#ddtrace.trace.Span.record_exception) + for more details. \ No newline at end of file diff --git a/tests/tracer/test_span.py b/tests/tracer/test_span.py index 1725f0d7675..5d1c6a3b367 100644 --- a/tests/tracer/test_span.py +++ b/tests/tracer/test_span.py @@ -533,6 +533,61 @@ def test_span_pointers(self): }, ] + def test_span_record_exception(self): + span = self.start_span("span") + try: + raise RuntimeError("bim") + except RuntimeError as e: + span.record_exception(e) + span.finish() + + span.assert_span_event_count(1) + span.assert_span_event_attributes( + 0, {"exception.type": "builtins.RuntimeError", "exception.message": "bim", "exception.escaped": "False"} + ) + + def test_span_record_multiple_exceptions(self): + span = self.start_span("span") + try: + raise RuntimeError("bim") + except RuntimeError as e: + span.record_exception(e) + + try: + raise RuntimeError("bam") + except RuntimeError as e: + span.record_exception(e) + span.finish() + + span.assert_span_event_count(2) + span.assert_span_event_attributes( + 0, {"exception.type": "builtins.RuntimeError", "exception.message": "bim", "exception.escaped": "False"} + ) + span.assert_span_event_attributes( + 1, {"exception.type": "builtins.RuntimeError", "exception.message": "bam", "exception.escaped": "False"} + ) + + def test_span_record_escaped_exception(self): + exc = RuntimeError("bim") + span = self.start_span("span") + try: + raise exc + except RuntimeError as e: + span.record_exception(e, escaped=True) + span.finish() + + span.assert_matches( + error=1, + meta={ + "error.message": str(exc), + "error.type": "%s.%s" % (exc.__class__.__module__, exc.__class__.__name__), + }, + ) + span.assert_span_event_count(1) + span.assert_span_event_attributes( + 0, {"exception.type": "builtins.RuntimeError", "exception.message": "bim", "exception.escaped": "True"} + ) + @pytest.mark.parametrize( "value,assertion", diff --git a/tests/tracer/test_tracer.py b/tests/tracer/test_tracer.py index 5e68e6bc588..1c45f424679 100644 --- a/tests/tracer/test_tracer.py +++ b/tests/tracer/test_tracer.py @@ -164,81 +164,6 @@ def f(): ), ) - def test_tracer_record_exception(self): - @self.tracer.wrap() - def f(): - try: - raise RuntimeError("bim") - except RuntimeError as e: - self.tracer.record_exception(e) - - f() - self.assert_structure( - dict( - name="tests.tracer.test_tracer.f", - error=0, - ), - ) - self.spans[0].assert_span_event_count(1) - self.spans[0].assert_span_event_attributes( - 0, {"exception.type": "builtins.RuntimeError", "exception.message": "bim", "exception.escaped": "False"} - ) - - def test_tracer_record_multiple_exceptions(self): - @self.tracer.wrap() - def f(): - try: - raise RuntimeError("bim") - except RuntimeError as e: - self.tracer.record_exception(e) - - try: - raise RuntimeError("bam") - except RuntimeError as e: - self.tracer.record_exception(e) - - f() - self.assert_structure( - dict( - name="tests.tracer.test_tracer.f", - error=0, - ) - ) - self.spans[0].assert_span_event_count(2) - self.spans[0].assert_span_event_attributes( - 0, {"exception.type": "builtins.RuntimeError", "exception.message": "bim", "exception.escaped": "False"} - ) - self.spans[0].assert_span_event_attributes( - 1, {"exception.type": "builtins.RuntimeError", "exception.message": "bam", "exception.escaped": "False"} - ) - - def test_tracer_record_escaped_exception(self): - exc = RuntimeError("bim") - - @self.tracer.wrap() - def f(): - try: - raise exc - except RuntimeError as e: - self.tracer.record_exception(e, escaped=True) - - f() - - self.assert_structure( - dict( - name="tests.tracer.test_tracer.f", - error=1, - meta={ - "error.message": str(exc), - "error.type": "%s.%s" % (exc.__class__.__module__, exc.__class__.__name__), - }, - ), - ) - self.spans[0].assert_span_event_count(1) - self.spans[0].assert_span_event_attributes( - 0, {"exception.type": "builtins.RuntimeError", "exception.message": "bim", "exception.escaped": "True"} - ) - def test_tracer_wrap_multiple_calls(self): @self.tracer.wrap() def f(): From 601845bf7cb1bde71706cea6083f02e510942741 Mon Sep 17 00:00:00 2001 From: Louis Tricot Date: Mon, 10 Feb 2025 11:18:10 +0100 Subject: [PATCH 16/18] chore: add return line in releasenotes --- .../notes/feat-add-dd-record-exception-033fd0436dfd2723.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/releasenotes/notes/feat-add-dd-record-exception-033fd0436dfd2723.yaml b/releasenotes/notes/feat-add-dd-record-exception-033fd0436dfd2723.yaml index 8a6231e3ce8..a9a87aefec1 100644 --- a/releasenotes/notes/feat-add-dd-record-exception-033fd0436dfd2723.yaml +++ b/releasenotes/notes/feat-add-dd-record-exception-033fd0436dfd2723.yaml @@ -3,4 +3,4 @@ features: - | tracing: tracing: Introduces a record_exception method that adds an exception to a Span as a span event. Refer to [Span.record_exception](https://ddtrace.readthedocs.io/en/stable/api.html#ddtrace.trace.Span.record_exception) - for more details. \ No newline at end of file + for more details. From 1f7b4369078ea744f2ff79c62b7d839be0bc30d8 Mon Sep 17 00:00:00 2001 From: Louis Tricot Date: Thu, 13 Feb 2025 10:24:38 +0100 Subject: [PATCH 17/18] review: quinna comments --- ddtrace/_trace/span.py | 4 ++-- tests/tracer/test_span.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ddtrace/_trace/span.py b/ddtrace/_trace/span.py index fe868fb2d03..7f3877defe7 100644 --- a/ddtrace/_trace/span.py +++ b/ddtrace/_trace/span.py @@ -544,7 +544,7 @@ def set_exc_info( def record_exception( self, exception: Exception, - attributes: Optional[Dict[str, str]] = None, + attributes: Optional[Dict[str, _JSONType]] = None, timestamp: Optional[int] = None, escaped=False, ) -> None: @@ -577,7 +577,7 @@ def record_exception( attrs = { "exception.type": "%s.%s" % (exception.__class__.__module__, exception.__class__.__name__), "exception.message": str(exception), - "exception.escaped": str(escaped), + "exception.escaped": escaped, "exception.stacktrace": tb, } if attributes: diff --git a/tests/tracer/test_span.py b/tests/tracer/test_span.py index 5ffacdad800..52d3f17d09f 100644 --- a/tests/tracer/test_span.py +++ b/tests/tracer/test_span.py @@ -543,7 +543,7 @@ def test_span_record_exception(self): span.assert_span_event_count(1) span.assert_span_event_attributes( - 0, {"exception.type": "builtins.RuntimeError", "exception.message": "bim", "exception.escaped": "False"} + 0, {"exception.type": "builtins.RuntimeError", "exception.message": "bim", "exception.escaped": False} ) def test_span_record_multiple_exceptions(self): @@ -561,10 +561,10 @@ def test_span_record_multiple_exceptions(self): span.assert_span_event_count(2) span.assert_span_event_attributes( - 0, {"exception.type": "builtins.RuntimeError", "exception.message": "bim", "exception.escaped": "False"} + 0, {"exception.type": "builtins.RuntimeError", "exception.message": "bim", "exception.escaped": False} ) span.assert_span_event_attributes( - 1, {"exception.type": "builtins.RuntimeError", "exception.message": "bam", "exception.escaped": "False"} + 1, {"exception.type": "builtins.RuntimeError", "exception.message": "bam", "exception.escaped": False} ) def test_span_record_escaped_exception(self): @@ -585,7 +585,7 @@ def test_span_record_escaped_exception(self): ) span.assert_span_event_count(1) span.assert_span_event_attributes( - 0, {"exception.type": "builtins.RuntimeError", "exception.message": "bim", "exception.escaped": "True"} + 0, {"exception.type": "builtins.RuntimeError", "exception.message": "bim", "exception.escaped": True} ) From 771a84ba741c52f730aaa47d610f0d692ba28cb1 Mon Sep 17 00:00:00 2001 From: Louis Tricot Date: Mon, 17 Feb 2025 09:20:16 +0100 Subject: [PATCH 18/18] suggestions from munir --- ddtrace/_trace/span.py | 4 ++-- .../notes/feat-add-dd-record-exception-033fd0436dfd2723.yaml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ddtrace/_trace/span.py b/ddtrace/_trace/span.py index 7f3877defe7..8fcd559b4db 100644 --- a/ddtrace/_trace/span.py +++ b/ddtrace/_trace/span.py @@ -543,7 +543,7 @@ def set_exc_info( def record_exception( self, - exception: Exception, + exception: BaseException, attributes: Optional[Dict[str, _JSONType]] = None, timestamp: Optional[int] = None, escaped=False, @@ -553,7 +553,7 @@ def record_exception( If the exception is uncaught, :obj:`escaped` should be set :obj:`True`. It will tag the span with an error tuple. - :param Exception exception: the exception to record< + :param Exception exception: the exception to record :param dict attributes: optional attributes to add to the span event. It will override the base attributes if :obj:`attributes` contains existing keys. :param int timestamp: the timestamp of the span event. Will be set to now() if timestamp is :obj:`None`. diff --git a/releasenotes/notes/feat-add-dd-record-exception-033fd0436dfd2723.yaml b/releasenotes/notes/feat-add-dd-record-exception-033fd0436dfd2723.yaml index a9a87aefec1..0808b23b71e 100644 --- a/releasenotes/notes/feat-add-dd-record-exception-033fd0436dfd2723.yaml +++ b/releasenotes/notes/feat-add-dd-record-exception-033fd0436dfd2723.yaml @@ -1,6 +1,6 @@ --- features: - | - tracing: tracing: Introduces a record_exception method that adds an exception to a Span as a span event. + tracing: Introduces a record_exception method that adds an exception to a Span as a span event. Refer to [Span.record_exception](https://ddtrace.readthedocs.io/en/stable/api.html#ddtrace.trace.Span.record_exception) for more details.