From 539f2c393b00e118a7766329dc659858143d7229 Mon Sep 17 00:00:00 2001 From: Taegyun Kim Date: Mon, 28 Oct 2024 08:36:56 -0400 Subject: [PATCH 1/2] fix(profiling): handle `Span`s with `None` span type for stack v2 endpoint profiling [backport 2.12] (#11174) Manual backport of #11164 to 2.12 Fixes #11141 ## 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 - [ ] 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) --- .../datadog/profiling/stack_v2/src/stack_v2.cpp | 10 ++++++++-- ...ck-v2-endpoint-span-type-none-48a1196296be3742.yaml | 5 +++++ 2 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/profiling-stack-v2-endpoint-span-type-none-48a1196296be3742.yaml diff --git a/ddtrace/internal/datadog/profiling/stack_v2/src/stack_v2.cpp b/ddtrace/internal/datadog/profiling/stack_v2/src/stack_v2.cpp index 62fa4b38b77..b5cd8a81312 100644 --- a/ddtrace/internal/datadog/profiling/stack_v2/src/stack_v2.cpp +++ b/ddtrace/internal/datadog/profiling/stack_v2/src/stack_v2.cpp @@ -91,7 +91,7 @@ _stack_v2_link_span(PyObject* self, PyObject* args, PyObject* kwargs) uint64_t thread_id; uint64_t span_id; uint64_t local_root_span_id; - const char* span_type; + const char* span_type = nullptr; PyThreadState* state = PyThreadState_Get(); @@ -104,10 +104,16 @@ _stack_v2_link_span(PyObject* self, PyObject* args, PyObject* kwargs) static const char* const_kwlist[] = { "span_id", "local_root_span_id", "span_type", NULL }; static char** kwlist = const_cast(const_kwlist); - if (!PyArg_ParseTupleAndKeywords(args, kwargs, "KKs", kwlist, &span_id, &local_root_span_id, &span_type)) { + if (!PyArg_ParseTupleAndKeywords(args, kwargs, "KKz", kwlist, &span_id, &local_root_span_id, &span_type)) { return NULL; } + // From Python, span_type is a string or None, and when given None, it is passed as a nullptr. + static const std::string empty_string = ""; + if (span_type == nullptr) { + span_type = empty_string.c_str(); + } + ThreadSpanLinks::get_instance().link_span(thread_id, span_id, local_root_span_id, std::string(span_type)); Py_RETURN_NONE; diff --git a/releasenotes/notes/profiling-stack-v2-endpoint-span-type-none-48a1196296be3742.yaml b/releasenotes/notes/profiling-stack-v2-endpoint-span-type-none-48a1196296be3742.yaml new file mode 100644 index 00000000000..1e4fea09afe --- /dev/null +++ b/releasenotes/notes/profiling-stack-v2-endpoint-span-type-none-48a1196296be3742.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + profiling: resolves an issue where endpoint profiling for stack v2 throws + ``TypeError`` exception when it is given a ``Span`` with ``None`` span_type. From 0307129d19de9abfa0878348725b244ffa7a3c5a Mon Sep 17 00:00:00 2001 From: lievan <42917263+lievan@users.noreply.github.com> Date: Mon, 28 Oct 2024 10:57:55 -0400 Subject: [PATCH 2/2] fix(llmobs): don't drop IO annotations equal to zero (#11044) [backport 2.12] (#11163) Backport https://github.com/DataDog/dd-trace-py/commit/2f23eb28c6b4d98b6cac4f84edc2ee3c42095aad from https://github.com/DataDog/dd-trace-py/pull/11044 to 2.12. Fixes an issue where we are dropping I/O annotations that were equal to zero for workflow, task, agent and tool spans. - [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)) - [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) --------- ## 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: lievan --- ddtrace/llmobs/_llmobs.py | 6 +++--- .../fix-numeric-annotations-7cf06b5ceb615282.yaml | 5 +++++ tests/llmobs/test_llmobs_service.py | 11 +++++++++++ 3 files changed, 19 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/fix-numeric-annotations-7cf06b5ceb615282.yaml diff --git a/ddtrace/llmobs/_llmobs.py b/ddtrace/llmobs/_llmobs.py index c76f90f7782..f25adb3bff0 100644 --- a/ddtrace/llmobs/_llmobs.py +++ b/ddtrace/llmobs/_llmobs.py @@ -508,7 +508,7 @@ def annotate( if parameters is not None: log.warning("Setting parameters is deprecated, please set parameters and other metadata as tags instead.") cls._tag_params(span, parameters) - if input_data or output_data: + if input_data is not None or output_data is not None: if span_kind == "llm": cls._tag_llm_io(span, input_messages=input_data, output_messages=output_data) elif span_kind == "embedding": @@ -598,9 +598,9 @@ def _tag_text_io(cls, span, input_value=None, output_value=None): """Tags input/output values for non-LLM kind spans. Will be mapped to span's `meta.{input,output}.values` fields. """ - if input_value: + if input_value is not None: span.set_tag_str(INPUT_VALUE, safe_json(input_value)) - if output_value: + if output_value is not None: span.set_tag_str(OUTPUT_VALUE, safe_json(output_value)) @staticmethod diff --git a/releasenotes/notes/fix-numeric-annotations-7cf06b5ceb615282.yaml b/releasenotes/notes/fix-numeric-annotations-7cf06b5ceb615282.yaml new file mode 100644 index 00000000000..05ed3ddb964 --- /dev/null +++ b/releasenotes/notes/fix-numeric-annotations-7cf06b5ceb615282.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + LLM Observability: This fix resolves an issue where input and output values equal to zero were not being annotated + on workflow, task, agent and tool spans when using `LLMObs.annotate`. diff --git a/tests/llmobs/test_llmobs_service.py b/tests/llmobs/test_llmobs_service.py index 7c51705853d..9f0f4e44710 100644 --- a/tests/llmobs/test_llmobs_service.py +++ b/tests/llmobs/test_llmobs_service.py @@ -471,6 +471,17 @@ def test_annotate_input_string(LLMObs): assert retrieval_span.get_tag(INPUT_VALUE) == "test_input" +def test_annotate_numeric_io(LLMObs): + with LLMObs.task() as task_span: + LLMObs.annotate(span=task_span, input_data=0, output_data=0) + assert task_span.get_tag(INPUT_VALUE) == "0" + assert task_span.get_tag(OUTPUT_VALUE) == "0" + with LLMObs.task() as task_span: + LLMObs.annotate(span=task_span, input_data=1.23, output_data=1.23) + assert task_span.get_tag(INPUT_VALUE) == "1.23" + assert task_span.get_tag(OUTPUT_VALUE) == "1.23" + + def test_annotate_input_serializable_value(LLMObs): with LLMObs.task() as task_span: LLMObs.annotate(span=task_span, input_data=["test_input"])