Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(llmobs): update ragas trace ml app #11952

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

chore(llmobs): update ragas trace ml app #11952

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
f90caa4
small changes
Jan 14, 2025
5bd3840
ragas version parse
Jan 15, 2025
0d8f9a6
ragas ml app updates
Jan 15, 2025
5f35100
Merge branch 'main' of github.com:DataDog/dd-trace-py into evan.li/ra…
Jan 15, 2025
a2580e8
Merge branch 'main' of github.com:DataDog/dd-trace-py into evan.li/ra…
Jan 15, 2025
ea038a9
clarify the ml app is temp
Jan 15, 2025
d7f4630
make sure listeners are removed after all spans are flushed
Jan 16, 2025
e52f564
revert acc change
Jan 16, 2025
74f9b92
Merge branch 'main' into evan.li/ragas-ml-app
lievan Jan 17, 2025
3d2a446
change how we detect eval spans
Jan 17, 2025
2b11d86
Merge branch 'evan.li/ragas-ml-app' of github.com:DataDog/dd-trace-py…
Jan 17, 2025
fb4bb8c
Merge branch 'main' of github.com:DataDog/dd-trace-py into evan.li/ra…
Jan 17, 2025
195cb4a
remove unneeded change
Jan 17, 2025
38e1f62
Merge branch 'main' into evan.li/ragas-ml-app
lievan Jan 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions ddtrace/llmobs/_constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@
RUNNER_IS_INTEGRATION_SPAN_TAG = "runner.integration"

# The ml app of all ragas traces have this prefix that we use to detect
# whether a span is generated from the ragas evaluation itself.
RAGAS_ML_APP_PREFIX = "dd-ragas"
# whether a span is generated from the ragas evaluation itself. We then
# remove this prefix from the ml app before we submit the span.
TEMP_RAGAS_ML_APP_PREFIX = "_dd_ragas_"
lievan marked this conversation as resolved.
Show resolved Hide resolved

ANNOTATIONS_CONTEXT_ID = "annotations_context_id"
INTERNAL_CONTEXT_VARIABLE_KEYS = "_dd_context_variable_keys"
Expand Down
12 changes: 7 additions & 5 deletions ddtrace/llmobs/_evaluators/ragas/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from ddtrace.internal.utils.version import parse_version
from ddtrace.llmobs._constants import INTERNAL_CONTEXT_VARIABLE_KEYS
from ddtrace.llmobs._constants import INTERNAL_QUERY_VARIABLE_KEYS
from ddtrace.llmobs._constants import RAGAS_ML_APP_PREFIX
from ddtrace.llmobs._constants import TEMP_RAGAS_ML_APP_PREFIX


logger = get_logger(__name__)
Expand All @@ -26,8 +26,10 @@ class RagasDependencies:
def __init__(self):
import ragas

self.ragas_version = parse_version(ragas.__version__)
if self.ragas_version >= (0, 2, 0) or self.ragas_version < (0, 1, 10):
self.ragas_version = ragas.__version__ # type: str

parsed_version = parse_version(ragas.__version__)
if parsed_version >= (0, 2, 0) or parsed_version < (0, 1, 10):
raise NotImplementedError(
"Ragas version: {} is not supported".format(self.ragas_version),
)
Expand Down Expand Up @@ -93,8 +95,8 @@ def _get_ml_app_for_ragas_trace(span_event: dict) -> str:
ml_app = tag.split(":")[1]
break
if not ml_app:
return RAGAS_ML_APP_PREFIX
return "{}-{}".format(RAGAS_ML_APP_PREFIX, ml_app)
return TEMP_RAGAS_ML_APP_PREFIX
return "{}{}".format(TEMP_RAGAS_ML_APP_PREFIX, ml_app)


