Skip to content

Commit

Permalink
fix(llmobs): don't drop IO annotations equal to zero (#11044) [backpo…
Browse files Browse the repository at this point in the history
…rt 2.12] (#11163)

Backport
2f23eb2
from #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 <evan.li@datadoqhq.com>
  • Loading branch information
lievan and lievan authored Oct 28, 2024
1 parent 539f2c3 commit 0307129
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 3 deletions.
6 changes: 3 additions & 3 deletions ddtrace/llmobs/_llmobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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":
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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`.
11 changes: 11 additions & 0 deletions tests/llmobs/test_llmobs_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"])
Expand Down

0 comments on commit 0307129

Please sign in to comment.