class BaseRagasEvaluator:
Expand Down
14 changes: 7 additions & 7 deletions ddtrace/llmobs/_llmobs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,13 +184,13 @@ def _llmobs_span_event(cls, span: Span) -> Tuple[Dict[str, Any], bool]:
meta.pop("output")
metrics = span._get_ctx_item(METRICS) or {}
ml_app = _get_ml_app(span)
span._set_ctx_item(ML_APP, ml_app)

is_ragas_integration_span = False

if ml_app.startswith(constants.RAGAS_ML_APP_PREFIX):
if ml_app.startswith(constants.TEMP_RAGAS_ML_APP_PREFIX):
is_ragas_integration_span = True
ml_app = ml_app.replace(constants.TEMP_RAGAS_ML_APP_PREFIX, "")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of this temporary set/replace/remove behavior. What is the ultimate goal of using the ragas temp prefix? Can we not set a ragas identifier onto the span's internal store object instead of relying on a temporary ml app name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the suggestion, just added in logic to use the span's store instead


span._set_ctx_item(ML_APP, ml_app)
parent_id = str(_get_llmobs_parent_id(span) or "undefined")

llmobs_span_event = {
Expand Down Expand Up @@ -272,10 +272,6 @@ def _start_service(self) -> None:
log.debug("Error starting evaluator runner")

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)

try:
self._evaluator_runner.stop()
# flush remaining evaluation spans & evaluations
Expand All @@ -290,6 +286,10 @@ def _stop_service(self) -> None:
except ServiceStatusError:
log.debug("Error stopping LLMObs writers")

# Remove listener hooks for span events
Copy link
Contributor Author

@lievan lievan Jan 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makes sure hooks aren't removed prematurely while ragas is still running as triggered by self._evaluator_runner.stop()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this triggering any bugs?

core.reset_listeners("trace.span_start", self._on_span_start)
core.reset_listeners("trace.span_finish", self._on_span_finish)

forksafe.unregister(self._child_after_fork)

@classmethod
Expand Down
2 changes: 1 addition & 1 deletion tests/llmobs/_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ def expected_ragas_trace_tags():
"env:",
"service:tests.llmobs",
"source:integration",
"ml_app:dd-ragas-unnamed-ml-app",
"ml_app:unnamed-ml-app",
"ddtrace.version:{}".format(ddtrace.__version__),
"language:python",
"error:0",
Expand Down
4 changes: 2 additions & 2 deletions tests/llmobs/test_llmobs_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@
from ddtrace.llmobs._constants import OUTPUT_MESSAGES
from ddtrace.llmobs._constants import OUTPUT_VALUE
from ddtrace.llmobs._constants import PROPAGATED_PARENT_ID_KEY
from ddtrace.llmobs._constants import RAGAS_ML_APP_PREFIX
from ddtrace.llmobs._constants import SESSION_ID
from ddtrace.llmobs._constants import SPAN_KIND
from ddtrace.llmobs._constants import SPAN_START_WHILE_DISABLED_WARNING
from ddtrace.llmobs._constants import TAGS
from ddtrace.llmobs._constants import TEMP_RAGAS_ML_APP_PREFIX
from ddtrace.llmobs._llmobs import SUPPORTED_LLMOBS_INTEGRATIONS
from ddtrace.llmobs._writer import LLMObsAgentlessEventClient
from ddtrace.llmobs._writer import LLMObsProxiedEventClient
Expand Down Expand Up @@ -1538,7 +1538,7 @@ def test_llmobs_with_evaluator_runner(llmobs, mock_llmobs_evaluator_runner):


def test_llmobs_with_evaluator_runner_does_not_enqueue_evaluation_spans(mock_llmobs_evaluator_runner, llmobs):
with llmobs.llm(model_name="test_model", ml_app="{}-dummy".format(RAGAS_ML_APP_PREFIX)):
with llmobs.llm(model_name="test_model", ml_app="{}-dummy".format(TEMP_RAGAS_ML_APP_PREFIX)):
pass
time.sleep(0.1)
assert llmobs._instance._evaluator_runner.enqueue.call_count == 0
Expand Down
Loading